Skip to content

Strict unused variable check #819

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

Merged
bulldozer-bot merged 18 commits into from
Sep 13, 2019
Merged

Strict unused variable check #819

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
08e0013
Test demonstrates that public methods aren't checked
iamdanfox Sep 12, 2019
3d9e6ca
Copy in
iamdanfox Sep 12, 2019
94fdb82
remove unnecessary filters
Sep 12, 2019
a907a59
checkstyle
Sep 12, 2019
62188fc
Keep original license
iamdanfox Sep 12, 2019
07c6634
fix
Sep 12, 2019
be9db35
custom supression
Sep 12, 2019
937e078
Merge branch 'dfox/unused-param' of github.com:palantir/gradle-baseli…
Sep 12, 2019
a7d71e4
comment
Sep 12, 2019
e2386a5
default warning
Sep 12, 2019
24a3599
handle interfaces, abstract classes
Sep 12, 2019
7b57bde
Merge remote-tracking branch 'origin/develop' into dfox/unused-param
Sep 12, 2019
a4944d8
supress
Sep 12, 2019
f9920fa
supports enums
Sep 12, 2019
d37f6ee
rename to StrictUnusedVariable
Sep 12, 2019
efdf116
Add generated changelog entries
Sep 12, 2019
14162ed
validate loggers, fix checkstyle
Sep 13, 2019
082a962
warn
Sep 13, 2019
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
Prev Previous commit
Next Next commit
remove unnecessary filters
  • Loading branch information
forozco committed Sep 12, 2019
commit 94fdb82033cf55013fbac83e2519d133af2209e3
23 changes: 2 additions & 21 deletions baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/UnusedParameter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,9 @@
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.util.ASTHelpers.findSuperMethodInType;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
import static com.google.errorprone.util.ASTHelpers.*;
import static com.google.errorprone.util.SideEffectAnalysis.hasSideEffect;
import static com.sun.source.tree.Tree.Kind.POSTFIX_DECREMENT;
import static com.sun.source.tree.Tree.Kind.POSTFIX_INCREMENT;
import static com.sun.source.tree.Tree.Kind.PREFIX_DECREMENT;
import static com.sun.source.tree.Tree.Kind.PREFIX_INCREMENT;
import static com.sun.source.tree.Tree.Kind.*;

import com.google.auto.service.AutoService;
import com.google.common.base.Ascii;
Expand Down Expand Up @@ -180,9 +174,6 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
// and fields. As we go we remove those variables which are used.
Map<Symbol, TreePath> unusedElements = variableFinder.unusedElements;

// Whether a symbol should only be checked for reassignments (e.g. public methods' parameters).
Set<Symbol> onlyCheckForReassignments = variableFinder.onlyCheckForReassignments;

// Map of symbols to their usage sites. In this map we also include the definition site in
// addition to all the trees where symbol is used. This map is designed to keep the usage sites
// of variables (parameters, fields, locals).
Expand Down Expand Up @@ -219,15 +210,10 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
}
SuggestedFix makeFirstAssignmentDeclaration =
makeAssignmentDeclaration(unusedSymbol, specs, allUsageSites, state);
// Don't complain if this is a public method and we only overwrote it once.
if (onlyCheckForReassignments.contains(unusedSymbol) && specs.size() <= 1) {
continue;
}
Tree unused = specs.iterator().next().variableTree().getLeaf();
Symbol.VarSymbol symbol = (Symbol.VarSymbol) unusedSymbol;
ImmutableList<SuggestedFix> fixes;
if (symbol.getKind() == ElementKind.PARAMETER
&& !onlyCheckForReassignments.contains(unusedSymbol)
&& !isEverUsed.contains(unusedSymbol)) {
fixes = buildUnusedParameterFixes(symbol, allUsageSites, state);
} else {
Expand Down Expand Up @@ -533,8 +519,6 @@ private static boolean exemptedByName(Name name) {
private class VariableFinder extends TreePathScanner<Void, Void> {
private final Map<Symbol, TreePath> unusedElements = new HashMap<>();

private final Set<Symbol> onlyCheckForReassignments = new HashSet<>();

private final ListMultimap<Symbol, TreePath> usageSites = ArrayListMultimap.create();

private final VisitorState state;
Expand Down Expand Up @@ -582,9 +566,6 @@ && exemptedFieldBySuperType(getType(variableTree), state)) {
return null;
}
unusedElements.put(symbol, getCurrentPath());
if (!isParameterSubjectToAnalysis(symbol)) {
onlyCheckForReassignments.add(symbol);
}
break;
default:
break;
Expand Down