Skip to content

Commit

Permalink
Fix the finding position for unused reassignments.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=245015769
  • Loading branch information
graememorgan authored and ronshapiro committed May 5, 2019
1 parent 73322df commit 390f2b1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
Expand Up @@ -665,7 +665,7 @@ private static final class FilterUsedVariables extends TreePathScanner<Void, Voi
// call. // call.
private int inMethodCall = 0; private int inMethodCall = 0;


private final Set<Symbol> hasBeenAssigned = new HashSet<>(); private final Map<Symbol, TreePath> assignmentSite = new HashMap<>();


private TreePath currentExpressionStatement = null; private TreePath currentExpressionStatement = null;


Expand Down Expand Up @@ -702,7 +702,7 @@ private boolean isUsed(@Nullable Symbol symbol) {
public Void visitVariable(VariableTree tree, Void unused) { public Void visitVariable(VariableTree tree, Void unused) {
VarSymbol symbol = getSymbol(tree); VarSymbol symbol = getSymbol(tree);
if (hasBeenAssigned(tree, symbol)) { if (hasBeenAssigned(tree, symbol)) {
hasBeenAssigned.add(symbol); assignmentSite.put(symbol, getCurrentPath());
} }
return super.visitVariable(tree, null); return super.visitVariable(tree, null);
} }
Expand Down Expand Up @@ -779,31 +779,35 @@ private void handleReassignment(AssignmentTree tree) {
} }
Symbol symbol = getSymbol(tree.getVariable()); Symbol symbol = getSymbol(tree.getVariable());
// Check if it was actually assigned to at this depth (or is a parameter). // Check if it was actually assigned to at this depth (or is a parameter).
if (!((hasBeenAssigned.contains(symbol) && symbol.getKind() == ElementKind.LOCAL_VARIABLE) if (!((assignmentSite.containsKey(symbol) && symbol.getKind() == ElementKind.LOCAL_VARIABLE)
|| symbol.getKind() == ElementKind.PARAMETER)) { || symbol.getKind() == ElementKind.PARAMETER)) {
return; return;
} }
if (!declarationSites.containsKey(symbol)) {
return;
}
// Don't regard assigning `null` as a potentially unused assignment, as people do this for GC // Don't regard assigning `null` as a potentially unused assignment, as people do this for GC
// reasons. // reasons.
if (getType(tree.getExpression()) instanceof NullType) { if (getType(tree.getExpression()) instanceof NullType) {
return; return;
} }
hasBeenAssigned.add(symbol); TreePath lastAssignmentSite = assignmentSite.get(symbol);
TreePath assignmentSite = declarationSites.get(symbol); if (lastAssignmentSite == null) {
if (scopeDepth(assignmentSite) != Iterables.size(getCurrentPath().getParentPath())) { return;
}
TreePath declarationSite = declarationSites.get(symbol);
if (declarationSite == null) {
return;
}
if (scopeDepth(declarationSite) != Iterables.size(getCurrentPath().getParentPath())) {
return; return;
} }
if (unusedElements.containsKey(symbol)) { if (unusedElements.containsKey(symbol)) {
unusedSpecs.add(UnusedSpec.of(symbol, assignmentSite, usageSites.get(symbol), tree)); unusedSpecs.add(UnusedSpec.of(symbol, lastAssignmentSite, usageSites.get(symbol), tree));
} else { } else {
isEverUsed.add(symbol); isEverUsed.add(symbol);
} }
unusedElements.put(symbol, getCurrentPath()); unusedElements.put(symbol, getCurrentPath());
usageSites.removeAll(symbol); usageSites.removeAll(symbol);
usageSites.put(symbol, getCurrentPath().getParentPath()); usageSites.put(symbol, getCurrentPath().getParentPath());
assignmentSite.put(symbol, getCurrentPath().getParentPath());
} }


// This is a crude proxy for when a variable is unconditionally overwritten. It doesn't match // This is a crude proxy for when a variable is unconditionally overwritten. It doesn't match
Expand Down
Expand Up @@ -1026,7 +1026,8 @@ public void unusedAssignment_messages() {
" int c = b;", " int c = b;",
" // BUG: Diagnostic contains: This assignment to the local variable", " // BUG: Diagnostic contains: This assignment to the local variable",
" b = 2;", " b = 2;",
" return c;", " b = 3;",
" return b + c;",
" }", " }",
"}") "}")
.doTest(); .doTest();
Expand Down

0 comments on commit 390f2b1

Please sign in to comment.