Checker for parameter order on autovalue constructor call

The check works by looking at the arguments used to call the constructor of the AutoValue object. If a different permutation of the arguments would exactly match the parameters of the constructor then a warning is generated. Arguments for which a name cannot be extracted are left in their original position. Occasionally an author will deliberately swap the order of the constructor call e.g.

 if (first < last) {
   return new AutoValue_Foo(first, last);
 }
 else {
   return new AutoValue_Foo(last, first);
 }

We avoid matching on these by looking to see if the permutation of the arguments exists in another call within the same method.

The check is implemented as a special case of the ArgumentSelectionDefectChecker which in general looks to see if a permutation of the argument names is more suitable than the current one.

RELNOTES: Checker for parameter order on autovalue constructor call

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=155635122

core/src/main/java/com/google/errorprone/bugpatterns/argumentselectiondefects/AutoValueConstructorOrderChecker.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright 2017 Google Inc. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns.argumentselectiondefects;
+
+import static com.google.errorprone.BugPattern.Category.GUAVA;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.NewClassTree;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Type;
+import java.util.function.Function;
+
+/**
+ * Checker to make sure that constructors for AutoValue types are invoked with arguments in the
+ * correct order.
+ *
+ * <p>Warning: this check relies on recovering parameter names from library class files. These names
+ * are only included if you compile with debugging symbols (-g) or with -parameters. You also need
+ * to tell the compiler to read these names from the classfiles and so must compile your project
+ * with -parameters too.
+ *
+ * @author andrewrice@google.com (Andrew Rice)
+ */
+@BugPattern(
+  name = "AutoValueConstructorOrderChecker",
+  summary = "Arguments to AutoValue constructor are in the wrong order",
+  explanation =
+      "AutoValue constructors are synthesized with their parameters in the same order as the "
+          + "abstract accessor methods. Calls to the constructor need to match this ordering.",
+  category = GUAVA,
+  severity = WARNING
+)
+public class AutoValueConstructorOrderChecker extends BugChecker implements NewClassTreeMatcher {
+
+  private ArgumentChangeFinder argumentChangeFinder =
+      ArgumentChangeFinder.builder()
+          .setDistanceFunction(buildDistanceFunction())
+          .addHeuristic(allArgumentsMustMatch())
+          .addHeuristic(new CreatesDuplicateCallHeuristic())
+          .build();
+
+  @Override
+  public Description matchNewClass(NewClassTree tree, VisitorState state) {
+    MethodSymbol sym = ASTHelpers.getSymbol(tree);
+    if (sym == null) {
+      return Description.NO_MATCH;
+    }
+
+    ClassSymbol owner = (ClassSymbol) sym.owner;
+    if (owner == null) {
+      return Description.NO_MATCH;
+    }
+
+    Type superType = owner.getSuperclass();
+    if (superType == null) {
+      return Description.NO_MATCH;
+    }
+
+    Symbol superSymbol = superType.tsym;
+    if (superSymbol == null) {
+      return Description.NO_MATCH;
+    }
+
+    if (!ASTHelpers.hasDirectAnnotationWithSimpleName(superSymbol, "AutoValue")) {
+      return Description.NO_MATCH;
+    }
+
+    MethodSymbol symbol = ASTHelpers.getSymbol(tree);
+    if (symbol == null) {
+      return Description.NO_MATCH;
+    }
+
+    InvocationInfo invocationInfo = InvocationInfo.createFromNewClass(tree, symbol, state);
+
+    Changes changes = argumentChangeFinder.findChanges(invocationInfo);
+
+    if (changes.isEmpty()) {
+      return Description.NO_MATCH;
+    }
+
+    return buildDescription(invocationInfo.tree())
+        .addFix(changes.buildPermuteArgumentsFix(invocationInfo))
+        .build();
+  }
+
+  private static Function<ParameterPair, Double> buildDistanceFunction() {
+    return new Function<ParameterPair, Double>() {
+      @Override
+      public Double apply(ParameterPair parameterPair) {
+        Parameter formal = parameterPair.formal();
+        Parameter actual = parameterPair.actual();
+        if (formal.isUnknownName() || actual.isUnknownName()) {
+          return formal.index() == actual.index() ? 0.0 : 1.0;
+        } else {
+          return formal.name().equals(actual.name()) ? 0.0 : 1.0;
+        }
+      }
+    };
+  }
+
+  private static Heuristic allArgumentsMustMatch() {
+    return (changes, node, sym, state) -> changes.assignmentCost().stream().allMatch(c -> c < 1.0);
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -189,6 +189,7 @@
 import com.google.errorprone.bugpatterns.android.StaticOrDefaultInterfaceMethod;
 import com.google.errorprone.bugpatterns.argumentselectiondefects.ArgumentSelectionDefectChecker;
 import com.google.errorprone.bugpatterns.argumentselectiondefects.AssertEqualsArgumentOrderChecker;
+import com.google.errorprone.bugpatterns.argumentselectiondefects.AutoValueConstructorOrderChecker;
 import com.google.errorprone.bugpatterns.collectionincompatibletype.CollectionIncompatibleType;
 import com.google.errorprone.bugpatterns.collectionincompatibletype.CompatibleWithMisuse;
 import com.google.errorprone.bugpatterns.collectionincompatibletype.IncompatibleArgumentType;
@@ -439,6 +440,7 @@ public static ScannerSupplier errorChecks() {
           AssertFalse.class,
           AssistedInjectAndInjectOnConstructors.class,
           AssistedInjectAndInjectOnSameConstructor.class,
+          AutoValueConstructorOrderChecker.class,
           BigDecimalLiteralDouble.class,
           BindingToUnqualifiedCommonType.class,
           ClassName.class,

core/src/test/java/com/google/errorprone/bugpatterns/argumentselectiondefects/AutoValueConstructorOrderCheckerTest.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2012 Google Inc. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.bugpatterns.argumentselectiondefects;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for AutoValueConstructorOrderChecker */
+@RunWith(JUnit4.class)
+public class AutoValueConstructorOrderCheckerTest {
+
+  private CompilationTestHelper compilationHelper;
+
+  @Before
+  public void setUp() {
+    compilationHelper =
+        CompilationTestHelper.newInstance(AutoValueConstructorOrderChecker.class, getClass());
+  }
+
+  @Test
+  public void autoValueChecker_detectsSwap_withExactNames() throws Exception {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  abstract String valueOne();",
+            "  abstract String valueTwo();",
+            "  static Test create(String valueOne, String valueTwo) {",
+            "    // BUG: Diagnostic contains: new AutoValue_Test(valueOne, valueTwo)",
+            "    return new AutoValue_Test(valueTwo, valueOne);",
+            "  }",
+            "}",
+            "class AutoValue_Test extends Test {",
+            "  String valueOne() { return null; }",
+            "  String valueTwo() { return null; }",
+            "  AutoValue_Test(String valueOne, String valueTwo) {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void autoValueChecker_ignoresSwap_withInexactNames() throws Exception {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  abstract String valueOne();",
+            "  abstract String valueTwo();",
+            "  static Test create(String valueOneArg, String valueTwoArg) {",
+            "    return new AutoValue_Test(valueTwoArg, valueOneArg);",
+            "  }",
+            "}",
+            "class AutoValue_Test extends Test {",
+            "  String valueOne() { return null; }",
+            "  String valueTwo() { return null; }",
+            "  AutoValue_Test(String valueOne, String valueTwo) {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void autoValueChecker_makesNoSuggestion_withCorrectOrder() throws Exception {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.auto.value.AutoValue;",
+            "@AutoValue",
+            "abstract class Test {",
+            "  abstract String valueOne();",
+            "  abstract String valueTwo();",
+            "  static Test create(String valueOne, String valueTwo) {",
+            "    return new AutoValue_Test(valueOne, valueTwo);",
+            "  }",
+            "}",
+            "class AutoValue_Test extends Test {",
+            "  String valueOne() { return null; }",
+            "  String valueTwo() { return null; }",
+            "  AutoValue_Test(String valueOne, String valueTwo) {}",
+            "}")
+        .doTest();
+  }
+}