From f5c416862c776bc9d6db9182c3fc7ca5281c975f Mon Sep 17 00:00:00 2001 From: Kevin Brightwell Date: Tue, 30 May 2017 14:15:02 -0400 Subject: [PATCH] Backports fixes from 3de4c3b26c1 - Fixes #105 * Added `findGetterDescriptionForField(FieldDescription base)` to check against all known prefixes for methods that clash * Extended test case for FieldPropertyClash to include predicate case --- .../generator/BaseAssertionGenerator.java | 6 +-- .../description/ClassDescription.java | 44 ++++++++++++++-- .../assertions/generator/util/ClassUtil.java | 51 ++++++++++++------- .../generator/data/FieldPropertyClash.java | 17 +++++-- .../resources/FieldPropertyClash.expected.txt | 37 ++++++++++++++ 5 files changed, 125 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java b/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java index 39879ebf..06bdf43a 100644 --- a/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java +++ b/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java @@ -466,11 +466,11 @@ protected String generateAssertionsForPublicFields(Set 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. emptyList()))) { + + if (classDescription.findGetterDescriptionForField(field) != null) { 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 diff --git a/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java b/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java index d732a494..81ce2928 100644 --- a/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java +++ b/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java @@ -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; /** * @@ -129,4 +132,39 @@ public Class getSuperType() { public void setSuperType(Class superType) { this.superType = superType; } + + public GetterDescription findGetterDescriptionForField(FieldDescription base) { + // Build a map for better look-up + Map 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 { + + String propName = "get" + capName; + + GetterDescription getterDesc = fieldMap.get(propName); + if (getterDesc != null) { + return getterDesc; + } + } + + // wasn't found + return null; + } } diff --git a/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java b/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java index 3939c58b..42dcb128 100644 --- a/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java +++ b/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java @@ -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; @@ -238,28 +230,49 @@ private static Class loadClass(String className, ClassLoader classLoader) thr /** * Returns the property name of given getter method, examples : *

- * + * * 

    * getName -> name
    *
*

- * + * * 

    * isMostValuablePlayer -> mostValuablePlayer
    *
* - * @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 : + *

+ * + * 

+   * getName -> name
+   *
+ *

+ * + * 

+   * isMostValuablePlayer -> mostValuablePlayer
+   *
+ * + * @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; } } @@ -308,7 +321,7 @@ public static boolean isValidGetterName(String methodName) { static private final Pattern PREFIX_PATTERN; - static private final Map PREDICATE_PREFIXES; + static public final Map PREDICATE_PREFIXES; static private final Comparator LONGEST_TO_SHORTEST = new Comparator() { @Override diff --git a/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java b/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java index 63f55e9c..439fe6c6 100644 --- a/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java +++ b/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java @@ -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; } +} \ No newline at end of file diff --git a/src/test/resources/FieldPropertyClash.expected.txt b/src/test/resources/FieldPropertyClash.expected.txt index 25b9f330..85e14e23 100644 --- a/src/test/resources/FieldPropertyClash.expected.txt +++ b/src/test/resources/FieldPropertyClash.expected.txt @@ -51,5 +51,42 @@ public class FieldPropertyClashAssert extends AbstractObjectAssert