Skip to content

SONARJAVA-6328 - Implement S8688: Time-based .now() methods should specify a ZoneId or a Clock#5606

Merged
NoemieBenard merged 12 commits intomasterfrom
nb/sonarjava-6328-implement-S8688
May 7, 2026
Merged

SONARJAVA-6328 - Implement S8688: Time-based .now() methods should specify a ZoneId or a Clock#5606
NoemieBenard merged 12 commits intomasterfrom
nb/sonarjava-6328-implement-S8688

Conversation

@NoemieBenard
Copy link
Copy Markdown
Contributor

No description provided.

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented May 6, 2026

SONARJAVA-6328

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented May 6, 2026

Summary

This PR implements rule S8688, which detects calls to .now() without parameters on Java 8+ java.time API classes (LocalDate, LocalDateTime, LocalTime, MonthDay, OffsetDateTime, OffsetTime, Year, YearMonth, ZonedDateTime).

Why it matters: Relying on the system's default timezone can cause inconsistent behavior across different deployment environments (developer machine, CI, production). Explicitly passing a ZoneId or Clock makes the intent clear and ensures consistent behavior.

Implementation: The check extends AbstractMethodDetection to match parameterless .now() calls on the 9 target classes, reporting a single issue at the method name. Instant.now() is intentionally excluded since Instant doesn't store timezone information. The rule is added to the Sonar_way_profile (enabled by default) with "Major" severity.

What reviewers should know

Start with:

  • NowWithoutParametersCheck.java — The core check implementation (51 lines, straightforward MethodMatchers pattern)
  • NowWithoutParametersCheckSample.java — Test sample showing noncompliant calls and compliant fixes

Key design decisions:

  • No Instant flag: Instant.now() is not flagged because Instant doesn't store timezone info; this is correct per the rule's intent
  • 9 target classes: Explicitly listed in the MethodMatchers; LocalDate/Time variants are included even though they don't store timezone, because their .now() methods still depend on the system default to determine "now"
  • Single issue location: Issues point to the method name ("now"), making the fix location clear

What to verify:

  1. MethodMatchers pattern correctly targets only no-argument calls (.addWithoutParametersMatcher())
  2. Test cases cover edge cases (nested calls in print statements, chained method calls, etc.)
  3. Rule documentation is clear about why LocalDate/LocalTime are included despite not storing timezone
  4. Integration tests pass (ruling test data files show expected findings in real projects)

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

sonar-review-alpha[bot]

This comment was marked as resolved.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

}

@Override
public void visitNode(Tree tree) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you extend the AbstractMethodDetection class, you can override getMethodInvocationMatchers method which will trigger the rule on invocations of specific method matchers. Then, instead of calling visitNode and explicitly having to check that the node is a MethodInvocationTree, you can override the onMethodInvocationFound method and directly raise the issue without the need for the condition.

Comment thread java-checks/src/main/java/org/sonar/java/checks/NowWithoutParametersCheck.java Outdated
Comment on lines +49 to +51
if (NOW_MATCHER.matches(mit) && mit.methodSelect() instanceof MemberSelectExpressionTree mset) {
reportIssue(mset.identifier(), mit, "Explicitly specify the time zone by passing a ZoneId or a Clock to the .now() method.");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The call to NOW_MATCHER.matches(mit) is unnecessary here: the method will only be called on method invocations that match the method matchers returned by getMethodInvocationMatchers(). You also know that the method will only ever be invoked on method calls where methodSelect will be a MemberSelectExpressionTree, so you could do something like:

Suggested change
if (NOW_MATCHER.matches(mit) && mit.methodSelect() instanceof MemberSelectExpressionTree mset) {
reportIssue(mset.identifier(), mit, "Explicitly specify the time zone by passing a ZoneId or a Clock to the .now() method.");
}
reportIssue(((MemberSelectExpressionTree) mit.methodSelect()).identifier(), mit, "Explicitly specify the time zone by passing a ZoneId or a Clock to the .now() method.");

@sonarqube-next
Copy link
Copy Markdown

sonarqube-next Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

🗣️ Give feedback

Copy link
Copy Markdown
Contributor

@aurelien-coet-sonarsource aurelien-coet-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM

@NoemieBenard NoemieBenard merged commit 4d4c321 into master May 7, 2026
18 checks passed
@NoemieBenard NoemieBenard deleted the nb/sonarjava-6328-implement-S8688 branch May 7, 2026 10:24
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