Skip to content

Commit

Permalink
Make Unused more clever about which comments to delete.
Browse files Browse the repository at this point in the history
RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218659686
  • Loading branch information
graememorgan authored and cushon committed Nov 2, 2018
1 parent 765c76e commit 63ab3bd
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 20 deletions.
65 changes: 46 additions & 19 deletions core/src/main/java/com/google/errorprone/bugpatterns/Unused.java
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.bugpatterns; package com.google.errorprone.bugpatterns;


import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.Iterables.getLast; import static com.google.common.collect.Iterables.getLast;
import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION; import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
Expand Down Expand Up @@ -53,6 +54,8 @@
import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers; import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers; import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.ErrorProneToken;
import com.google.errorprone.util.ErrorProneTokens;
import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ArrayAccessTree; import com.sun.source.tree.ArrayAccessTree;
import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.AssignmentTree;
Expand Down Expand Up @@ -83,12 +86,14 @@
import com.sun.tools.javac.code.Symbol.TypeSymbol; import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol; import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.parser.Tokens.Comment;
import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree;
import com.sun.tools.javac.tree.JCTree.JCAnnotation; import com.sun.tools.javac.tree.JCTree.JCAnnotation;
import com.sun.tools.javac.tree.JCTree.JCAssign; import com.sun.tools.javac.tree.JCTree.JCAssign;
import com.sun.tools.javac.tree.JCTree.JCAssignOp; import com.sun.tools.javac.tree.JCTree.JCAssignOp;
import com.sun.tools.javac.tree.JCTree.JCExpression; import com.sun.tools.javac.tree.JCTree.JCExpression;
import java.util.Collection; import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -783,39 +788,61 @@ private void removeByIndex(List<? extends Tree> trees) {
* Replaces the tree at {@code path} along with any Javadocs/associated single-line comments. * Replaces the tree at {@code path} along with any Javadocs/associated single-line comments.
* *
* <p>This is the same as just deleting the tree for non-class members. For class members, we * <p>This is the same as just deleting the tree for non-class members. For class members, we
* delete from the end of the previous member (or the start of the class, if it's the first). * tokenize and scan backwards to try to work out which prior comments are associated with this
* node.
*/ */
private static SuggestedFix replaceWithComments( private static SuggestedFix replaceWithComments(
TreePath path, String replacement, VisitorState state) { TreePath path, String replacement, VisitorState state) {
Tree tree = path.getLeaf(); Tree tree = path.getLeaf();
Tree parent = path.getParentPath().getLeaf(); Tree parent = path.getParentPath().getLeaf();
if (parent instanceof ClassTree) { if (!(parent instanceof ClassTree)) {
Tree previousMember = null; return SuggestedFix.replace(tree, replacement);
ClassTree classTree = (ClassTree) parent; }
for (Tree member : classTree.getMembers()) { Tree previousMember = null;
if (member instanceof MethodTree ClassTree classTree = (ClassTree) parent;
&& ASTHelpers.isGeneratedConstructor((MethodTree) member)) { int startTokenization;
continue; for (Tree member : classTree.getMembers()) {
} if (member instanceof MethodTree && ASTHelpers.isGeneratedConstructor((MethodTree) member)) {
if (member.equals(tree)) { continue;
break;
}
previousMember = member;
} }
if (previousMember != null) { if (member.equals(tree)) {
return SuggestedFix.replace( break;
state.getEndPosition(previousMember), state.getEndPosition(tree), replacement);
} }
previousMember = member;
}
if (previousMember != null) {
startTokenization = state.getEndPosition(previousMember);
} else {
int startOfClass = ((JCTree) classTree).getStartPosition(); int startOfClass = ((JCTree) classTree).getStartPosition();
String source = String source =
state state
.getSourceCode() .getSourceCode()
.subSequence(startOfClass, ((JCTree) tree).getStartPosition()) .subSequence(startOfClass, ((JCTree) tree).getStartPosition())
.toString(); .toString();
return SuggestedFix.replace( startTokenization = startOfClass + source.indexOf("{") + 1;
startOfClass + source.indexOf("{") + 1, state.getEndPosition(tree), replacement); }
String source =
state.getSourceCode().subSequence(startTokenization, state.getEndPosition(tree)).toString();
List<ErrorProneToken> tokens = ErrorProneTokens.getTokens(source, state.context);
if (tokens.isEmpty() || tokens.get(0).comments().isEmpty()) {
return SuggestedFix.replace(startTokenization, state.getEndPosition(tree), replacement);
}
List<Comment> comments =
tokens.get(0).comments().stream()
.sorted(Comparator.<Comment>comparingInt(c -> c.getSourcePos(0)).reversed())
.collect(toImmutableList());
int startPos = ((JCTree) tree).getStartPosition() - startTokenization;
// Delete backwards for comments which are not separated from our target by a blank line.
for (Comment comment : comments) {
String stringBetweenComments =
source.substring(comment.getSourcePos(comment.getText().length() - 1), startPos);
if (stringBetweenComments.chars().filter(c -> c == '\n').count() > 1) {
break;
}
startPos = comment.getSourcePos(0);
} }
return SuggestedFix.replace(tree, replacement); return SuggestedFix.replace(
startTokenization + startPos, state.getEndPosition(tree), replacement);
} }


private static boolean isEnhancedForLoopVar(TreePath variablePath) { private static boolean isEnhancedForLoopVar(TreePath variablePath) {
Expand Down
Expand Up @@ -543,7 +543,7 @@ public void unusedTryResource() {
} }


@Test @Test
public void unusedWithComment() { public void removal_javadocsAndNonJavadocs() {
refactoringHelper refactoringHelper
.addInputLines( .addInputLines(
"UnusedWithComment.java", "UnusedWithComment.java",
Expand Down Expand Up @@ -572,6 +572,45 @@ public void unusedWithComment() {
.doTest(TestMode.TEXT_MATCH); .doTest(TestMode.TEXT_MATCH);
} }


@Test
public void removal_trailingComment() {
refactoringHelper
.addInputLines(
"Test.java",
"public class Test {",
" public static final int A = 1; // foo",
"",
" private static final int B = 2;",
"}")
.addOutputLines(
"Test.java", //
"public class Test {",
" public static final int A = 1; // foo",
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test
public void removal_javadocAndSingleLines() {
refactoringHelper
.addInputLines(
"Test.java",
"public class Test {",
" public static final int A = 1; // foo",
"",
" /** Javadoc. */",
" // TODO: fix",
" // BUG: bug",
" private static final int B = 2;",
"}")
.addOutputLines(
"Test.java", //
"public class Test {",
" public static final int A = 1; // foo",
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test @Test
public void usedInLambda() { public void usedInLambda() {
helper helper
Expand Down

0 comments on commit 63ab3bd

Please sign in to comment.