SONARJAVA-6330: Implement S8692 - The system clock should not be used in tests by aurelien-coet-sonarsource · Pull Request #5611 · SonarSource/sonar-java

aurelien-coet-sonarsource · 2026-05-08T06:32:37Z

No description provided.

sonar-review-alpha · 2026-05-08T13:05:57Z

Summary

Implements S8692: a new static analysis rule that detects direct use of the system clock in unit tests. The rule flags calls to .now() methods on java.time types (without parameters or with ZoneId), Clock.system*() methods, and System.currentTimeMillis().

Tests should use fixed or mocked clocks via dependency injection to remain deterministic and reproducible. The PR includes the check implementation, comprehensive unit test samples showing compliant and noncompliant patterns, rule documentation explaining the problem and solutions, and baseline expectations from real-world projects (commons-beanutils, eclipse-jetty, sonar-server).

What reviewers should know

Check implementation: SystemClockCheck.java uses MethodMatchers to detect three groups of invocations: parameterless .now() calls, .now(ZoneId) calls on java.time types; Clock.system*() methods; and System.currentTimeMillis(). Straightforward pattern-based detection with no complex logic.

Test coverage: SystemClockCheckSample.java demonstrates the rule in action—start here to see what triggers and what's compliant (fixed clocks, mocked clocks, nanoTime).

Documentation: S8692.html provides clear rationale (test flakiness/non-determinism) and two solution patterns (fixed clocks, injected mocks). Good reference for reviewers unfamiliar with clock testing patterns.

Ruling baselines: Four JSON files contain expected findings from the autoscan and ruling test suites. These are critical for integration testing—verify they align with actual scan results. Minor note: S3577 false negatives increased by 1, likely a side effect of test file scanning changes.

Profile integration: Added to Sonar_way_profile.json, so it's active in the default quality gate.

Generate Walkthrough
Generate Diagram

🗣️ Give feedback

tomasz-tylenda-sonarsource · 2026-05-08T13:18:59Z

