-
Notifications
You must be signed in to change notification settings - Fork 39
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 BugCheckerRules
Refaster rule collection
#526
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package tech.picnic.errorprone.refasterrules; | ||
|
||
import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChooser; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; | ||
import com.google.errorprone.refaster.Refaster; | ||
import com.google.errorprone.refaster.annotation.AfterTemplate; | ||
import com.google.errorprone.refaster.annotation.BeforeTemplate; | ||
import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; | ||
|
||
/** Refaster rules related to {@link com.google.errorprone.bugpatterns.BugChecker} classes. */ | ||
@OnlineDocumentation | ||
final class BugCheckerRules { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly not sure about the reach of this class 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, good question. In the past we have dodged questions such as this one by locating single rules in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hahaha, yeah I saw this class and hesitated, but felt like doing this was cleaner ;) |
||
private BugCheckerRules() {} | ||
|
||
/** | ||
* Avoid calling {@link BugCheckerRefactoringTestHelper#setFixChooser(FixChooser)} or {@link | ||
* BugCheckerRefactoringTestHelper#setImportOrder(String)} with their respective default values. | ||
*/ | ||
static final class BugCheckerRefactoringTestHelperIdentity { | ||
@BeforeTemplate | ||
BugCheckerRefactoringTestHelper before(BugCheckerRefactoringTestHelper helper) { | ||
return Refaster.anyOf( | ||
helper.setFixChooser(FixChoosers.FIRST), helper.setImportOrder("static-first")); | ||
} | ||
|
||
@AfterTemplate | ||
BugCheckerRefactoringTestHelper after(BugCheckerRefactoringTestHelper helper) { | ||
return helper; | ||
} | ||
} | ||
|
||
/** | ||
* Prefer {@link BugCheckerRefactoringTestHelper.ExpectOutput#expectUnchanged()} over repeating | ||
* the input. | ||
*/ | ||
// XXX: This rule assumes that the full source code is specified as a single string, e.g. using a | ||
// text block. Support for multi-line source code input would require a `BugChecker` | ||
// implementation instead. | ||
Comment on lines
+37
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now this rule won't match, but after #198 it'll be relevant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @Stephan202 💪 |
||
static final class BugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged { | ||
@BeforeTemplate | ||
BugCheckerRefactoringTestHelper before( | ||
BugCheckerRefactoringTestHelper helper, String path, String source) { | ||
return helper.addInputLines(path, source).addOutputLines(path, source); | ||
} | ||
|
||
@AfterTemplate | ||
BugCheckerRefactoringTestHelper after( | ||
BugCheckerRefactoringTestHelper helper, String path, String source) { | ||
return helper.addInputLines(path, source).expectUnchanged(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package tech.picnic.errorprone.refasterrules; | ||
|
||
import com.google.common.collect.ImmutableSet; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; | ||
import com.google.errorprone.bugpatterns.BugChecker; | ||
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; | ||
|
||
final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { | ||
@Override | ||
public ImmutableSet<?> elidedTypesAndStaticImports() { | ||
return ImmutableSet.of(FixChoosers.class); | ||
} | ||
|
||
ImmutableSet<BugCheckerRefactoringTestHelper> testBugCheckerRefactoringTestHelperIdentity() { | ||
return ImmutableSet.of( | ||
BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) | ||
.setFixChooser(FixChoosers.FIRST), | ||
BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) | ||
.setImportOrder("static-first")); | ||
} | ||
|
||
BugCheckerRefactoringTestHelper | ||
testBugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged() { | ||
return BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) | ||
.addInputLines("A.java", "class A {}") | ||
.addOutputLines("A.java", "class A {}"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package tech.picnic.errorprone.refasterrules; | ||
|
||
import com.google.common.collect.ImmutableSet; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper; | ||
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers; | ||
import com.google.errorprone.bugpatterns.BugChecker; | ||
import tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase; | ||
|
||
final class BugCheckerRulesTest implements RefasterRuleCollectionTestCase { | ||
@Override | ||
public ImmutableSet<?> elidedTypesAndStaticImports() { | ||
return ImmutableSet.of(FixChoosers.class); | ||
} | ||
|
||
ImmutableSet<BugCheckerRefactoringTestHelper> testBugCheckerRefactoringTestHelperIdentity() { | ||
return ImmutableSet.of( | ||
BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()), | ||
BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass())); | ||
} | ||
|
||
BugCheckerRefactoringTestHelper | ||
testBugCheckerRefactoringTestHelperAddInputLinesExpectUnchanged() { | ||
return BugCheckerRefactoringTestHelper.newInstance(BugChecker.class, getClass()) | ||
.addInputLines("A.java", "class A {}") | ||
.expectUnchanged(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to pull in test dependencies at runtime; this should become
provided
.