Skip to content

SONARJAVA-6358 Implement quickfix for S8688 #5612

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.

Sign up for GitHub

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

Open
NoemieBenard wants to merge 2 commits into
base: master
Choose a base branch
from
Open

SONARJAVA-6358 Implement quickfix for S8688 #5612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87c2d08
implement quickfix for S8688
NoemieBenard May 8, 2026
0bd8ae5
update rule metadata
NoemieBenard May 8, 2026
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
14 changes: 14 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/NowWithoutParametersCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,18 @@ Year nowOnOtherPatterns() {
// ^^^^^
}

// fix@qf1 {{Explicitly use the system default by adding "ZoneId.systemDefault()"}}
// edit@qf1 [[sc=47;ec=49]] {{(ZoneId.systemDefault())}}
void quickfix() {
LocalDateTime dateTime = LocalDateTime.now(); // Noncompliant [[quickfixes=qf1]]
// ^^^^^
}

// fix@qf2 {{Explicitly use the system default by adding "ZoneId.systemDefault()"}}
// edit@qf2 [[sc=41;ec=43]] {{(ZoneId.systemDefault())}}
void quickfix2() {
String formatted = ZonedDateTime.now().format(DateTimeFormatter.ISO_DATE); // Noncompliant [[quickfixes=qf2]]
// ^^^^^
}

}
18 changes: 17 additions & 1 deletion java-checks/src/main/java/org/sonar/java/checks/NowWithoutParametersCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
package org.sonar.java.checks;

import org.sonar.check.Rule;
import org.sonar.java.checks.helpers.QuickFixHelper;
import org.sonar.java.checks.methods.AbstractMethodDetection;
import org.sonar.java.reporting.JavaQuickFix;
import org.sonar.java.reporting.JavaTextEdit;
import org.sonar.plugins.java.api.JavaVersion;
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
Expand Down Expand Up @@ -46,6 +49,19 @@ protected MethodMatchers getMethodInvocationMatchers() {

@Override
protected void onMethodInvocationFound(MethodInvocationTree mit) {
reportIssue(((MemberSelectExpressionTree) mit.methodSelect()).identifier(), mit, "Explicitly specify the time zone by passing a ZoneId or a Clock to the .now() method.");
MemberSelectExpressionTree mset = (MemberSelectExpressionTree) mit.methodSelect();
QuickFixHelper.newIssue(context)
.forRule(this)
.onRange(mset.identifier(), mit)
.withMessage("Explicitly specify the time zone by passing a ZoneId or a Clock to the .now() method.")
.withQuickFix(() -> computeQuickFix(mit))
.report();
}

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())"))
Comment on lines +61 to +63
Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot May 8, 2026

Choose a reason for hiding this comment

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

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:

  1. Add a QuickFixHelper.ImportSupplier importSupplier field (lazily initialised, reset in scanFile).
  2. Pass it (or context) into computeQuickFix so it can call importSupplier.newImportEdit("java.time.ZoneId").ifPresent(builder::addTextEdit).
  • Mark as noise

.build();
}

}
1 change: 1 addition & 0 deletions java-checks/src/test/java/org/sonar/java/checks/NowWithoutParametersCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ void test() {
.withCheck(new NowWithoutParametersCheck())
.verifyIssues();
Comment on lines 30 to 31
Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot May 8, 2026

Choose a reason for hiding this comment

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

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

}

}
2 changes: 1 addition & 1 deletion sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8688.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"ruleSpecification": "RSPEC-8688",
"sqKey": "S8688",
"scope": "All",
"quickfix": "unknown",
"quickfix": "covered",
"code": {
"impacts": {
"MAINTAINABILITY": "HIGH",
Expand Down
Loading