An Error Prone checker for duplicated DateFormat fields.

Ignores fields in optional groups.

PiperOrigin-RevId: 568912310

core/src/main/java/com/google/errorprone/bugpatterns/DuplicateDateFormatField.java

@@ -0,0 +1,116 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * 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;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.constValue;
+import static java.util.stream.Collectors.joining;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.Tree;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+import javax.annotation.Nullable;
+
+/** Flag DateFormats which use the same field more than once. */
+@BugPattern(summary = "Reuse of DateFormat fields is most likely unintentional", severity = WARNING)
+public final class DuplicateDateFormatField extends MisusedDateFormat {
+  private static final ImmutableSet<Character> PATTERN_CHARACTERS =
+      ImmutableSet.of(
+          'G', 'y', 'Y', 'M', 'L', 'w', 'W', 'D', 'd', 'F', 'E', 'u', 'a', 'H', 'k', 'K', 'h', 'm',
+          's', 'S', 'z', 'Z', 'X');
+
+  private static class PatternCounter implements DateFormatConsumer {
+
+    private final Set<Character> seen = new HashSet<>();
+    private final Set<Character> duplicates = new HashSet<>();
+    @Nullable private Character prev = null;
+    private int optionalGroupDepth = 0;
+
+    @Override
+    public void consumeSpecial(char special) {
+      if (special == '[') {
+        optionalGroupDepth++;
+      } else if (special == ']') {
+        optionalGroupDepth--;
+      }
+      if (!PATTERN_CHARACTERS.contains(special) || optionalGroupDepth > 0) {
+        prev = null;
+        return;
+      }
+      if (prev == null || prev != special) {
+        if (!seen.add(special)) {
+          duplicates.add(special);
+        }
+        prev = special;
+      }
+    }
+
+    @Override
+    public void consumeLiteral(char literal) {
+      prev = null;
+    }
+
+    public Set<Character> getDuplicates() {
+      return duplicates;
+    }
+
+    public static ImmutableSet<Character> getDuplicates(String pattern) {
+      PatternCounter counter = new PatternCounter();
+      parseDateFormat(pattern, counter);
+      return ImmutableSet.copyOf(counter.getDuplicates());
+    }
+  }
+
+  @Override
+  public Optional<String> rewriteTo(String pattern) {
+    return Optional.empty();
+  }
+
+  @Override
+  Description constructDescription(Tree tree, ExpressionTree patternArg, VisitorState state) {
+
+    Optional<String> pattern = Optional.ofNullable(constValue(patternArg, String.class));
+    if (pattern.isEmpty()) {
+      return NO_MATCH;
+    }
+    ImmutableSet<Character> duplicates = PatternCounter.getDuplicates(pattern.get());
+    if (!duplicates.isEmpty()) {
+      return buildDescription(tree).setMessage(buildMessage(pattern.get(), duplicates)).build();
+    }
+    return NO_MATCH;
+  }
+
+  private static String buildMessage(String pattern, ImmutableSet<Character> duplicates) {
+    String duplicatedFields =
+        duplicates.stream().sorted().map(c -> "'" + c + "'").collect(joining(", "));
+    String fieldDescription =
+        duplicates.size() > 1
+            ? String.format("the fields [%s]", duplicatedFields)
+            : String.format("the field %s", duplicatedFields);
+    return String.format(
+        "DateFormat pattern \"%s\" uses %s more than once. Reuse of DateFormat fields is most"
+            + " likely unintentional.",
+        pattern, fieldDescription);
+  }
+}

core/src/main/java/com/google/errorprone/bugpatterns/MisusedDateFormat.java

@@ -114,8 +114,7 @@ public Description matchNewClass(NewClassTree tree, VisitorState state) {
    */
   abstract Optional<String> rewriteTo(String pattern);
 
-  private Description constructDescription(
-      Tree tree, ExpressionTree patternArg, VisitorState state) {
+  Description constructDescription(Tree tree, ExpressionTree patternArg, VisitorState state) {
     return Optional.ofNullable(constValue(patternArg, String.class))
         .flatMap(this::rewriteTo)
         .map(replacement -> describeMatch(tree, replaceArgument(patternArg, replacement, state)))

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

@@ -117,6 +117,7 @@
 import com.google.errorprone.bugpatterns.DoNotMockChecker;
 import com.google.errorprone.bugpatterns.DoNotUseRuleChain;
 import com.google.errorprone.bugpatterns.DoubleBraceInitialization;
+import com.google.errorprone.bugpatterns.DuplicateDateFormatField;
 import com.google.errorprone.bugpatterns.DuplicateMapKeys;
 import com.google.errorprone.bugpatterns.EmptyCatch;
 import com.google.errorprone.bugpatterns.EmptyIfStatement;
@@ -874,6 +875,7 @@ public static ScannerSupplier warningChecks() {
           DoNotClaimAnnotations.class,
           DoNotMockAutoValue.class,
           DoubleCheckedLocking.class,
+          DuplicateDateFormatField.class,
           EmptyBlockTag.class,
           EmptyCatch.class,
           EmptySetMultibindingContributions.class,

core/src/test/java/com/google/errorprone/bugpatterns/DuplicateDateFormatFieldTest.java

@@ -0,0 +1,190 @@
+/*
+ * Copyright 2023 The Error Prone Authors.
+ *
+ * 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;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class DuplicateDateFormatFieldTest {
+
+  private final CompilationTestHelper compilationHelper =
+      CompilationTestHelper.newInstance(DuplicateDateFormatField.class, getClass());
+
+  @Test
+  public void singleDuplicateField() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"mm/dd/yyyy hh:mm:ss\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void doubleDuplicateFields() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the fields ['m', 's'] more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"mm/dd/yyyy" + " hh:mm:ss.sss\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void constantWithDuplicateField() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  static final String PATTERN = \"mm/dd/yyyy hh:mm:ss\";",
+            "  // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(PATTERN);",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void recognizedDateTimeFormat() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.time.format.DateTimeFormatter;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "  DateTimeFormatter formatter = DateTimeFormatter.ofPattern(\"mm/dd/yyyy hh:mm:ss\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void simpleDateFormat_applyPattern() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  public void foo() {",
+            "    SimpleDateFormat format = new SimpleDateFormat();",
+            "    // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "    format.applyPattern(\"mm/dd/yyyy hh:mm:ss\");",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void simpleDateFormat_applyLocalizedPattern() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  public void foo() {",
+            "    SimpleDateFormat format = new SimpleDateFormat();",
+            "    // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "    format.applyLocalizedPattern(\"mm/dd/yyyy hh:mm:ss\");",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void forgotToEscapteSpecialCharacters() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the field 'W' more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"Week W ' of ' L\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void withOptionalGroup() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"hh:mm[:ss] yyyy/mm/dd\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void withNeestedOptionalGroup() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: uses the field 'm' more than once",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"hh:mm[:ss[.SSS]] yyyy/mm/dd\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void noDupliatedFields() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"yyyy-MM-dd\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void ignoresEscapedPatternCharacters() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  SimpleDateFormat format = new SimpleDateFormat(\"'Week' W ' of ' L\");",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void ignoredOptionalGroups() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import java.text.SimpleDateFormat;",
+            "class Test {",
+            "  SimpleDateFormat format = ",
+            "    new SimpleDateFormat(\"yyyy'-'MM'-'dd'T'HH':'mm[':'ss][XXX][X]\");",
+            "}")
+        .doTest();
+  }
+}