Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,11 @@ protected String generateAssertionsForPublicFields(Set<FieldDescription> fields,
private String assertionContentForField(FieldDescription field, ClassDescription classDescription) {
final String fieldName = field.getName();
final String fieldNameCap = capitalize(field.getName());
if (classDescription.getGettersDescriptions().contains(new GetterDescription(fieldName, "get" + fieldNameCap,
field.getTypeDescription(),
Collections.<TypeName> emptyList()))) {

if (classDescription.findGetterDescriptionForField(field) != null) {
Copy link
Member

@joel-costigliola joel-costigliola Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename findGetterDescriptionForField to hasGetterForField and simply return a boolean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the name. For return type, I think it could be useful later to have it returning the value. If we were on Java 8 I'd rather return an Optional<>, though. Thought?s

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, I've realized that there is a very real possibility of multiple. So either:

  1. return a Set<> of Getters
  2. Boolean on first found

I'm okay with either.

return "";
}

String assertionContent = baseAssertionContentFor(field, classDescription);

// we reuse template for properties to have consistent assertions for property and field but change the way we get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
*/
package org.assertj.assertions.generator.description;

import java.util.Collection;
import java.util.Set;
import java.util.TreeSet;
import static org.assertj.assertions.generator.util.ClassUtil.propertyNameOf;

import java.util.*;

import org.apache.commons.lang3.StringUtils;
import org.assertj.assertions.generator.util.ClassUtil;

/**
*
Expand Down Expand Up @@ -129,4 +132,39 @@ public Class<?> getSuperType() {
public void setSuperType(Class<?> superType) {
this.superType = superType;
}

public GetterDescription findGetterDescriptionForField(FieldDescription base) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add unit tests for this in FieldDescriptionTest ?

Copy link
Contributor Author

@Nava2 Nava2 Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense in ClassDescriptionTest?

Edit: It would, but that class doesn't exist... 😅

// Build a map for better look-up
Map<String, GetterDescription> fieldMap = new HashMap<>();
for (GetterDescription getter: this.gettersDescriptions) {
fieldMap.put(getter.getOriginalMember(), getter);
}
for (GetterDescription getter: this.declaredGettersDescriptions) {
fieldMap.put(getter.getOriginalMember(), getter);
}

final String capName = StringUtils.capitalize(propertyNameOf(base.getName()));
if (base.getTypeDescription().isBoolean()) {
// deal with predicates
for (String prefix: ClassUtil.PREDICATE_PREFIXES.keySet()) {
String propName = prefix + capName;

GetterDescription getterDesc = fieldMap.get(propName);
if (getterDesc != null) {
return getterDesc;
}
}
} else {
Copy link
Member

@joel-costigliola joel-costigliola Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the else can be removed and replaced by this one line (assuming we return a boolean):

return fieldMap.containsKey("get" + capName)


String propName = "get" + capName;

GetterDescription getterDesc = fieldMap.get(propName);
if (getterDesc != null) {
return getterDesc;
}
}

// wasn't found
return null;
}
}
51 changes: 32 additions & 19 deletions src/main/java/org/assertj/assertions/generator/util/ClassUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,7 @@
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
import java.lang.reflect.Field;
import java.lang.reflect.GenericArrayType;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.lang.reflect.WildcardType;
import java.lang.reflect.*;
import java.net.URL;
import java.net.URLDecoder;
import java.util.ArrayList;
Expand Down Expand Up @@ -238,28 +230,49 @@ private static Class<?> loadClass(String className, ClassLoader classLoader) thr
/**
* Returns the property name of given getter method, examples :
* <p/>
*
*
* <pre>
* getName -> name
* </pre>
* <p/>
*
*
* <pre>
* isMostValuablePlayer -> mostValuablePlayer
* </pre>
*
* @param getter getter method to deduce property from.
* @param member getter method to deduce property from.
* @return the property name of given getter method
*/
public static String propertyNameOf(Method getter) {
String methodName = getter.getName();
String prefixToRemove = isPredicate(getter) ? IS_PREFIX : GET_PREFIX;
int pos = methodName.indexOf(prefixToRemove);
public static String propertyNameOf(Member member) {
return propertyNameOf(member.getName());
}

/**
* Returns the property name of given getter method, examples :
* <p/>
*
* <pre>
* getName -> name
* </pre>
* <p/>
*
* <pre>
* isMostValuablePlayer -> mostValuablePlayer
* </pre>
*
* @param memberName name of getter or field
* @return the property name of field or method
*/
public static String propertyNameOf(String memberName) {
String predicatePrefix = getPredicatePrefix(memberName);
String prefixToRemove = predicatePrefix != null ? predicatePrefix : GET_PREFIX;

int pos = memberName.indexOf(prefixToRemove);
if (pos != StringUtils.INDEX_NOT_FOUND) {
String propertyWithCapitalLetter = methodName.substring(pos + prefixToRemove.length());
String propertyWithCapitalLetter = memberName.substring(pos + prefixToRemove.length());
return uncapitalize(propertyWithCapitalLetter);
} else {
return methodName;
return memberName;
}
}

Expand Down Expand Up @@ -308,7 +321,7 @@ public static boolean isValidGetterName(String methodName) {

static private final Pattern PREFIX_PATTERN;

static private final Map<String, String> PREDICATE_PREFIXES;
static public final Map<String, String> PREDICATE_PREFIXES;

static private final Comparator<String> LONGEST_TO_SHORTEST = new Comparator<String>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@
* This is a class with properties that clash with public fields.
*/
public class FieldPropertyClash {
public String string;
public String getString() {
return "";
}
}
public String string;
public String getString() {
return "";
}

// joel-costigliola/assertj-assertions-generator#105
// Predicate properties were not properly discerned vs non-predicate
public boolean isBoolean;
public boolean isBoolean() { return false; }

public boolean isNotBoolean() { return false; }
}
37 changes: 37 additions & 0 deletions src/test/resources/FieldPropertyClash.expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,42 @@ public class FieldPropertyClashAssert extends AbstractObjectAssert<FieldProperty
return this;
}

/**
* Verifies that the actual FieldPropertyClash is boolean.
* @return this assertion object.
* @throws AssertionError - if the actual FieldPropertyClash is not boolean.
*/
public FieldPropertyClashAssert isBoolean() {
// check that actual FieldPropertyClash we want to make assertions on is not null.
isNotNull();

// check
if (!actual.isBoolean()) {
failWithMessage("\nExpecting that actual FieldPropertyClash is boolean but is not.");
}

// return the current assertion for method chaining
return this;
}

/**
* Verifies that the actual FieldPropertyClash is not boolean.
* @return this assertion object.
* @throws AssertionError - if the actual FieldPropertyClash is boolean.
*/
public FieldPropertyClashAssert isNotBoolean() {
// check that actual FieldPropertyClash we want to make assertions on is not null.
isNotNull();

// check
if (!actual.isNotBoolean()) {
failWithMessage("\nExpecting that actual FieldPropertyClash is not boolean but is.");
}

// return the current assertion for method chaining
return this;
}



}