Add a check to prevent record components of array type.

The default `equals` and `hashCode` for a record with an array component will be inaccurate. Also, records should be immutable, and non-empty arrays are mutable.

PiperOrigin-RevId: 674062270

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

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2024 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 com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.VariableTree;
+import javax.lang.model.type.TypeKind;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(summary = "Record components should not be arrays.", severity = WARNING)
+public final class ArrayRecordComponent extends BugChecker implements VariableTreeMatcher {
+  @Override
+  public Description matchVariable(VariableTree tree, VisitorState state) {
+    var sym = ASTHelpers.getSymbol(tree);
+    // isRecord(VarSymbol) is true iff the symbol represents a record component
+    if (ASTHelpers.isRecord(sym) && sym.asType().getKind() == TypeKind.ARRAY) {
+      return describeMatch(tree);
+    }
+    return Description.NO_MATCH;
+  }
+}

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

@@ -34,6 +34,7 @@
 import com.google.errorprone.bugpatterns.ArrayEquals;
 import com.google.errorprone.bugpatterns.ArrayFillIncompatibleType;
 import com.google.errorprone.bugpatterns.ArrayHashCode;
+import com.google.errorprone.bugpatterns.ArrayRecordComponent;
 import com.google.errorprone.bugpatterns.ArrayToString;
 import com.google.errorprone.bugpatterns.ArraysAsListPrimitiveArray;
 import com.google.errorprone.bugpatterns.AssertFalse;
@@ -855,6 +856,7 @@ public static ScannerSupplier warningChecks() {
           AnnotateFormatMethod.class,
           ArgumentSelectionDefectChecker.class,
           ArrayAsKeyOfSetOrMap.class,
+          ArrayRecordComponent.class,
           AssertEqualsArgumentOrderChecker.class,
           AssertThrowsMultipleStatements.class,
           AssertionFailureIgnored.class,

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

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2024 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 ArrayRecordComponentTest {
+  private final CompilationTestHelper testHelper =
+      CompilationTestHelper.newInstance(ArrayRecordComponent.class, getClass());
+
+  @Test
+  public void positive_recordWithArrayComponents() {
+    testHelper
+        .addSourceLines(
+            "Foo.java",
+            "record Foo(",
+            "    String string,",
+            "    // BUG: Diagnostic contains: Record components should not be arrays",
+            "    int[] array,",
+            "    String otherString,",
+            "    // BUG: Diagnostic contains: Record components should not be arrays",
+            "    String[] otherArray) {}")
+        .doTest();
+  }
+
+  @Test
+  public void negative_recordWithNonArrayComponents() {
+    testHelper
+        .addSourceLines(
+            "Foo.java",
+            "record Foo(",
+            "    String string,",
+            "    java.util.List<String> list,",
+            "    int integer) {}")
+        .doTest();
+  }
+
+  @Test
+  public void negative_recordWithStaticArrayField() {
+    testHelper
+        .addSourceLines(
+            "Foo.java",
+            "record Foo(String string) {",
+            "  private static final int[] ints = {1, 2, 3};",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void negative_nonRecordWithArrayField() {
+    testHelper
+        .addSourceLines(
+            "IntArrayHolder.java",
+            "public class IntArrayHolder {",
+            "  public final int[] ints;",
+            "",
+            "  public IntArrayHolder(int[] ints) {",
+            "    this.ints = ints;",
+            "  }",
+            "}")
+        .doTest();
+  }
+}

docs/bugpattern/ArrayRecordComponent.md

@@ -0,0 +1,12 @@
+There are two main problems with having a component of a record be an array.
+
+1.  By default, the generated `equals` and `hashCode` will just call `equals` or
+    `hashCode` on the array. Two distinct arrays are never considered equal by
+    `equals` even if their contents are the same. The generated `toString` is
+    similarly not useful, since it will be something like `[B@723279cf`.
+
+2.  Arrays are mutable, but records should not be mutable. A client of a record
+    with an array component can change the contents of the array.
+
+Instead of an array component, consider something like `ImmutableList<String>`,
+or, for primitive arrays, something like `ByteString` or `ImmutableIntArray`.