diff --git a/.gitignore b/.gitignore index b270863a..74f1fd65 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,5 @@ *.jar *.war *.ear +/Vagrantfile +/.vagrant/ diff --git a/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java b/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java index 81ef9838..e85e4b7a 100644 --- a/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java +++ b/src/main/java/org/assertj/assertions/generator/BaseAssertionGenerator.java @@ -12,29 +12,27 @@ */ package org.assertj.assertions.generator; -import com.google.common.collect.Sets; -import com.google.common.reflect.TypeToken; -import org.assertj.assertions.generator.Template.Type; -import org.assertj.assertions.generator.description.ClassDescription; -import org.assertj.assertions.generator.description.DataDescription; -import org.assertj.assertions.generator.description.FieldDescription; -import org.assertj.assertions.generator.description.GetterDescription; -import org.assertj.assertions.generator.util.ClassUtil; +import static com.google.common.collect.Sets.newHashSet; +import static java.lang.String.format; +import static org.apache.commons.lang3.StringUtils.*; +import static org.assertj.assertions.generator.Template.Type.ASSERT_CLASS; +import static org.assertj.assertions.generator.util.ClassUtil.getTypeDeclarationWithinPackage; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.lang.reflect.Method; import java.nio.file.Paths; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; -import static com.google.common.collect.Sets.newHashSet; -import static java.lang.String.format; -import static org.apache.commons.lang3.StringUtils.*; -import static org.assertj.assertions.generator.Template.Type.ASSERT_CLASS; -import static org.assertj.assertions.generator.util.ClassUtil.getTypeDeclarationWithinPackage; +import com.google.common.reflect.TypeToken; +import org.assertj.assertions.generator.Template.Type; +import org.assertj.assertions.generator.description.ClassDescription; +import org.assertj.assertions.generator.description.DataDescription; +import org.assertj.assertions.generator.description.FieldDescription; +import org.assertj.assertions.generator.description.GetterDescription; +import org.assertj.assertions.generator.util.ClassUtil; @SuppressWarnings("WeakerAccess") public class BaseAssertionGenerator implements AssertionGenerator, AssertionsEntryPointGenerator { @@ -372,12 +370,7 @@ private String generateAssertionEntryPointMethodsFor(final Set } private String determineBestEntryPointsAssertionsClassPackage(final Set classDescriptionSet) { - SortedSet packages = new TreeSet<>(new Comparator() { - @Override - public int compare(final String o1, final String o2) { - return o1.length() - o2.length(); - } - }); + SortedSet packages = new TreeSet<>(Comparator.comparing(String::length)); for (ClassDescription classDescription : classDescriptionSet) { packages.add(classDescription.getPackageName()); } @@ -469,19 +462,13 @@ protected void generateAssertionsForPublicFields(StringBuilder assertionsForPubl } private String assertionContentForField(FieldDescription field, ClassDescription classDescription) { - final TypeToken owningType = field.getOwningType(); - final String fieldName = field.getName(); - final String fieldNameCap = capitalize(field.getName()); - try { - Method m = owningType.getRawType().getMethod("get" + fieldNameCap); - if (classDescription.getGettersDescriptions().contains(new GetterDescription(fieldName, owningType, m))) { - return ""; - } - } catch (NoSuchMethodException nsme) { - // ignore it, let flow keep going + // Check for getter existing + if (classDescription.hasGetterForField(field)) { + return ""; } + final String fieldName = field.getName(); String assertionContent = baseAssertionContentFor(field, classDescription); // we reuse template for properties to have consistent assertions for property and field but change the way we get @@ -510,7 +497,7 @@ private String assertionContentForField(FieldDescription field, ClassDescription assertionContent = replace(assertionContent, PREDICATE, field.getPredicate()); assertionContent = replace(assertionContent, PREDICATE_NEG, field.getNegativePredicate()); } - assertionContent = replace(assertionContent, PROPERTY_WITH_UPPERCASE_FIRST_CHAR, fieldNameCap); + assertionContent = replace(assertionContent, PROPERTY_WITH_UPPERCASE_FIRST_CHAR, capitalize(field.getName())); assertionContent = replace(assertionContent, PROPERTY_SIMPLE_TYPE, field.getTypeName(false, false)); assertionContent = replace(assertionContent, PROPERTY_ASSERT_TYPE, 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 15b63659..77a3842d 100644 --- a/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java +++ b/src/main/java/org/assertj/assertions/generator/description/ClassDescription.java @@ -12,13 +12,16 @@ */ package org.assertj.assertions.generator.description; -import com.google.common.reflect.TypeToken; -import org.assertj.assertions.generator.util.ClassUtil; - import java.util.Collection; import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import com.google.common.reflect.TypeToken; +import org.apache.commons.lang3.StringUtils; +import org.assertj.assertions.generator.util.ClassUtil; /** * @@ -102,6 +105,38 @@ public void addDeclaredFieldDescriptions(Set declaredFieldDesc this.declaredFieldsDescriptions.addAll(declaredFieldDescriptions); } + Stream getGetterStreamForField(FieldDescription base) { + final String capName = StringUtils.capitalize(base.getName()); + + Stream getterStream = + Stream.concat(this.gettersDescriptions.stream(), + this.declaredGettersDescriptions.stream()) + .distinct() + .filter(getter -> Objects.equals(base.getValueType(), getter.getValueType())); + + if (ClassUtil.isBoolean(base.getValueType())) { + // deal with predicates by building a set of all of the valid predicates available + Set validNames = ClassUtil.PREDICATE_PREFIXES.keySet() + .stream() + .map(prefix -> prefix + capName) + .collect(Collectors.toSet()); + + return getterStream.filter(getter -> validNames.contains(getter.getOriginalMember().getName())); + } + + final String propName = "get" + capName; + + return getterStream.filter(getter -> Objects.equals(getter.getOriginalMember().getName(), propName)); + } + + public boolean hasGetterForField(FieldDescription base) { + return getGetterStreamForField(base).findAny().isPresent(); + } + + public Set findGettersForField(FieldDescription base) { + return getGetterStreamForField(base).collect(Collectors.toCollection(TreeSet::new)); + } + @Override public String toString() { return "ClassDescription [valueType=" + type + "]"; 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 4ed3befe..039d87ad 100644 --- a/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java +++ b/src/main/java/org/assertj/assertions/generator/util/ClassUtil.java @@ -302,7 +302,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 3bb62381..a0b5d242 100644 --- a/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java +++ b/src/test/java/org/assertj/assertions/generator/data/FieldPropertyClash.java @@ -20,4 +20,24 @@ public class FieldPropertyClash { 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; } + + + public boolean shouldNotBeSomewhere; + public boolean shouldNotBeSomewhere() { return shouldNotBeSomewhere; } + + // different "tenses" + public boolean willBeOutside; + public boolean willNotBeOutside() { return willBeOutside; } + + // + public boolean willNotBeUpsideDown; + public boolean willBeUpsideDown() { return !willNotBeUpsideDown; } + } diff --git a/src/test/java/org/assertj/assertions/generator/description/FieldDescriptionTest.java b/src/test/java/org/assertj/assertions/generator/description/FieldDescriptionTest.java index fe0ccb0f..1dfb77fd 100644 --- a/src/test/java/org/assertj/assertions/generator/description/FieldDescriptionTest.java +++ b/src/test/java/org/assertj/assertions/generator/description/FieldDescriptionTest.java @@ -12,13 +12,16 @@ */ package org.assertj.assertions.generator.description; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.Collections; + import com.google.common.reflect.TypeToken; import org.assertj.assertions.generator.data.Name; import org.assertj.assertions.generator.data.nba.Player; import org.junit.Test; -import static org.assertj.core.api.Assertions.assertThat; - public class FieldDescriptionTest { private static final TypeToken PLAYER_TYPE = TypeToken.of(Player.class); @@ -82,4 +85,133 @@ public void should_generate_readable_predicate_for_error_message() throws Except assertThat(fieldDescription.getPredicateForErrorMessagePart1()).isEqualTo("is disabled"); assertThat(fieldDescription.getPredicateForErrorMessagePart2()).isEqualTo("is not"); } + + private static final TypeToken FIELD_PROP_TYPE = TypeToken.of(FieldPropertyNames.class); + + private static class FieldPropertyNames { + public boolean isBoolean = true; + public boolean isBoolean() { + return isBoolean; + } + + public boolean isNotBoolean() { + return !isBoolean; + } + + public String stringProp; + + public String getStringProp() { + return stringProp; + } + + public int onlyField; + + public String badGetter; + public int getBadGetter() { + return -1; + } + } + + @Test + public void should_find_getter_method_for_predicate_field() throws Exception { + + fieldDescription = new FieldDescription(FieldPropertyNames.class.getDeclaredField("isBoolean"), FIELD_PROP_TYPE); + + GetterDescription posGetter = new GetterDescription("boolean", FIELD_PROP_TYPE, + FieldPropertyNames.class.getMethod("isBoolean")); + GetterDescription negGetter = new GetterDescription("boolean", FIELD_PROP_TYPE, + FieldPropertyNames.class.getMethod("isNotBoolean")); + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + classDesc.addGetterDescriptions(Arrays.asList(posGetter, negGetter)); + + assertThat(classDesc.findGettersForField(fieldDescription)) + .as("found pos/neg getters") + .containsOnlyOnce(posGetter, negGetter); + + assertThat(classDesc.hasGetterForField(fieldDescription)) + .as("Correctly identifies there is a getter") + .isTrue(); + } + + @Test + public void should_find_getter_method_for_field() throws Exception { + + fieldDescription = new FieldDescription(FieldPropertyNames.class.getDeclaredField("stringProp"), FIELD_PROP_TYPE); + + GetterDescription getter = new GetterDescription("stringProp", FIELD_PROP_TYPE, + FieldPropertyNames.class.getMethod("getStringProp")); + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + classDesc.addGetterDescriptions(Collections.singletonList(getter)); + + assertThat(classDesc.findGettersForField(fieldDescription)) + .as("found single getter") + .containsOnly(getter); + + assertThat(classDesc.hasGetterForField(fieldDescription)) + .as("Correctly identifies there is a getter") + .isTrue(); + } + + @Test + public void should_not_find_getter_method_for_mismatched_return_type() throws Exception { + + fieldDescription = new FieldDescription(FieldPropertyNames.class.getDeclaredField("badGetter"), FIELD_PROP_TYPE); + + GetterDescription getter = new GetterDescription("badGetter", FIELD_PROP_TYPE, + FieldPropertyNames.class.getMethod("getBadGetter")); + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + classDesc.addGetterDescriptions(Collections.singletonList(getter)); + + assertThat(classDesc.findGettersForField(fieldDescription)) + .as("found no getters") + .isEmpty(); + + assertThat(classDesc.hasGetterForField(fieldDescription)) + .as("Correctly identifies there is no getter") + .isFalse(); + } + + + @Test + public void should_not_find_getter_method_for_missing_getter() throws Exception { + + fieldDescription = new FieldDescription(FieldPropertyNames.class.getDeclaredField("onlyField"), FIELD_PROP_TYPE); + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + + assertThat(classDesc.findGettersForField(fieldDescription)) + .as("found no getters") + .isEmpty(); + + assertThat(classDesc.hasGetterForField(fieldDescription)) + .as("Correctly identifies there is no getter") + .isFalse(); + } + + // Given that everything is written via reflection, we can not actually get access to a + // non-existant field anymore. Thus, there is no need to test it! +// @Test +// public void should_not_find_getter_method_for_non_existant_field() throws Exception { +// +// } + + @Test(expected = NullPointerException.class) + public void should_fail_find_with_npe_for_null_field_desc() throws Exception { + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + + classDesc.findGettersForField(null); + } + + @Test(expected = NullPointerException.class) + public void should_fail_has_with_npe_for_null_field_desc() throws Exception { + + ClassDescription classDesc = new ClassDescription(FIELD_PROP_TYPE); + + classDesc.hasGetterForField(null); + } + } diff --git a/src/test/resources/FieldPropertyClash.expected.txt b/src/test/resources/FieldPropertyClash.expected.txt index 1a2be51d..da79bf4e 100644 --- a/src/test/resources/FieldPropertyClash.expected.txt +++ b/src/test/resources/FieldPropertyClash.expected.txt @@ -28,6 +28,114 @@ public class FieldPropertyClashAssert extends AbstractObjectAssert