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.

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

Introduce SourceCode#unwrapMethodInvocation utility method #561

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
package tech.picnic.errorprone.bugpatterns.util;

import static com.sun.tools.javac.parser.Tokens.TokenKind.RPAREN;
import static com.sun.tools.javac.util.Position.NOPOS;
import static java.util.stream.Collectors.joining;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.util.ErrorProneToken;
import com.google.errorprone.util.ErrorProneTokens;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.Position;
import java.util.Optional;

/**
* A collection of Error Prone utility methods for dealing with the source code representation of
Expand Down Expand Up @@ -59,4 +69,57 @@ public static SuggestedFix deleteWithTrailingWhitespace(Tree tree, VisitorState
whitespaceEndPos == -1 ? sourceCode.length() : whitespaceEndPos,
"");
}

/**
* Creates a {@link SuggestedFix} for the replacement of the given {@link MethodInvocationTree}
* with just the arguments to the method invocation, effectively "unwrapping" the method
* invocation.
*
* <p>For example, given the method invocation {@code foo.bar(1, 2, 3)}, this method will return a
* {@link SuggestedFix} that replaces the method invocation with {@code 1, 2, 3}.
*
* <p>This method aims to preserve the original formatting of the method invocation, including
* whitespace and comments.
*
* @param tree The AST node to be unwrapped.
* @param state A {@link VisitorState} describing the context in which the given {@link
* MethodInvocationTree} is found.
* @return A non-{@code null} {@link SuggestedFix}.
*/
public static SuggestedFix unwrapMethodInvocation(MethodInvocationTree tree, VisitorState state) {
CharSequence sourceCode = state.getSourceCode();
int startPosition = state.getEndPosition(tree.getMethodSelect());
int endPosition = state.getEndPosition(tree);

if (sourceCode == null || startPosition == Position.NOPOS || endPosition == Position.NOPOS) {
return unwrapMethodInvocationDroppingWhitespaceAndComments(tree, state);
}
Comment on lines +94 to +96
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below: not sure how to test the "source code absent" case, so Pitest will complain about those.


ImmutableList<ErrorProneToken> tokens =
ErrorProneTokens.getTokens(
sourceCode.subSequence(startPosition, endPosition).toString(), state.context);

Optional<Integer> leftParenPosition =
tokens.stream().findFirst().map(t -> startPosition + t.endPos());
Optional<Integer> rightParenPosition =
Streams.findLast(tokens.stream().filter(t -> t.kind() == RPAREN))
.map(t -> startPosition + t.pos());
if (leftParenPosition.isEmpty() || rightParenPosition.isEmpty()) {
return unwrapMethodInvocationDroppingWhitespaceAndComments(tree, state);
}

return SuggestedFix.replace(
tree,
sourceCode
.subSequence(leftParenPosition.orElseThrow(), rightParenPosition.orElseThrow())
.toString());
}

