-
Notifications
You must be signed in to change notification settings - Fork 724
SONARJAVA-6304: Implement S8218: Instant APIs should only use supported temporal units #6039
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
143 changes: 143 additions & 0 deletions
143
...est-sources/default/src/main/java/checks/UnsupportedChronoUnitWithInstantCheckSample.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| package checks; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.time.ZonedDateTime; | ||
| import java.time.temporal.ChronoUnit; | ||
| import java.time.temporal.Temporal; | ||
| import java.time.temporal.TemporalUnit; | ||
|
|
||
| import static java.time.temporal.ChronoUnit.YEARS; | ||
| import static checks.UnsupportedChronoUnitWithInstantCheckSample.CustomUnit.MONTHS; | ||
|
|
||
| class UnsupportedChronoUnitWithInstantCheckSample { | ||
|
|
||
| private static final TemporalUnit STORED_UNIT = ChronoUnit.MONTHS; | ||
|
|
||
| void unsupported(Instant instant, Instant end) { | ||
| instant.plus(1, ChronoUnit.WEEKS); // Noncompliant {{"WEEKS" is unsupported by Instant and causes an UnsupportedTemporalTypeException.}} | ||
| // ^^^^^^^^^^^^^^^^ | ||
| instant.plus(1, ChronoUnit.MONTHS); // Noncompliant {{"MONTHS" is unsupported by Instant and causes an UnsupportedTemporalTypeException.}} | ||
| // ^^^^^^^^^^^^^^^^^ | ||
| instant.plus(1, YEARS); // Noncompliant {{"YEARS" is unsupported by Instant and causes an UnsupportedTemporalTypeException.}} | ||
| // ^^^^^ | ||
| instant.plus(1, ChronoUnit.DECADES); // Noncompliant | ||
| instant.plus(1, ChronoUnit.CENTURIES); // Noncompliant | ||
| instant.plus(1, java.time.temporal.ChronoUnit.MILLENNIA); // Noncompliant | ||
| instant.plus(1, ChronoUnit.ERAS); // Noncompliant | ||
| instant.plus(1, ChronoUnit.FOREVER); // Noncompliant | ||
|
|
||
| instant.minus(1, ChronoUnit.WEEKS); // Noncompliant | ||
| instant.minus(1, ChronoUnit.MONTHS); // Noncompliant | ||
| instant.minus(1, ChronoUnit.YEARS); // Noncompliant | ||
| instant.minus(1, ChronoUnit.DECADES); // Noncompliant | ||
| instant.minus(1, ChronoUnit.CENTURIES); // Noncompliant | ||
| instant.minus(1, ChronoUnit.MILLENNIA); // Noncompliant | ||
| instant.minus(1, ChronoUnit.ERAS); // Noncompliant | ||
| instant.minus(1, ChronoUnit.FOREVER); // Noncompliant | ||
|
|
||
| instant.until(end, ChronoUnit.WEEKS); // Noncompliant | ||
| instant.until(end, (ChronoUnit.MONTHS)); // Noncompliant | ||
| // ^^^^^^^^^^^^^^^^^^^ | ||
| instant.until(end, ChronoUnit.YEARS); // Noncompliant | ||
| instant.until(end, ChronoUnit.DECADES); // Noncompliant | ||
| instant.until(end, ChronoUnit.CENTURIES); // Noncompliant | ||
| instant.until(end, ChronoUnit.MILLENNIA); // Noncompliant | ||
| instant.until(end, ChronoUnit.ERAS); // Noncompliant | ||
| instant.until(end, ChronoUnit.FOREVER); // Noncompliant | ||
| } | ||
|
|
||
| void supported(Instant instant, Instant end) { | ||
| instant.plus(1, ChronoUnit.NANOS); | ||
| instant.plus(1, ChronoUnit.MICROS); | ||
| instant.plus(1, ChronoUnit.MILLIS); | ||
| instant.plus(1, ChronoUnit.SECONDS); | ||
| instant.plus(1, ChronoUnit.MINUTES); | ||
| instant.plus(1, ChronoUnit.HOURS); | ||
| instant.plus(1, ChronoUnit.HALF_DAYS); | ||
| instant.plus(1, ChronoUnit.DAYS); | ||
|
|
||
| instant.minus(1, ChronoUnit.NANOS); | ||
| instant.minus(1, ChronoUnit.MICROS); | ||
| instant.minus(1, ChronoUnit.MILLIS); | ||
| instant.minus(1, ChronoUnit.SECONDS); | ||
| instant.minus(1, ChronoUnit.MINUTES); | ||
| instant.minus(1, ChronoUnit.HOURS); | ||
| instant.minus(1, ChronoUnit.HALF_DAYS); | ||
| instant.minus(1, ChronoUnit.DAYS); | ||
|
|
||
| instant.until(end, ChronoUnit.NANOS); | ||
| instant.until(end, ChronoUnit.MICROS); | ||
| instant.until(end, ChronoUnit.MILLIS); | ||
| instant.until(end, ChronoUnit.SECONDS); | ||
| instant.until(end, ChronoUnit.MINUTES); | ||
| instant.until(end, ChronoUnit.HOURS); | ||
| instant.until(end, ChronoUnit.HALF_DAYS); | ||
| instant.until(end, ChronoUnit.DAYS); | ||
| } | ||
|
|
||
| void excludedApis(Instant instant) { | ||
| instant.plus(Duration.ofDays(1)); | ||
| instant.minus(Duration.ofDays(1)); | ||
| instant.truncatedTo(ChronoUnit.WEEKS); | ||
| } | ||
|
|
||
| void otherReceiver(ZonedDateTime dateTime, ZonedDateTime end) { | ||
| dateTime.plus(1, ChronoUnit.MONTHS); | ||
| dateTime.minus(1, ChronoUnit.YEARS); | ||
| dateTime.until(end, ChronoUnit.WEEKS); | ||
| } | ||
|
|
||
| void indirectAndDynamic(Instant instant, Instant end, TemporalUnit unit, boolean condition) { | ||
| TemporalUnit localUnit = ChronoUnit.YEARS; | ||
| instant.plus(1, localUnit); | ||
| instant.minus(1, STORED_UNIT); | ||
| instant.until(end, unit); | ||
| instant.plus(1, condition ? ChronoUnit.MONTHS : ChronoUnit.DAYS); | ||
| } | ||
|
|
||
| void customUnits(Instant instant, Instant end) { | ||
| instant.plus(1, CustomUnit.MONTHS); | ||
| instant.minus(1, MONTHS); | ||
| instant.until(end, CustomUnit.MONTHS); | ||
| } | ||
|
|
||
| enum CustomUnit implements TemporalUnit { | ||
| MONTHS; | ||
|
|
||
| @Override | ||
| public Duration getDuration() { | ||
| return Duration.ZERO; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isDurationEstimated() { | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isDateBased() { | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isTimeBased() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isSupportedBy(Temporal temporal) { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public <R extends Temporal> R addTo(R temporal, long amount) { | ||
| return temporal; | ||
| } | ||
|
|
||
| @Override | ||
| public long between(Temporal first, Temporal second) { | ||
| return 0; | ||
| } | ||
| } | ||
| } |
103 changes: 103 additions & 0 deletions
103
java-checks/src/main/java/org/sonar/java/checks/UnsupportedChronoUnitWithInstantCheck.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package org.sonar.java.checks; | ||
|
|
||
| import java.util.Set; | ||
| import javax.annotation.Nullable; | ||
| import org.sonar.check.Rule; | ||
| import org.sonar.java.checks.methods.AbstractMethodDetection; | ||
| import org.sonar.java.model.ExpressionUtils; | ||
| import org.sonar.plugins.java.api.JavaVersion; | ||
| import org.sonar.plugins.java.api.JavaVersionAwareVisitor; | ||
| import org.sonar.plugins.java.api.semantic.MethodMatchers; | ||
| import org.sonar.plugins.java.api.semantic.Symbol; | ||
| import org.sonar.plugins.java.api.tree.ExpressionTree; | ||
| import org.sonar.plugins.java.api.tree.IdentifierTree; | ||
| import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; | ||
| import org.sonar.plugins.java.api.tree.MethodInvocationTree; | ||
|
|
||
| @Rule(key = "S8218") | ||
| public class UnsupportedChronoUnitWithInstantCheck extends AbstractMethodDetection implements JavaVersionAwareVisitor { | ||
|
|
||
| private static final String INSTANT = "java.time.Instant"; | ||
| private static final String TEMPORAL = "java.time.temporal.Temporal"; | ||
| private static final String TEMPORAL_UNIT = "java.time.temporal.TemporalUnit"; | ||
| private static final String CHRONO_UNIT = "java.time.temporal.ChronoUnit"; | ||
|
|
||
| private static final Set<String> UNSUPPORTED_UNITS = Set.of( | ||
| "WEEKS", | ||
| "MONTHS", | ||
| "YEARS", | ||
| "DECADES", | ||
| "CENTURIES", | ||
| "MILLENNIA", | ||
| "ERAS", | ||
| "FOREVER" | ||
| ); | ||
|
|
||
| private static final MethodMatchers MATCHERS = MethodMatchers.create() | ||
| .ofTypes(INSTANT) | ||
| .names("plus", "minus") | ||
| .addParametersMatcher("long", TEMPORAL_UNIT) | ||
| .build(); | ||
|
|
||
| private static final MethodMatchers UNTIL_MATCHER = MethodMatchers.create() | ||
| .ofTypes(INSTANT) | ||
| .names("until") | ||
| .addParametersMatcher(TEMPORAL, TEMPORAL_UNIT) | ||
| .build(); | ||
|
|
||
| @Override | ||
| public boolean isCompatibleWithJavaVersion(JavaVersion version) { | ||
| return version.isJava8Compatible(); | ||
| } | ||
|
|
||
| @Override | ||
| protected MethodMatchers getMethodInvocationMatchers() { | ||
| return MethodMatchers.or(MATCHERS, UNTIL_MATCHER); | ||
| } | ||
|
|
||
| @Override | ||
| protected void onMethodInvocationFound(MethodInvocationTree mit) { | ||
| if (context.getSemanticModel() == null) { | ||
| return; | ||
| } | ||
| ExpressionTree argument = mit.arguments().get(1); | ||
| Symbol symbol = referencedSymbol(ExpressionUtils.skipParentheses(argument)); | ||
| if (isChronoUnitConstant(symbol) && UNSUPPORTED_UNITS.contains(symbol.name())) { | ||
| reportIssue(argument, String.format("\"%s\" is unsupported by Instant and causes an UnsupportedTemporalTypeException.", symbol.name())); | ||
| } | ||
| } | ||
|
|
||
| private static @Nullable Symbol referencedSymbol(ExpressionTree argument) { | ||
| if (argument instanceof IdentifierTree identifier) { | ||
| return identifier.symbol(); | ||
| } | ||
| if (argument instanceof MemberSelectExpressionTree memberSelect) { | ||
| return memberSelect.identifier().symbol(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private static boolean isChronoUnitConstant(@Nullable Symbol symbol) { | ||
| if (symbol == null || symbol.isUnknown() || !symbol.isVariableSymbol() || !symbol.isEnum()) { | ||
| return false; | ||
| } | ||
| Symbol owner = symbol.owner(); | ||
| return owner != null && !owner.isUnknown() && owner.type().is(CHRONO_UNIT); | ||
| } | ||
| } | ||
53 changes: 53 additions & 0 deletions
53
...checks/src/test/java/org/sonar/java/checks/UnsupportedChronoUnitWithInstantCheckTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /* | ||
| * SonarQube Java | ||
| * Copyright (C) SonarSource Sàrl | ||
| * mailto:info AT sonarsource DOT com | ||
| * | ||
| * You can redistribute and/or modify this program under the terms of | ||
| * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the Sonar Source-Available License for more details. | ||
| * | ||
| * You should have received a copy of the Sonar Source-Available License | ||
| * along with this program; if not, see https://sonarsource.com/license/ssal/ | ||
| */ | ||
| package org.sonar.java.checks; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.sonar.java.checks.verifier.CheckVerifier; | ||
|
|
||
| import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; | ||
|
|
||
| class UnsupportedChronoUnitWithInstantCheckTest { | ||
|
|
||
| private static final String SAMPLE = "checks/UnsupportedChronoUnitWithInstantCheckSample.java"; | ||
|
|
||
| @Test | ||
| void test() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile(mainCodeSourcesPath(SAMPLE)) | ||
| .withCheck(new UnsupportedChronoUnitWithInstantCheck()) | ||
| .verifyIssues(); | ||
| } | ||
|
|
||
| @Test | ||
| void test_without_semantic() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile(mainCodeSourcesPath(SAMPLE)) | ||
| .withCheck(new UnsupportedChronoUnitWithInstantCheck()) | ||
| .withoutSemantic() | ||
| .verifyNoIssues(); | ||
| } | ||
|
|
||
| @Test | ||
| void no_issue_before_java_8() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile(mainCodeSourcesPath(SAMPLE)) | ||
| .withCheck(new UnsupportedChronoUnitWithInstantCheck()) | ||
| .withJavaVersion(7) | ||
| .verifyNoIssues(); | ||
| } | ||
| } |
85 changes: 85 additions & 0 deletions
85
sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8218.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| <p><code>Instant</code> supports only time-based <code>ChronoUnit</code> values and <code>ChronoUnit.DAYS</code>. Passing another | ||
| <code>ChronoUnit</code> value to its arithmetic methods causes an <code>UnsupportedTemporalTypeException</code> when the unit is evaluated.</p> | ||
| <h2>Why is this an issue?</h2> | ||
| <p>An <code>Instant</code> represents a point on the timeline as a number of seconds and nanoseconds from the epoch. It does not define how date-based | ||
| <code>ChronoUnit</code> values such as <code>MONTHS</code> or <code>YEARS</code> apply to its timeline. Custom <code>TemporalUnit</code> | ||
| implementations may define their own behavior.</p> | ||
| <p>The <code>plus(long, TemporalUnit)</code>, <code>minus(long, TemporalUnit)</code>, and <code>until(Temporal, TemporalUnit)</code> methods support | ||
| the following <code>ChronoUnit</code> values:</p> | ||
| <ul> | ||
| <li><code>NANOS</code></li> | ||
| <li><code>MICROS</code></li> | ||
| <li><code>MILLIS</code></li> | ||
| <li><code>SECONDS</code></li> | ||
| <li><code>MINUTES</code></li> | ||
| <li><code>HOURS</code></li> | ||
| <li><code>HALF_DAYS</code></li> | ||
| <li><code>DAYS</code></li> | ||
| </ul> | ||
| <p>Using <code>WEEKS</code>, <code>MONTHS</code>, <code>YEARS</code>, <code>DECADES</code>, <code>CENTURIES</code>, <code>MILLENNIA</code>, | ||
| <code>ERAS</code>, or <code>FOREVER</code> with these methods throws an <code>UnsupportedTemporalTypeException</code> when the unit is evaluated. The | ||
| operation cannot produce a result, which can interrupt date and time processing in the application.</p> | ||
| <h3>Exceptions</h3> | ||
| <p>This rule reports only direct <code>ChronoUnit</code> constants. It does not track a constant stored in a variable.</p> | ||
| <p>Custom <code>TemporalUnit</code> implementations are not reported because <code>Instant</code> delegates the operation to them. The | ||
| <code>plus(TemporalAmount)</code> and <code>minus(TemporalAmount)</code> overloads and the <code>truncatedTo(TemporalUnit)</code> method are also | ||
| outside the scope of this rule.</p> | ||
| <h2>How to fix it</h2> | ||
| <p>Use a supported <code>ChronoUnit</code> for a fixed amount of elapsed time. When an operation must follow calendar rules, convert the | ||
| <code>Instant</code> to a calendar type such as <code>ZonedDateTime</code> using the intended <code>ZoneId</code>. A custom <code>TemporalUnit</code> | ||
| is also valid when its implementation explicitly defines the required timeline behavior.</p> | ||
| <h3>Code examples</h3> | ||
| <h4>Noncompliant code example</h4> | ||
| <pre data-diff-id="1" data-diff-type="noncompliant"> | ||
| import static java.time.temporal.ChronoUnit.YEARS; | ||
|
|
||
| void update(Instant instant, Instant end) { | ||
| instant.plus( | ||
| 1, | ||
| ChronoUnit.MONTHS); // Noncompliant: "MONTHS" is unsupported by Instant and causes an UnsupportedTemporalTypeException. | ||
|
|
||
| instant.minus( | ||
| 1, | ||
| YEARS); // Noncompliant: "YEARS" is unsupported by Instant and causes an UnsupportedTemporalTypeException. | ||
|
|
||
| instant.until( | ||
| end, | ||
| ChronoUnit.WEEKS); // Noncompliant: "WEEKS" is unsupported by Instant and causes an UnsupportedTemporalTypeException. | ||
| } | ||
| </pre> | ||
| <h4>Compliant solution</h4> | ||
| <pre data-diff-id="1" data-diff-type="compliant"> | ||
| import static java.time.temporal.ChronoUnit.YEARS; | ||
|
|
||
| void update(Instant instant, Instant end, ZoneId zone, TemporalUnit customUnit) { | ||
| // Use supported units for elapsed-time operations. | ||
| instant.plus(1, ChronoUnit.DAYS); | ||
| instant.minus(12, ChronoUnit.HOURS); | ||
| instant.until(end, ChronoUnit.MINUTES); | ||
|
|
||
| // Use a calendar type and an explicit time zone for calendar arithmetic. | ||
| ZonedDateTime dateTime = instant.atZone(zone); | ||
| ZonedDateTime endDateTime = end.atZone(zone); | ||
| dateTime.plus(1, ChronoUnit.MONTHS); | ||
| dateTime.minus(1, YEARS); | ||
| dateTime.until(endDateTime, ChronoUnit.WEEKS); | ||
|
|
||
| // Support for custom units is defined by their implementation. | ||
| instant.plus(1, customUnit); | ||
| instant.minus(1, customUnit); | ||
| instant.until(end, customUnit); | ||
| } | ||
| </pre> | ||
| <h2>Resources</h2> | ||
| <h3>Documentation</h3> | ||
| <ul> | ||
| <li>Java Documentation - <a | ||
| href="https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/time/Instant.html#isSupported(java.time.temporal.TemporalUnit)"><code>Instant.isSupported(TemporalUnit)</code></a></li> | ||
| <li>Java Documentation - <a | ||
| href="https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/time/Instant.html#plus(long,java.time.temporal.TemporalUnit)"><code>Instant.plus(long, TemporalUnit)</code></a></li> | ||
| <li>Java Documentation - <a | ||
| href="https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/time/Instant.html#minus(long,java.time.temporal.TemporalUnit)"><code>Instant.minus(long, TemporalUnit)</code></a></li> | ||
| <li>Java Documentation - <a | ||
| href="https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/time/Instant.html#until(java.time.temporal.Temporal,java.time.temporal.TemporalUnit)"><code>Instant.until(Temporal, TemporalUnit)</code></a></li> | ||
| </ul> | ||
|
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.