#427 Issue compiler error if put overridden

Author: alicederyn

src/main/java/org/inferred/freebuilder/processor/property/BiMapProperty.java

@@ -27,39 +27,42 @@
 import static org.inferred.freebuilder.processor.model.ModelUtils.erasesToAnyOf;
 import static org.inferred.freebuilder.processor.model.ModelUtils.maybeDeclared;
 import static org.inferred.freebuilder.processor.model.ModelUtils.maybeUnbox;
+import static org.inferred.freebuilder.processor.model.ModelUtils.override;
 import static org.inferred.freebuilder.processor.model.ModelUtils.overrides;
 import static org.inferred.freebuilder.processor.model.ModelUtils.upperBound;
 import static org.inferred.freebuilder.processor.property.MergeAction.appendingToCollections;
 import static org.inferred.freebuilder.processor.source.FunctionalType.consumer;
 import static org.inferred.freebuilder.processor.source.FunctionalType.functionalTypeAcceptedByMethod;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+
+import org.inferred.freebuilder.processor.Datatype;
+import org.inferred.freebuilder.processor.Declarations;
+import org.inferred.freebuilder.processor.excerpt.CheckedBiMap;
+import org.inferred.freebuilder.processor.source.Excerpt;
+import org.inferred.freebuilder.processor.source.FunctionalType;
+import org.inferred.freebuilder.processor.source.SourceBuilder;
+import org.inferred.freebuilder.processor.source.Type;
+import org.inferred.freebuilder.processor.source.Variable;
+
 import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 
+import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.Elements;
 import javax.lang.model.util.Types;
-
-import org.inferred.freebuilder.processor.Datatype;
-import org.inferred.freebuilder.processor.Declarations;
-import org.inferred.freebuilder.processor.excerpt.CheckedBiMap;
-import org.inferred.freebuilder.processor.source.Excerpt;
-import org.inferred.freebuilder.processor.source.FunctionalType;
-import org.inferred.freebuilder.processor.source.SourceBuilder;
-import org.inferred.freebuilder.processor.source.Type;
-import org.inferred.freebuilder.processor.source.Variable;
-
-import com.google.common.base.Preconditions;
-import com.google.common.collect.BiMap;
-import com.google.common.collect.HashBiMap;
-import com.google.common.collect.ImmutableBiMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
+import javax.tools.Diagnostic.Kind;
 
 /**
  * {@link PropertyCodeGenerator} providing fluent methods for {@link BiMap} properties.
@@ -79,8 +82,21 @@ public Optional<BiMapProperty> create(Config config) {
       TypeMirror valueType = upperBound(config.getElements(), type.getTypeArguments().get(1));
       Optional<TypeMirror> unboxedKeyType = maybeUnbox(keyType, config.getTypes());
       Optional<TypeMirror> unboxedValueType = maybeUnbox(valueType, config.getTypes());
+      Optional<ExecutableElement> putMethodOverride = putMethodOverride(
+          config, unboxedKeyType.orElse(keyType), unboxedValueType.orElse(valueType));
       boolean overridesForcePutMethod = hasForcePutMethodOverride(
           config, unboxedKeyType.orElse(keyType), unboxedValueType.orElse(valueType));
+      
+      if (putMethodOverride.isPresent() && !overridesForcePutMethod) {
+        config.getEnvironment().getMessager().printMessage(
+            Kind.ERROR, 
+            "Overriding "
+                + putMethod(property)
+                + " will not correctly validate all inputs. Please override "
+                + forcePutMethod(property)
+                + ".",
+            putMethodOverride.get());
+      }
 
       FunctionalType mutatorType = functionalTypeAcceptedByMethod(
           config.getBuilder(),
@@ -100,6 +116,16 @@ public Optional<BiMapProperty> create(Config config) {
           mutatorType));
     }
 
+    private static Optional<ExecutableElement> putMethodOverride(
+        Config config, TypeMirror keyType, TypeMirror valueType) {
+      return override(
+          config.getBuilder(),
+          config.getTypes(),
+          putMethod(config.getProperty()),
+          keyType,
+          valueType);
+    }
+
     private static boolean hasForcePutMethodOverride(
         Config config, TypeMirror keyType, TypeMirror valueType) {
       return overrides(

src/test/java/org/inferred/freebuilder/processor/property/BiMapPropertyTest.java

@@ -668,6 +668,32 @@ public void testEquality() {
         .runTest();
   }
 
+  @Test
+  public void testCompileErrorIfOnlyPutOverridden() {
+    behaviorTester
+        .with(SourceBuilder.forTesting()
+            .addLine("package com.example;")
+            .addLine("@%s", FreeBuilder.class)
+            .addLine("public interface DataType {")
+            .addLine("  %s<%s, %s> %s;", BiMap.class, keys.type(), values.type(), convention.get())
+            .addLine("")
+            .addLine("  class Builder extends DataType_Builder {")
+            .addLine("    @Override public Builder putItems(%s key, %s value) {",
+                keys.unwrappedType(), values.unwrappedType())
+            .addLine("      %s.checkArgument(%s, \"%s\");",
+                Preconditions.class, keys.validation("key"), keys.errorMessage("key"))
+            .addLine("      %s.checkArgument(%s, \"%s\");",
+                Preconditions.class, values.validation("value"), values.errorMessage("value"))
+            .addLine("      return super.putItems(key, value);")
+            .addLine("    }")
+            .addLine("  }")
+            .addLine("}"))
+        .failsToCompile()
+        .withErrorThat(subject -> subject
+            .hasMessage("Overriding putItems will not correctly validate all inputs. "
+                + "Please override forcePutItems."));
+  }
+
   @Test
   public void testJacksonInteroperability() {
     // See also https://github.com/google/FreeBuilder/issues/68