@VisibleForTesting
static SuggestedFix unwrapMethodInvocationDroppingWhitespaceAndComments(
MethodInvocationTree tree, VisitorState state) {
return SuggestedFix.replace(
tree,
tree.getArguments().stream().map(arg -> treeToString(arg, state)).collect(joining(", ")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import javax.lang.model.element.Name;
import org.junit.jupiter.api.Test;

final class SourceCodeTest {
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
BugCheckerRefactoringTestHelper.newInstance(TestChecker.class, getClass());

@Test
void deleteWithTrailingWhitespaceAnnotations() {
refactoringTestHelper
BugCheckerRefactoringTestHelper.newInstance(
DeleteWithTrailingWhitespaceTestChecker.class, getClass())
.addInputLines("AnnotationToBeDeleted.java", "@interface AnnotationToBeDeleted {}")
.expectUnchanged()
.addInputLines(
Expand Down Expand Up @@ -96,7 +96,8 @@ void deleteWithTrailingWhitespaceAnnotations() {

@Test
void deleteWithTrailingWhitespaceMethods() {
refactoringTestHelper
BugCheckerRefactoringTestHelper.newInstance(
DeleteWithTrailingWhitespaceTestChecker.class, getClass())
.addInputLines(
"MethodDeletions.java",
"interface MethodDeletions {",
Expand Down Expand Up @@ -162,13 +163,78 @@ void deleteWithTrailingWhitespaceMethods() {
.doTest(TestMode.TEXT_MATCH);
}

@Test
void unwrapMethodInvocation() {
BugCheckerRefactoringTestHelper.newInstance(UnwrapMethodInvocationTestChecker.class, getClass())
.addInputLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"",
"class A {",
" Object[] m() {",
" return new Object[][] {",
" {ImmutableList.of()},",
" {ImmutableList.of(1)},",
" {com.google.common.collect.ImmutableList.of(1, 2)},",
" {",
" 0, /*a*/",
" ImmutableList /*b*/./*c*/ <Integer> /*d*/of /*e*/(/*f*/ 1 /*g*/, /*h*/ 2 /*i*/) /*j*/",
" }",
" };",
" }",
"}")
.addOutputLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"",
"class A {",
" Object[] m() {",
" return new Object[][] {{}, {1}, {1, 2}, {0, /*a*/ /*f*/ 1 /*g*/, /*h*/ 2 /*i*/ /*j*/}};",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}

@Test
void unwrapMethodInvocationDroppingWhitespaceAndComments() {
BugCheckerRefactoringTestHelper.newInstance(
UnwrapMethodInvocationDroppingWhitespaceAndCommentsTestChecker.class, getClass())
.addInputLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"",
"class A {",
" Object[] m() {",
" return new Object[][] {",
" {ImmutableList.of()},",
" {ImmutableList.of(1)},",
" {com.google.common.collect.ImmutableList.of(1, 2)},",
" {",
" 0, /*a*/",
" ImmutableList /*b*/./*c*/ <Integer> /*d*/of /*e*/(/*f*/ 1 /*g*/, /*h*/ 2 /*i*/) /*j*/",
" }",
" };",
" }",
"}")
.addOutputLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"",
"class A {",
" Object[] m() {",
" return new Object[][] {{}, {1}, {1, 2}, {0, /*a*/ 1, 2 /*j*/}};",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}

/**
* A {@link BugChecker} that uses {@link SourceCode#deleteWithTrailingWhitespace(Tree,
* VisitorState)} to suggest the deletion of annotations and methods with a name containing
* {@value DELETION_MARKER}.
*/
@BugPattern(severity = ERROR, summary = "Interacts with `SourceCode` for testing purposes")
public static final class TestChecker extends BugChecker
public static final class DeleteWithTrailingWhitespaceTestChecker extends BugChecker
implements AnnotationTreeMatcher, MethodTreeMatcher {
private static final long serialVersionUID = 1L;
private static final String DELETION_MARKER = "ToBeDeleted";
Expand All @@ -192,4 +258,37 @@ private Description match(Tree tree, Name name, VisitorState state) {
: Description.NO_MATCH;
}
}

/**
* A {@link BugChecker} that applies {@link
* SourceCode#unwrapMethodInvocation(MethodInvocationTree, VisitorState)} to all method
* invocations.
*/
@BugPattern(severity = ERROR, summary = "Interacts with `SourceCode` for testing purposes")
public static final class UnwrapMethodInvocationTestChecker extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return describeMatch(tree, SourceCode.unwrapMethodInvocation(tree, state));
}
}

/**
* A {@link BugChecker} that applies {@link
* SourceCode#unwrapMethodInvocationDroppingWhitespaceAndComments(MethodInvocationTree,
* VisitorState)} to all method invocations.
*/
@BugPattern(severity = ERROR, summary = "Interacts with `SourceCode` for testing purposes")
public static final class UnwrapMethodInvocationDroppingWhitespaceAndCommentsTestChecker
extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return describeMatch(
tree, SourceCode.unwrapMethodInvocationDroppingWhitespaceAndComments(tree, state));
}
}
}