diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ArrayRecordComponent.java b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayRecordComponent.java new file mode 100644 index 000000000000..e3fb9fb98b8d --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayRecordComponent.java @@ -0,0 +1,45 @@ +/* + * 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.BugPattern; +import com.google.errorprone.BugPattern.SeverityLevel; +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.", + explanation = + "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.", + severity = SeverityLevel.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; + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 786d0668bd99..a330e5e41bca 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/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; @@ -667,6 +668,7 @@ public static ScannerSupplier warningChecks() { ArrayEquals.class, ArrayFillIncompatibleType.class, ArrayHashCode.class, + ArrayRecordComponent.class, ArrayToString.class, ArraysAsListPrimitiveArray.class, AssistedInjectScoping.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ArrayRecordComponentTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ArrayRecordComponentTest.java new file mode 100644 index 000000000000..f2b57218e1c3 --- /dev/null +++ b/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 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(); + } +}