Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't suggest removing parameters in private methods that are overridden within the current file #4433

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Don't suggest removing parameters in private methods that are overridden within the current file #4433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.matchers.Matchers.SERIALIZATION_METHODS;
import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
import static com.google.errorprone.util.ASTHelpers.findSuperMethods;
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.getType;
Expand All @@ -46,6 +47,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
Expand Down Expand Up @@ -120,6 +122,7 @@
import javax.lang.model.element.Modifier;
import javax.lang.model.element.Name;
import javax.lang.model.type.NullType;
import javax.tools.JavaFileObject;

/** Bugpattern to detect unused declarations. */
@BugPattern(
Expand Down Expand Up @@ -213,7 +216,10 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
return Description.NO_MATCH;
}

VariableFinder variableFinder = new VariableFinder(state);
ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides =
getSuperMethodsToOverrides(tree, state);

VariableFinder variableFinder = new VariableFinder(state, superMethodsToOverrides);
variableFinder.scan(state.getPath(), null);

// Map of symbols to variable declarations. Initially this is a map of all of the local variable
Expand Down Expand Up @@ -353,6 +359,23 @@ public Void visitMethod(MethodTree tree, Void unused) {
return hasAnyNativeMethods.get();
}

private static ImmutableMultimap<MethodSymbol, MethodSymbol> getSuperMethodsToOverrides(
CompilationUnitTree tree, VisitorState state) {
ImmutableMultimap.Builder<MethodSymbol, MethodSymbol> overrides = ImmutableMultimap.builder();
JavaFileObject sourceFile = tree.getSourceFile();
new TreeScanner<Void, Void>() {
@Override
public Void visitMethod(MethodTree tree, Void unused) {
MethodSymbol sym = getSymbol(tree);
findSuperMethods(getSymbol(tree), state.getTypes()).stream()
.filter(m -> sourceFile.equals(m.enclClass().sourcefile))
.forEach(m -> overrides.put(m, sym));
return null;
}
}.scan(tree, null);
return overrides.build();
}

// https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-ExpressionStatement
private static final ImmutableSet<Tree.Kind> TOP_LEVEL_EXPRESSIONS =
ImmutableSet.of(
Expand Down Expand Up @@ -600,9 +623,12 @@ private class VariableFinder extends TreePathScanner<Void, Void> {
private final ListMultimap<Symbol, TreePath> usageSites = ArrayListMultimap.create();

private final VisitorState state;
private final ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides;

private VariableFinder(VisitorState state) {
private VariableFinder(
VisitorState state, ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides) {
this.state = state;
this.superMethodsToOverrides = superMethodsToOverrides;
}

@Override
Expand Down Expand Up @@ -706,7 +732,8 @@ private boolean isParameterSubjectToAnalysis(Symbol sym) {
Symbol enclosingMethod = sym.owner;

if (!(enclosingMethod instanceof MethodSymbol)
|| isAbstract((MethodSymbol) enclosingMethod)) {
|| isAbstract((MethodSymbol) enclosingMethod)
|| superMethodsToOverrides.containsKey(enclosingMethod)) {
return false;
}

Expand Down
1 change: 0 additions & 1 deletion core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1656,7 +1656,6 @@ public void unusedFunctionalInterfaceParameter_noFix() {
.doTest();
}

@Ignore("https://github.com/google/error-prone/issues/4409")
@Test
public void parameterUsedInOverride() {
refactoringHelper
Expand Down
Loading