Introduce UnqualifiedSuggestedFixImport check
#880
Conversation
|
Looks good. All 20 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Stephan202
force-pushed
the
sschroevers/import-suggestion-check
branch
from
c6ecb82 to
b752e72
Compare
|
Added one more commit with some cleanup that is enabled by this check; that code will be further improved by #882 once it lands. |
|
Kudos, SonarCloud Quality Gate passed! |
|
Looks good. All 21 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| switch (ASTHelpers.getSymbol(tree).name.toString()) { |
There was a problem hiding this comment.
| switch (ASTHelpers.getSymbol(tree).name.toString()) { | |
| switch (ASTHelpers.getSymbol(tree).getSimpleName().toString()) { |
What do we prefer in these cases?
There was a problem hiding this comment.
We should use the method name indeed 👍
| linkType = CUSTOM, | ||
| severity = WARNING, | ||
| tags = FRAGILE_CODE) | ||
| public final class ImportSuggestion extends BugChecker implements MethodInvocationTreeMatcher { |
There was a problem hiding this comment.
I'm thinking that ImportSuggestion sounds a bit "generic" for its goal. Maybe one doesn't directly link Suggestion to the SuggestedFix part. Perhaps the word SuggestedFix should be in there as it is specific to that.
There was a problem hiding this comment.
Hmm, hard to come up with another name. Perhaps UnqualifiedSuggestedFixImport?
There was a problem hiding this comment.
Sounds good IMO, I'll commit the change.
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java
Show resolved
Hide resolved
Stephan202
force-pushed
the
sschroevers/import-suggestion-check
branch
from
b752e72 to
30777b3
Compare
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| switch (ASTHelpers.getSymbol(tree).name.toString()) { |
There was a problem hiding this comment.
We should use the method name indeed 👍
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotation.java
Show resolved
Hide resolved
| linkType = CUSTOM, | ||
| severity = WARNING, | ||
| tags = FRAGILE_CODE) | ||
| public final class ImportSuggestion extends BugChecker implements MethodInvocationTreeMatcher { |
There was a problem hiding this comment.
Hmm, hard to come up with another name. Perhaps UnqualifiedSuggestedFixImport?
|
Looks good. All 21 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Stephan202
changed the title
ImportSuggestion checkUnqualifiedSuggestedFixImport check
Usage of `SuggestedFix.Builder#add{,Static}Import` does not always yield
valid code, so this check suggests alternatives instead.
|
Rename LGTM. I updated the PR title and suggested commit message. Now resolving conflict. |
Stephan202
force-pushed
the
sschroevers/import-suggestion-check
branch
from
5a2b276 to
6ccd2a0
Compare
|
|
Looks good. All 21 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Oh I missed something? 🤔 |
|
Nope, unrelated conflict. |
|
Haha just did it 😄. |
Suggested commit message: