Detect conflicts between record components and explicit accessor methods https://github.com/google/error-prone/issues/5177 PiperOrigin-RevId: 911220057
diff --git a/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java b/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java index 80bf5fc..b8fe73e 100644 --- a/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java +++ b/check_api/src/main/java/com/google/errorprone/util/MoreAnnotations.java
@@ -19,6 +19,7 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.Streams; import com.sun.tools.javac.code.Attribute; +import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -89,6 +90,25 @@ return Optional.ofNullable(a.accept(new Visitor(), null)); } + public static <T extends Enum<T>> EnumSet<T> asEnumValues(Class<T> clazz, AnnotationValue a) { + EnumSet<T> result = EnumSet.noneOf(clazz); + class Visitor extends SimpleAnnotationValueVisitor8<Void, Void> { + @Override + public Void visitEnumConstant(VariableElement c, Void unused) { + result.add(Enum.valueOf(clazz, c.getSimpleName().toString())); + return null; + } + + @Override + public Void visitArray(List<? extends AnnotationValue> list, Void unused) { + list.stream().forEach(a -> a.accept(this, null)); + return null; + } + } + a.accept(new Visitor(), null); + return result; + } + /** Converts the given attribute to a type. */ public static Optional<TypeMirror> asTypeValue(AnnotationValue a) { class Visitor extends SimpleAnnotationValueVisitor8<TypeMirror, Void> {
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflict.java b/core/src/main/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflict.java new file mode 100644 index 0000000..6d87ad5 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflict.java
@@ -0,0 +1,137 @@ +/* + * Copyright 2026 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.common.collect.ImmutableMap.toImmutableMap; +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.getSymbol; +import static com.google.errorprone.util.ASTHelpers.getType; +import static com.google.errorprone.util.ASTHelpers.hasExplicitSource; +import static com.google.errorprone.util.ASTHelpers.isRecord; +import static com.google.errorprone.util.MoreAnnotations.asEnumValues; +import static com.google.errorprone.util.MoreAnnotations.getValue; + +import com.google.common.collect.ImmutableMap; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; +import com.sun.tools.javac.code.Attribute; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.code.Symbol.ClassSymbol; +import com.sun.tools.javac.code.Symbol.RecordComponent; +import com.sun.tools.javac.code.Symbol.TypeSymbol; +import com.sun.tools.javac.tree.JCTree; +import java.lang.annotation.ElementType; +import java.util.EnumSet; +import javax.lang.model.element.Name; + +/** A BugPattern; see the summary. */ +@BugPattern(summary = "Annotation on record component is ignored.", severity = WARNING) +public class RecordComponentAccessorAnnotationConflict extends BugChecker + implements ClassTreeMatcher { + + @Override + public Description matchClass(ClassTree tree, VisitorState state) { + ClassSymbol classSymbol = getSymbol(tree); + if (classSymbol == null || !isRecord(classSymbol)) { + return NO_MATCH; + } + + ImmutableMap<Name, RecordComponent> components = + classSymbol.getRecordComponents().stream() + .collect(toImmutableMap(c -> c.getSimpleName(), c -> c)); + for (Tree member : tree.getMembers()) { + if (!(member instanceof MethodTree methodTree)) { + continue; + } + if (getSymbol(methodTree).isConstructor()) { + for (VariableTree parameter : methodTree.getParameters()) { + check(state, parameter, ElementType.PARAMETER, components); + } + } + if (methodTree.getParameters().isEmpty()) { + check(state, methodTree, ElementType.METHOD, components); + } + } + return NO_MATCH; + } + + private void check( + VisitorState state, + Tree tree, + ElementType elementType, + ImmutableMap<Name, RecordComponent> components) { + if (!hasExplicitSource(tree, state)) { + return; + } + Symbol symbol = getSymbol(tree); + RecordComponent component = components.get(symbol.getSimpleName()); + if (component == null) { + return; + } + for (JCTree.JCAnnotation annotation : component.getOriginalAnnos()) { + TypeSymbol annotationElement = getType(annotation).tsym; + if (!targetsOnly(annotationElement, elementType)) { + continue; + } + if (symbol.getAnnotationMirrors().stream() + .anyMatch(a -> a.getAnnotationType().asElement().equals(annotationElement))) { + continue; + } + // The annotation doesn't have an end position, so we can't use + // state.getSourceForNode. + // See also https://bugs.openjdk.org/browse/JDK-8383786. + @SuppressWarnings("TreeToString") + String string = annotation.toString(); + state.reportMatch( + buildDescription(tree) + .setMessage( + switch (elementType) { + case METHOD -> + String.format( + "Annotation %s on record component %s with @Target(METHOD) is ignored" + + " when an explicit accessor is present.", + string, symbol); + case PARAMETER -> + String.format( + "Annotation %s on record component %s with @Target(PARAMETER) is" + + " ignored when an explicit constructor is present.", + string, symbol); + default -> message(); + }) + .addFix(SuggestedFix.prefixWith(tree, string)) + .build()); + } + } + + private static boolean targetsOnly(TypeSymbol annotation, ElementType elementType) { + Attribute.Compound target = annotation.getAnnotationTypeMetadata().getTarget(); + if (target == null) { + // If there's no explicit @Target, the annotation implicitly targets all declaration contexts + return false; + } + return getValue(target, "value") + .map(v -> asEnumValues(ElementType.class, v).equals(EnumSet.of(elementType))) + .orElse(false); + } +}
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 18b06b4..1fa73ab 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -343,6 +343,7 @@ import com.google.errorprone.bugpatterns.RandomModInteger; import com.google.errorprone.bugpatterns.ReachabilityFenceUsage; import com.google.errorprone.bugpatterns.RecordAccessorInCompactConstructor; +import com.google.errorprone.bugpatterns.RecordComponentAccessorAnnotationConflict; import com.google.errorprone.bugpatterns.RecordComponentOverride; import com.google.errorprone.bugpatterns.RedundantControlFlow; import com.google.errorprone.bugpatterns.RedundantOverride; @@ -1305,6 +1306,7 @@ PrivateConstructorForUtilityClass.class, PublicApiNamedStreamShouldReturnStream.class, QualifierWithTypeUse.class, + RecordComponentAccessorAnnotationConflict.class, RedundantNullCheck.class, RedundantOverride.class, RedundantThrows.class,
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflictTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflictTest.java new file mode 100644 index 0000000..39dd7d8 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/RecordComponentAccessorAnnotationConflictTest.java
@@ -0,0 +1,246 @@ +/* + * Copyright 2026 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.BugCheckerRefactoringTestHelper; +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class RecordComponentAccessorAnnotationConflictTest { + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance( + RecordComponentAccessorAnnotationConflict.class, getClass()); + private final BugCheckerRefactoringTestHelper refactoringHelper = + BugCheckerRefactoringTestHelper.newInstance( + RecordComponentAccessorAnnotationConflict.class, getClass()); + + @Test + public void recordWithExplicitAccessorMissingAnnotation_warns() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target(ElementType.METHOD) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + // BUG: Diagnostic contains: RecordComponentAccessorAnnotationConflict + public int x() { + return this.x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithExplicitAccessorMissingAnnotation_refactor() { + refactoringHelper + .addInputLines( + "MyAnnotation.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target(ElementType.METHOD) + @interface MyAnnotation { + int value(); + } + """) + .expectUnchanged() + .addInputLines( + "Foo.java", + """ + record Foo(@MyAnnotation(42) int x) { + public int x() { + return this.x; + } + } + """) + .addOutputLines( + "Foo.java", + """ + record Foo(@MyAnnotation(42) int x) { + @MyAnnotation(42) + public int x() { + return this.x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithExplicitAccessorRepeatingAnnotation_noWarning() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target({ElementType.METHOD, ElementType.RECORD_COMPONENT}) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + @MyAnnotation + public int x() { + return this.x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithGeneratedAccessor_noWarning() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target({ElementType.METHOD, ElementType.RECORD_COMPONENT}) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) {} + """) + .doTest(); + } + + @Test + public void recordWithGeneratedAccessor_defaultTarget_noWarning() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import javax.annotation.Nullable; + + record Foo(@Nullable int x) {} + """) + .doTest(); + } + + @Test + public void recordWithExplicitConstructor() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target(ElementType.PARAMETER) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + // BUG: Diagnostic contains: RecordComponentAccessorAnnotationConflict + Foo(int x) { + this.x = x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithImplicitConstructor() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target(ElementType.PARAMETER) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) {} + """) + .doTest(); + } + + @Test + public void recordWithCompactConstrucutor() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target(ElementType.PARAMETER) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + Foo { + x = x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithDualAnnotation_explicitAccessor() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target({ElementType.METHOD, ElementType.PARAMETER}) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + public int x() { + return this.x; + } + } + """) + .doTest(); + } + + @Test + public void recordWithDualAnnotation_explicitConstructor() { + compilationHelper + .addSourceLines( + "Foo.java", + """ + import java.lang.annotation.ElementType; + import java.lang.annotation.Target; + + @Target({ElementType.METHOD, ElementType.PARAMETER}) + @interface MyAnnotation {} + + record Foo(@MyAnnotation int x) { + Foo { + x = x; + } + } + """) + .doTest(); + } +}
diff --git a/docs/bugpattern/RecordComponentAccessorAnnotationConflict.md b/docs/bugpattern/RecordComponentAccessorAnnotationConflict.md new file mode 100644 index 0000000..27459c1 --- /dev/null +++ b/docs/bugpattern/RecordComponentAccessorAnnotationConflict.md
@@ -0,0 +1,27 @@ +This check detects cases where an annotation on a record component is ignored. + +## Method annotations + +When an annotation that is applicable to methods is placed on a record +component, the Java compiler automatically propagates it to the generated +accessor method. If you provide an explicit accessor method instead of letting +the compiler generate one, the annotation is NOT automatically propagated to +your explicit accessor. + +If you intended the annotation to be on the accessor method, you must repeat it +on the explicit accessor method declaration. If you did not intend it to be on +the accessor, and the annotation does not target `RECORD_COMPONENT`, then +placing it on the record component has no effect and is likely an error. + +## Parameter annotations + +When an annotation that is applicable to parameters is placed on a record +component, the Java compiler propagates it to the parameter of the generated +constructor. If you provide an explicit constructor, the annotation is not +automatically propagated to the parameters of the explicit constructor + +If you intended the annotation to be on the parameter method, you must repeat it +on the parameter of the explicit constructor declaration. If you did not intend +it to be on the parameter, and the annotation does not target +`RECORD_COMPONENT`, then placing it on the record component has no effect and is +likely an error.