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 SuggestedFixRules Refaster rule collection #559

Merged
merged 4 commits into from
Apr 5, 2023
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
Expand Up @@ -82,10 +82,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState

SuggestedFix serializationFix = SuggestedFixes.renameMethodInvocation(tree, "concatMap", state);
SuggestedFix concurrencyCapFix =
SuggestedFix.builder()
.postfixWith(
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME)
.build();
SuggestedFix.postfixWith(
Iterables.getOnlyElement(tree.getArguments()), ", " + MAX_CONCURRENCY_ARG_NAME);

Description.Builder description = buildDescription(tree);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package tech.picnic.errorprone.refasterrules;

import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.sun.source.tree.Tree;
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;

/** Refaster rules related to expressions dealing with {@link SuggestedFix}es. */
@OnlineDocumentation
final class SuggestedFixRules {
private SuggestedFixRules() {}

/** Prefer {@link SuggestedFix#delete(Tree)} over more contrived alternatives. */
static final class SuggestedFixDelete {
@BeforeTemplate
SuggestedFix before(Tree tree) {
return SuggestedFix.builder().delete(tree).build();
}

@AfterTemplate
SuggestedFix after(Tree tree) {
return SuggestedFix.delete(tree);
}
}

/** Prefer {@link SuggestedFix#replace(Tree, String)}} over more contrived alternatives. */
static final class SuggestedFixReplaceTree {
@BeforeTemplate
SuggestedFix before(Tree tree, String replaceWith) {
return SuggestedFix.builder().replace(tree, replaceWith).build();
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why the replaceWith part is left out in all titles 👀.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I started out with a name that didn't include it, I think.

}

@AfterTemplate
SuggestedFix after(Tree tree, String replaceWith) {
return SuggestedFix.replace(tree, replaceWith);
}
}

/** Prefer {@link SuggestedFix#replace(int, int, String)}} over more contrived alternatives. */
static final class SuggestedFixReplaceStartEnd {
@BeforeTemplate
SuggestedFix before(int start, int end, String replaceWith) {
return SuggestedFix.builder().replace(start, end, replaceWith).build();
}

@AfterTemplate
SuggestedFix after(int start, int end, String replaceWith) {
return SuggestedFix.replace(start, end, replaceWith);
}
}

/**
* Prefer {@link SuggestedFix#replace(Tree, String, int, int)}} over more contrived alternatives.
*/
static final class SuggestedFixReplaceTreeStartEnd {
@BeforeTemplate
SuggestedFix before(Tree tree, String replaceWith, int start, int end) {
return SuggestedFix.builder().replace(tree, replaceWith, start, end).build();
}

@AfterTemplate
SuggestedFix after(Tree tree, String replaceWith, int start, int end) {
return SuggestedFix.replace(tree, replaceWith, start, end);
}
}

/** Prefer {@link SuggestedFix#swap(Tree, Tree)} over more contrived alternatives. */
static final class SuggestedFixSwap {
Copy link
Member

Choose a reason for hiding this comment

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

What order do we use here? Should we use the same order as the methods are listed in SuggestedFix?

Copy link
Member Author

Choose a reason for hiding this comment

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

Long-term I guess lexicographical order makes most sense. Here I went with "likelihood of being used".

Copy link
Member

Choose a reason for hiding this comment

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

What order do we use here? Should we use the same order as the methods are listed in SuggestedFix?

@BeforeTemplate
SuggestedFix before(Tree tree1, Tree tree2) {
return SuggestedFix.builder().swap(tree1, tree2).build();
}

@AfterTemplate
SuggestedFix after(Tree tree1, Tree tree2) {
return SuggestedFix.swap(tree1, tree2);
}
}

/** Prefer {@link SuggestedFix#prefixWith(Tree, String)} over more contrived alternatives. */
static final class SuggestedFixPrefixWith {
@BeforeTemplate
SuggestedFix before(Tree tree, String prefix) {
return SuggestedFix.builder().prefixWith(tree, prefix).build();
}

@AfterTemplate
SuggestedFix after(Tree tree, String prefix) {
return SuggestedFix.prefixWith(tree, prefix);
}
}

/** Prefer {@link SuggestedFix#postfixWith(Tree, String)}} over more contrived alternatives. */
static final class SuggestedFixPostfixWith {
@BeforeTemplate
SuggestedFix before(Tree tree, String postfix) {
return SuggestedFix.builder().postfixWith(tree, postfix).build();
}

@AfterTemplate
SuggestedFix after(Tree tree, String postfix) {
return SuggestedFix.postfixWith(tree, postfix);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for omitting SuggestedFix#delete(Tree)? I added it :).

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package tech.picnic.errorprone.refasterrules;

import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class SuggestFixRulesTest implements RefasterRuleCollectionTestCase {
SuggestedFix testSuggestedFixDelete() {
return SuggestedFix.builder().delete(null).build();
}

SuggestedFix testSuggestedFixReplaceTree() {
return SuggestedFix.builder().replace(null, "foo").build();
}

SuggestedFix testSuggestedFixReplaceStartEnd() {
return SuggestedFix.builder().replace(1, 2, "foo").build();
}

SuggestedFix testSuggestedFixReplaceTreeStartEnd() {
return SuggestedFix.builder().replace(null, "foo", 1, 2).build();
}

SuggestedFix testSuggestedFixSwap() {
return SuggestedFix.builder().swap((Tree) null, (ExpressionTree) null).build();
}

SuggestedFix testSuggestedFixPrefixWith() {
return SuggestedFix.builder().prefixWith(null, "foo").build();
}

SuggestedFix testSuggestedFixPostfixWith() {
return SuggestedFix.builder().postfixWith(null, "foo").build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package tech.picnic.errorprone.refasterrules;

import com.google.errorprone.fixes.SuggestedFix;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase;

final class SuggestFixRulesTest implements RefasterRuleCollectionTestCase {
SuggestedFix testSuggestedFixDelete() {
return SuggestedFix.delete(null);
}

SuggestedFix testSuggestedFixReplaceTree() {
return Suggestions.replace(null, "foo");
}

SuggestedFix testSuggestedFixReplaceStartEnd() {
return Suggestions.replace(1, 2, "foo");
}

SuggestedFix testSuggestedFixReplaceTreeStartEnd() {
return Suggestions.replace(null, "foo", 1, 2);
}

SuggestedFix testSuggestedFixSwap() {
return Suggestions.swap((Tree) null, (ExpressionTree) null);
}

SuggestedFix testSuggestedFixPrefixWith() {
return Suggestions.prefixWith(null, "foo");
}

SuggestedFix testSuggestedFixPostfixWith() {
return Suggestions.postfixWith(null, "foo");
}
}
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,7 @@
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.8</version>
<version>0.8.9</version>
</plugin>
<plugin>
<groupId>org.pitest</groupId>
Expand Down