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

SONARJAVA-4462 Add quickfix support for S6485 #4461

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

dorian-burihabwa-sonarsource
Copy link
Contributor

No description provided.

@sonarqube-next
Copy link


import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;

class KnownCapacityHashBasedCollectionCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
InternalCheckVerifier.newInstance()

Choose a reason for hiding this comment

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

I realize now that it is misleading having 2 different types of verifier, and one of them as a .withQuickFixes method, but actually the CheckVerifier.newVerifier() that was there before was already checking quickfixes since we introduced the analyzer-commons-api underneath, so you can undo these changes here

.forRule(this)
.onTree(newClassTree)
.withMessage(getIssueMessage(newClassTree))
.withQuickFixes(() -> computeQuickFix(newClassTree))

Choose a reason for hiding this comment

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

Here we can use the singular api withQuickFix since we always provide only one

Comment on lines 80 to 87
private static List<JavaQuickFix> computeQuickFix(NewClassTree newClassTree) {
String replacementMethod = TYPES_TO_METHODS.get(newClassTree.symbolType().name()).replace("(int numMappings)", "");
JavaTextEdit edit = JavaTextEdit.replaceBetweenTree(newClassTree.firstToken(), newClassTree.identifier().lastToken(), replacementMethod);
JavaQuickFix quickFix = JavaQuickFix.newQuickFix("Replace with \"" + replacementMethod + "\".")
.addTextEdit(edit)
.build();
return List.of(quickFix);
}

Choose a reason for hiding this comment

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

This can be simplified to return 1 quickfix

Suggested change
private static List<JavaQuickFix> computeQuickFix(NewClassTree newClassTree) {
String replacementMethod = TYPES_TO_METHODS.get(newClassTree.symbolType().name()).replace("(int numMappings)", "");
JavaTextEdit edit = JavaTextEdit.replaceBetweenTree(newClassTree.firstToken(), newClassTree.identifier().lastToken(), replacementMethod);
JavaQuickFix quickFix = JavaQuickFix.newQuickFix("Replace with \"" + replacementMethod + "\".")
.addTextEdit(edit)
.build();
return List.of(quickFix);
}
private static JavaQuickFix computeQuickFix(NewClassTree newClassTree) {
String replacementMethod = TYPES_TO_METHODS.get(newClassTree.symbolType().name()).replace("(int numMappings)", "");
JavaTextEdit edit = JavaTextEdit.replaceBetweenTree(newClassTree.firstToken(), newClassTree.identifier().lastToken(), replacementMethod);
return JavaQuickFix.newQuickFix("Replace with \"" + replacementMethod + "\".")
.addTextEdit(edit)
.build();
}

* Use the right type of checkverifier
* Set singular quickfix with matching reporting method
* Remove ambiguous test with missing Java version
@leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource merged commit 8fef1ca into master Sep 30, 2024
14 of 15 checks passed
@leonardo-pilastri-sonarsource leonardo-pilastri-sonarsource deleted the db/SONARJAVA-4462 branch September 30, 2024 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants