Skip to content

Commit

Permalink
Extend MissingOverride to require @OverRide on "an explicitly declare…
Browse files Browse the repository at this point in the history
…d accessor method for a record component"

PiperOrigin-RevId: 677005515
  • Loading branch information
cushon authored and Error Prone Team committed Sep 20, 2024
1 parent 2c04ada commit be99217
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@

package com.google.errorprone.bugpatterns;

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.getEnclosedElements;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.ASTHelpers.streamSuperMethods;

import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.ErrorProneFlags;
Expand All @@ -30,6 +33,8 @@
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.util.Name;
import javax.inject.Inject;
import javax.lang.model.element.Modifier;

Expand Down Expand Up @@ -61,6 +66,25 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
if (ignoreInterfaceOverrides && sym.enclClass().isInterface()) {
return NO_MATCH;
}
if (ASTHelpers.isRecord(sym.owner)
&& sym.getModifiers().contains(Modifier.PUBLIC)
&& sym.getParameters().isEmpty()) {

This comment has been minimized.

Copy link
@wendigo

wendigo Oct 1, 2024

This should filter out secondary constructors :)

ImmutableSet<Name> components =
getEnclosedElements(sym.owner).stream()
.filter(ASTHelpers::isRecord)
.map(Symbol::getSimpleName)
.collect(toImmutableSet());
if (components.contains(sym.getSimpleName())) {
return buildDescription(tree)
.addFix(SuggestedFix.prefixWith(tree, "@Override "))
.setMessage(
String.format(
"%s is an explicitly declared accessor method for a record component; expected"
+ " @Override",
sym.getSimpleName()))
.build();
}
}
return streamSuperMethods(sym, state.getTypes())
.findFirst()
.filter(unused -> ASTHelpers.getGeneratedBy(state).isEmpty())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.errorprone.bugpatterns;

import static com.google.common.truth.TruthJUnit.assume;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.CompilationTestHelper;
import org.junit.Test;
Expand Down Expand Up @@ -190,4 +192,28 @@ public interface Test extends Super {
""")
.doTest();
}

@Test
public void explicitRecordAccessor() {
assume().that(Runtime.version().feature()).isAtLeast(16);

compilationHelper
.addSourceLines(
"Baz.java",
"""
public record Baz(int x) {
// BUG: Diagnostic contains: x is an explicitly declared accessor method
public int x() {
return x;
}
public int x(int y) {
return y;
}
public int y() {
return x;
}
}
""")
.doTest();
}
}

0 comments on commit be99217

Please sign in to comment.