StronglyTypeDuration: generalise to Instant, Joda types, and proto time.

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

core/pom.xml

@@ -280,6 +280,13 @@
       <version>${compile.testing.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <!-- BSD -->
+      <groupId>com.google.protobuf</groupId>
+      <artifactId>protobuf-java-util</artifactId>
+      <version>3.7.1</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <!-- ICU License -->
       <groupId>com.ibm.icu</groupId>

core/src/main/java/com/google/errorprone/bugpatterns/time/StronglyTypeDuration.java → core/src/main/java/com/google/errorprone/bugpatterns/time/StronglyTypeTime.java

@@ -18,8 +18,11 @@
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.constructor;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.google.errorprone.util.ASTHelpers.getType;
 import static com.google.errorprone.util.ASTHelpers.isConsideredFinal;
 import static com.google.errorprone.util.ASTHelpers.isSameType;
 
@@ -41,6 +44,7 @@
 import com.sun.source.tree.MemberSelectTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePathScanner;
@@ -55,24 +59,52 @@
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.Modifier;
 
-/** Flags fields which would be better expressed as Durations rather than primitive integers. */
+/** Flags fields which would be better expressed as time types rather than primitive integers. */
 @BugPattern(
-    name = "StronglyTypeDuration",
+    name = "StronglyTypeTime",
     summary =
-        "This primitive integral type is only used to construct Durations. It would be clearer to"
+        "This primitive integral type is only used to construct time types. It would be clearer to"
             + " strongly type the field instead.",
     severity = WARNING)
-public final class StronglyTypeDuration extends BugChecker implements CompilationUnitTreeMatcher {
-  private static final Matcher<ExpressionTree> DURATION_FACTORY =
-      staticMethod()
-          .onClass("java.time.Duration")
-          .namedAnyOf("ofDays", "ofHours", "ofMillis", "ofMinutes", "ofNanos", "ofSeconds")
-          .withParameters("long");
+public final class StronglyTypeTime extends BugChecker implements CompilationUnitTreeMatcher {
+  private static final Matcher<ExpressionTree> TIME_FACTORY =
+      anyOf(
+          // Java time.
+          staticMethod()
+              .onClass("java.time.Duration")
+              .namedAnyOf("ofDays", "ofHours", "ofMillis", "ofMinutes", "ofNanos", "ofSeconds")
+              .withParameters("long"),
+          staticMethod()
+              .onClass("java.time.Instant")
+              .namedAnyOf("ofEpochMilli", "ofEpochSecond")
+              .withParameters("long"),
+          // Proto time.
+          staticMethod()
+              .onClass("com.google.protobuf.util.Timestamps")
+              .namedAnyOf("fromNanos", "fromMicros", "fromMillis", "fromSeconds"),
+          staticMethod()
+              .onClass("com.google.protobuf.util.Durations")
+              .namedAnyOf(
+                  "fromNanos",
+                  "fromMicros",
+                  "fromMillis",
+                  "fromSeconds",
+                  "fromMinutes",
+                  "fromHours",
+                  "fromDays"),
+          // Joda time.
+          staticMethod()
+              .onClass("org.joda.time.Duration")
+              .namedAnyOf(
+                  "millis", "standardSeconds", "standardMinutes", "standardHours", "standardDays")
+              .withParameters("long"),
+          constructor().forClass("org.joda.time.Instant").withParameters("long"),
+          constructor().forClass("org.joda.time.DateTime").withParameters("long"));
 
   @Override
   public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
     Map<VarSymbol, VariableTree> fields = findPotentialFields(state);
-    SetMultimap<VarSymbol, MethodInvocationTree> usages = HashMultimap.create();
+    SetMultimap<VarSymbol, ExpressionTree> usages = HashMultimap.create();
 
     new TreePathScanner<Void, Void>() {
       @Override
@@ -93,12 +125,12 @@ private void handle(Tree tree) {
           return;
         }
         Tree parent = getCurrentPath().getParentPath().getLeaf();
-        if (!(parent instanceof MethodInvocationTree)
-            || !DURATION_FACTORY.matches((MethodInvocationTree) parent, state)) {
+        if (!(parent instanceof ExpressionTree)
+            || !TIME_FACTORY.matches((ExpressionTree) parent, state)) {
           fields.remove(symbol);
           return;
         }
-        usages.put((VarSymbol) symbol, (MethodInvocationTree) parent);
+        usages.put((VarSymbol) symbol, (ExpressionTree) parent);
       }
     }.scan(tree, null);
 
@@ -109,28 +141,47 @@ private void handle(Tree tree) {
   }
 
   private Description describeMatch(
-      VariableTree variableTree, Set<MethodInvocationTree> invocationTrees, VisitorState state) {
+      VariableTree variableTree, Set<ExpressionTree> invocationTrees, VisitorState state) {
     if (invocationTrees.stream().map(ASTHelpers::getSymbol).distinct().count() != 1) {
       return NO_MATCH;
     }
-    MethodInvocationTree method = invocationTrees.iterator().next();
+    ExpressionTree factory = invocationTrees.iterator().next();
     String newName = createNewName(variableTree.getName().toString());
     SuggestedFix.Builder fix = SuggestedFix.builder();
-    String duration = SuggestedFixes.qualifyType(state, fix, "java.time.Duration");
+    Type targetType = getType(factory);
+    String typeName = SuggestedFixes.qualifyType(state, fix, targetType);
     fix.replace(
         variableTree,
         String.format(
             "%s %s %s = %s(%s);",
             state.getSourceForNode(variableTree.getModifiers()),
-            duration,
+            typeName,
             newName,
-            state.getSourceForNode(method.getMethodSelect()),
+            getMethodSelectOrNewClass(factory, state),
             state.getSourceForNode(variableTree.getInitializer())));
 
-    for (MethodInvocationTree methodInvocationTree : invocationTrees) {
-      fix.replace(methodInvocationTree, newName);
+    for (ExpressionTree expressionTree : invocationTrees) {
+      fix.replace(expressionTree, newName);
+    }
+    return buildDescription(variableTree)
+        .setMessage(
+            String.format(
+                "This primitive integral type is only used to construct %s instances. It would be"
+                    + " clearer to strongly type the field instead.",
+                targetType.tsym.getSimpleName()))
+        .addFix(fix.build())
+        .build();
+  }
+
+  private static String getMethodSelectOrNewClass(ExpressionTree tree, VisitorState state) {
+    switch (tree.getKind()) {
+      case METHOD_INVOCATION:
+        return state.getSourceForNode(((MethodInvocationTree) tree).getMethodSelect());
+      case NEW_CLASS:
+        return "new " + state.getSourceForNode(((NewClassTree) tree).getIdentifier());
+      default:
+        throw new AssertionError();
     }
-    return describeMatch(variableTree, fix.build());
   }
 
   private static final Pattern TIME_UNIT_REMOVER =

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

@@ -433,7 +433,7 @@
 import com.google.errorprone.bugpatterns.time.PreferJavaTimeOverload;
 import com.google.errorprone.bugpatterns.time.ProtoDurationGetSecondsGetNano;
 import com.google.errorprone.bugpatterns.time.ProtoTimestampGetSecondsGetNano;
-import com.google.errorprone.bugpatterns.time.StronglyTypeDuration;
+import com.google.errorprone.bugpatterns.time.StronglyTypeTime;
 import com.google.errorprone.bugpatterns.time.TemporalAccessorGetChronoField;
 import com.google.errorprone.bugpatterns.time.TimeUnitConversionChecker;
 import com.google.errorprone.bugpatterns.time.TimeUnitMismatch;
@@ -880,7 +880,7 @@ public static ScannerSupplier errorChecks() {
           StaticOrDefaultInterfaceMethod.class,
           StaticQualifiedUsingExpression.class,
           StringEquality.class,
-          StronglyTypeDuration.class,
+          StronglyTypeTime.class,
           SwitchDefault.class,
           SystemExitOutsideMain.class,
           TestExceptionChecker.class,

core/src/test/java/com/google/errorprone/bugpatterns/time/StronglyTypeDurationTest.java → core/src/test/java/com/google/errorprone/bugpatterns/time/StronglyTypeTimeTest.java

@@ -22,14 +22,14 @@
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/** Tests for {@link StronglyTypeDuration}. */
+/** Tests for {@link StronglyTypeTime}. */
 @RunWith(JUnit4.class)
-public final class StronglyTypeDurationTest {
+public final class StronglyTypeTimeTest {
   private final CompilationTestHelper compilationHelper =
-      CompilationTestHelper.newInstance(StronglyTypeDuration.class, getClass());
+      CompilationTestHelper.newInstance(StronglyTypeTime.class, getClass());
 
   private final BugCheckerRefactoringTestHelper refactoringHelper =
-      BugCheckerRefactoringTestHelper.newInstance(new StronglyTypeDuration(), getClass());
+      BugCheckerRefactoringTestHelper.newInstance(new StronglyTypeTime(), getClass());
 
   @Test
   public void findingLocatedOnField() {
@@ -38,7 +38,7 @@ public void findingLocatedOnField() {
             "Test.java",
             "import java.time.Duration;",
             "class Test {",
-            "  // BUG: Diagnostic contains:",
+            "  // BUG: Diagnostic contains: Duration instances",
             "  private static final long FOO_MILLIS = 100;",
             "  public Duration get() {",
             "    return Duration.ofMillis(FOO_MILLIS);",
@@ -63,6 +63,57 @@ public void findingOnBoxedField() {
         .doTest();
   }
 
+
+  @Test
+  public void jodaInstant() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import org.joda.time.Instant;",
+            "class Test {",
+            "  private static final long FOO_MILLIS = 100L;",
+            "  public Instant get() {",
+            "    return new Instant(FOO_MILLIS);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import org.joda.time.Instant;",
+            "class Test {",
+            "  private static final Instant FOO = new Instant(100L);",
+            "  public Instant get() {",
+            "    return FOO;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void protoTimestamp() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+            "import com.google.protobuf.Timestamp;",
+            "import com.google.protobuf.util.Timestamps;",
+            "class Test {",
+            "  private static final long FOO_MILLIS = 100L;",
+            "  public Timestamp get() {",
+            "    return Timestamps.fromMillis(FOO_MILLIS);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import com.google.protobuf.Timestamp;",
+            "import com.google.protobuf.util.Timestamps;",
+            "class Test {",
+            "  private static final Timestamp FOO = Timestamps.fromMillis(100L);",
+            "  public Timestamp get() {",
+            "    return FOO;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void fieldQualifiedByThis() {
     compilationHelper