add values() to EnumOrdinal check

This check is in JavaCodeClarity, only runs on changed lines.

While testing, discovered that it would flag a lot of instances where `values()[` was being invoked from within the same class. I wasn't sure if that was a reasonable thing to do, seems like it might be a valid internal design choice. See the 'negative_enumValues_internalCall' test case as an example. Or see some of the instances in unknown commit

So I added an additional check of the enclosing class to ignore internal usage. That flagged fewer things: unknown commit

PiperOrigin-RevId: 705970604

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

@@ -18,29 +18,58 @@
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
 
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.ArrayAccessTreeMatcher;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ArrayAccessTree;
+import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
 
 /** Discourages the use of {@link Enum#ordinal()} and other ways to access enum values by index. */
 @BugPattern(
-    summary = "You should almost never invoke the Enum.ordinal() method.",
+    summary =
+        "You should almost never invoke the Enum.ordinal() method or depend on the enum values by"
+            + " index.",
     severity = WARNING)
-public final class EnumOrdinal extends BugChecker implements MethodInvocationTreeMatcher {
+public final class EnumOrdinal extends BugChecker
+    implements MethodInvocationTreeMatcher, ArrayAccessTreeMatcher {
 
   private static final Matcher<ExpressionTree> ORDINAL =
       instanceMethod().onDescendantOf("java.lang.Enum").named("ordinal");
 
+  private static final Matcher<ExpressionTree> VALUES =
+      staticMethod().onDescendantOf("java.lang.Enum").named("values");
+
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
     if (ORDINAL.matches(tree, state)) {
       return describeMatch(tree);
     }
     return Description.NO_MATCH;
   }
+
+  @Override
+  public Description matchArrayAccess(ArrayAccessTree tree, VisitorState state) {
+    if (!(tree.getExpression() instanceof MethodInvocationTree mit)) {
+      return Description.NO_MATCH;
+    }
+    if (!VALUES.matches(tree.getExpression(), state)) {
+      return Description.NO_MATCH;
+    }
+    MethodSymbol methodInvocationSymbol = getSymbol(mit);
+    ClassSymbol enclosingClassSymbol = getSymbol(state.findEnclosing(ClassTree.class));
+    if (methodInvocationSymbol.isEnclosedBy(enclosingClassSymbol)) {
+      return Description.NO_MATCH;
+    }
+    return describeMatch(tree);
+  }
 }

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

@@ -47,4 +47,63 @@ public int callOrdinal() {
             """)
         .doTest();
   }
+
+  @Test
+  public void positive_enumValues_externalCall() {
+    testHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            enum TestEnum {
+              FOO,
+              BAR,
+            }
+
+            class Caller {
+              public TestEnum callValues() {
+                // BUG: Diagnostic contains: ordinal
+                return TestEnum.values()[0];
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void negative_enumValues_internalCall() {
+    testHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            enum TestEnum {
+              FOO,
+              BAR;
+
+              private static TestEnum fromValues() {
+                return values()[0];
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void negative_enumValues_noIndex() {
+    testHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            enum TestEnum {
+              FOO,
+              BAR,
+            }
+
+            class Caller {
+              public TestEnum[] callValues() {
+                return TestEnum.values();
+              }
+            }
+            """)
+        .doTest();
+  }
 }

docs/bugpattern/EnumOrdinal.md

@@ -1,7 +1,8 @@
-You should almost never invoke the `Enum.ordinal()` method. The ordinal exists
-only to support low-level utilities like `EnumSet`. The ordinal of a given enum
-value is not guaranteed to be stable across builds because of the potential for
-enum values to be added, removed, or reordered.
+You should almost never invoke the `Enum.ordinal()` method, nor depend on some
+enum constant being at a particular index of the `values()` array. The ordinal
+of a given enum value is not guaranteed to be stable across builds because of
+the potential for enum values to be added, removed, or reordered. The ordinal
+exists only to support low-level utilities like `EnumSet`.
 
 Prefer using enum value directly:
 
@@ -13,7 +14,7 @@ ImmutableMap<MyEnum, String> MAPPING =
         .buildOrThrow();
 ```
 
-to this:
+instead of relying on the ordinal:
 
 ```java
 ImmutableMap<Integer, String> MAPPING =
@@ -23,18 +24,30 @@ ImmutableMap<Integer, String> MAPPING =
         .buildOrThrow();
 ```
 
-Or if you need a stable number for serialisation, consider defining an explicit
-field on the enum instead:
+If you need a stable number for serialisation, consider defining an explicit
+field on the enum:
 
 ```java
 enum MyStableEnum {
   FOO(1),
   BAR(2),
   ;
 
-  private final int index;
-  MyStableEnum(int index) {
-    this.index = index;
+  private final int wireCode;
+  MyStableEnum(int wireCode) {
+    this.wireCode = wireCode;
   }
 }
 ```
+
+rather than relying on the ordinal values:
+
+```java
+enum MyUnstableEnum {
+  FOO,
+  BAR,
+}
+MyUnstableEnum fromWire(int wireCode) {
+  return MyUnstableEnum.values()[wireCode];
+}
+```