SONARJAVA-6358 Implement quickfix for S8688#5612
SONARJAVA-6358 Implement quickfix for S8688#5612NoemieBenard wants to merge 2 commits intomasterfrom
Conversation
SummaryImplements quickfix for rule S8688 (NowWithoutParametersCheck). When developers call Changes:
What reviewers should knowWhere to start:
Key points:
Non-obvious decisions:
|
|
There was a problem hiding this comment.
One real bug needs fixing before merge: the quickfix doesn't add the import java.time.ZoneId; statement when it is missing from the user's file. The test passes only because the sample file already imports ZoneId (line 13). In real user code that only calls LocalDateTime.now() without any other ZoneId usage, the applied fix will produce non-compiling code.
| private static JavaQuickFix computeQuickFix(MethodInvocationTree mit) { | ||
| return JavaQuickFix.newQuickFix("Explicitly use the system default by adding \"ZoneId.systemDefault()\"") | ||
| .addTextEdit(JavaTextEdit.replaceTree(mit.arguments(), "(ZoneId.systemDefault())")) |
There was a problem hiding this comment.
The quickfix inserts ZoneId.systemDefault() as a literal string but never adds import java.time.ZoneId;. Users whose file doesn't already import ZoneId will get non-compiling code after applying the fix.
The test passes today only because NowWithoutParametersCheckSample.java happens to import ZoneId already (line 13). A new quickfix-specific test file without that import would surface this failure.
The established pattern in this codebase uses QuickFixHelper.newImportSupplier to conditionally inject the import. See ReturnEmptyArrayNotNullCheck (lines 45, 178–182) for the reference implementation. The fix here requires:
- Add a
QuickFixHelper.ImportSupplier importSupplierfield (lazily initialised, reset inscanFile). - Pass it (or
context) intocomputeQuickFixso it can callimportSupplier.newImportEdit("java.time.ZoneId").ifPresent(builder::addTextEdit).
- Mark as noise
| .withCheck(new NowWithoutParametersCheck()) | ||
| .verifyIssues(); |
There was a problem hiding this comment.
The quickfix annotations in the sample file (fix@qf1, edit@qf1, etc.) are only validated when the verifier runs against a file that does not pre-import ZoneId. Add a dedicated quickfix test file (without import java.time.ZoneId;) and a separate test method for it — following the pattern in ReturnEmptyArrayNotNullCheckTest.quick_fixes(). This will catch the missing-import bug described in the check implementation comment, and matches the convention used by other checks with quickfixes.
- Mark as noise




No description provided.