+
+  @Override
+  protected void onMethodInvocationFound(MethodInvocationTree mit) {
+    reportIssue(mit, "Replace this use of the system clock with a fixed clock.");


Let's keep the message generic, so it also covers reading the clock to measure elapsed time..

sonarqube-next · 2026-05-08T18:31:00Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

tomasz-tylenda-sonarsource · 2026-05-18T13:57:28Z

+  }
+
+  @Test
+  void testSystemClockInstants() {


Instant.ofEpochMilli(System.currentTimeMillis()); is compliant. Should this rule catch it too? (There is also System.nanoTime())

Yes, I think that in the first example, we should probably raise an issue on System.currentTimeMillis() (rather than Instant.ofEpochMilli(), since it doesn't have anything to do with the system clock). System.nanoTime() doesn't read the system clock and is very commonly used to measure elapsed time, so I don't think we should raise on it either.

Even if System.nanoTime doesn't read the wall-clock system time, it still uses the hardware to measure time (which still makes the tests non deterministic), so I think it could be targeted by the rule too

Yes, however my issue with including System.nanoTime is that it is often one of the recommended ways to measure elapsed time between two points in tests. It is not a problem to read System.nanoTime in a test because it does not depend on the system clock (which could change between runs or even during a run). As specified in the doc: "This method can only be used to measure elapsed time and is not related to any other notion of system or wall-clock time".

Ok, you're right, it seems to be out of scope for this rule

sonarqubecloud · 2026-05-19T07:14:02Z

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

15 issue(s) found across 1 file(s):

Rule	File	Line	Message
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	34	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	34	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	36	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	36	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	43	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	43	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	45	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	45	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	52	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	52	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	54	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	54	Move this trailing comment on the previous empty line.
`java:S125`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	60	This block of commented-out lines of code should be removed.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	60	Move this trailing comment on the previous empty line.
`java:S139`	`java-checks-test-sources/default/src/test/java/checks/tests/SystemClockCheckSample.java`	62	Move this trailing comment on the previous empty line.

_{Analyzed by SonarQube Agentic Analysis in 6.1 s}

…ssue

NoemieBenard · 2026-05-19T12:02:01Z

+    "datetime",
+    "tests"
+  ],
+  "defaultSeverity": "Major",


We should probably update the RSPEC because the default severity doesn't match the highest impact severity

sonar-review-alpha

The highlighting inconsistency flagged in the previous review round is still open — no other new issues found.

🗣️ Give feedback

sonarqube-next · 2026-05-19T12:50:07Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

tomasz-tylenda-sonarsource · 2026-05-19T14:36:58Z

+    long currentTimeMillis = System.currentTimeMillis(); // Noncompliant {{Do not use the system clock in tests.}}
+//                           ^^^^^^^^^^^^^^^^^^^^^^^^^^
+    long currentTimeNanos = System.nanoTime(); // Compliant: nanoTime is typically used to measure elapsed time in tests
+    assertTrue(currentTimeMillis < currentTimeNanos);


Here is an idea for another rule: if you use a suffix such as millis or nanos, don't mix them up :) It may not be straightfoward to implement though.

tomasz-tylenda-sonarsource · 2026-05-19T14:41:37Z

+      .build(),
+    MethodMatchers.create()
+      .ofTypes("java.lang.System")
+      .names("currentTimeMillis")


It just occured to me that new java.util.Date() is another way to read the system clock and I think it is used frequently. I know that is class should not be used, and we probably have a rule for that, but this is an orthogonal concern - someone may allow using java.util.Date in their code (compatibility with 3rd party code is a good reason to do so) and still want to have an issue about reading system clock in tests.

You're right, I'll add calls to the Date constructors to the nodes we should raise the issue on.

tomasz-tylenda-sonarsource · 2026-05-19T14:43:28Z

I assume you reviewed all these sources.

Yes, these are all occurrences of System.currentTimeMillis in tests !

aurelien-coet-sonarsource force-pushed the ac/SONARJAVA-6330 branch from a264a6e to add8e7f Compare May 8, 2026 06:47

aurelien-coet-sonarsource marked this pull request as ready for review May 8, 2026 13:05

aurelien-coet-sonarsource requested review from NoemieBenard and tomasz-tylenda-sonarsource May 8, 2026 13:05

tomasz-tylenda-sonarsource reviewed May 8, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

aurelien-coet-sonarsource requested a review from tomasz-tylenda-sonarsource May 13, 2026 09:20

tomasz-tylenda-sonarsource reviewed May 18, 2026

View reviewed changes

aurelien-coet-sonarsource temporarily deployed to dev May 19, 2026 07:13 — with GitHub Actions Inactive

This comment was marked as outdated.

Sign in to view

aurelien-coet-sonarsource added 5 commits May 19, 2026 13:46

Implement rule S8692

2a6f79f

Fix issues in QG

34e5c6e

Apply review comments and do not raise on .now(Clock)

94204e9

Add System.currentTimeMillis to the list of methods that trigger an i…

a62e055

…ssue

Apply review bot comment and fix ruling tests

acd3409

aurelien-coet-sonarsource force-pushed the ac/SONARJAVA-6330 branch from a5810bd to acd3409 Compare May 19, 2026 11:53

aurelien-coet-sonarsource temporarily deployed to dev May 19, 2026 11:53 — with GitHub Actions Inactive

This comment was marked as resolved.

Sign in to view

NoemieBenard reviewed May 19, 2026

View reviewed changes

Fix ruling tests and add test sample for System.currentTimeMillis

8e47a3c

aurelien-coet-sonarsource temporarily deployed to dev May 19, 2026 12:06 — with GitHub Actions Inactive

This comment was marked as outdated.

Sign in to view

Try to fix ruling and autoscan tests

c9ed396

aurelien-coet-sonarsource temporarily deployed to dev May 19, 2026 12:42 — with GitHub Actions Inactive

sonar-review-alpha Bot reviewed May 19, 2026

View reviewed changes

aurelien-coet-sonarsource requested review from NoemieBenard and tomasz-tylenda-sonarsource May 19, 2026 13:49

tomasz-tylenda-sonarsource reviewed May 19, 2026

View reviewed changes

Conversation

aurelien-coet-sonarsource commented May 8, 2026

Uh oh!

sonar-review-alpha Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What reviewers should know

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

sonarqube-next Bot commented May 8, 2026

Quality Gate passed

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Agentic Analysis: Early Results

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqube-next Bot commented May 19, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sonar-review-alpha Bot commented May 8, 2026 •

edited

Loading

sonarqubecloud Bot commented May 19, 2026 •

edited

Loading