Skip to content

Commit

Permalink
PRP-10968 Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Stephan202
Stephan202 committed Feb 14, 2021
1 parent 852fd6a commit d8a721b
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 72 deletions.
19 changes: 10 additions & 9 deletions error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@
/** A {@link BugChecker} which flags illegal time-zone related operations. */
@AutoService(BugChecker.class)
@BugPattern(
name = "TimeZoneUsageCheck",
summary = "Avoid illegal operations on assorted time related objects",
name = "TimeZoneUsage",
summary =
"Derive the current time from a `Clock` Spring bean, and don't rely on a `Clock`'s time zone",
linkType = LinkType.NONE,
severity = SeverityLevel.WARNING,
tags = StandardTags.FRAGILE_CODE)
public final class TimeZoneUsageCheck extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> ILLEGAL_CLOCK_USAGES =
private static final Matcher<ExpressionTree> IS_BANNED_TIME_METHOD =
anyOf(
instanceMethod().onDescendantOf(Clock.class.getName()).namedAnyOf("getZone", "withZone"),
staticMethod()
Expand All @@ -43,24 +44,24 @@ public final class TimeZoneUsageCheck extends BugChecker implements MethodInvoca
"systemUTC",
"tickMillis",
"tickMinutes",
"tickSeconds"));
private static final Matcher<ExpressionTree> NOW_USAGES =
anyOf(
"tickSeconds"),
staticMethod()
.onClassAny(
Instant.class.getName(),
LocalDate.class.getName(),
LocalDateTime.class.getName(),
LocalTime.class.getName())
.namedAnyOf("now"));
.named("now"));

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!ILLEGAL_CLOCK_USAGES.matches(tree, state) && !NOW_USAGES.matches(tree, state)) {
if (!IS_BANNED_TIME_METHOD.matches(tree, state)) {
return Description.NO_MATCH;
}

return buildDescription(tree)
.setMessage("This operation is not recommended, please refactor it")
.setMessage(
"Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone")
.build();
}
}
118 changes: 55 additions & 63 deletions ...rone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/TimeZoneUsageCheckTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.common.base.Predicates;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;

public final class TimeZoneUsageCheckTest {
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(TimeZoneUsageCheck.class, getClass());
CompilationTestHelper.newInstance(TimeZoneUsageCheck.class, getClass())
.expectErrorMessage(
"X",
Predicates.containsPattern(
"Derive the current time from an existing `Clock` Spring bean, and don't rely on a `Clock`'s time zone"));

@Test
public void testNoIdentificationFoundCases() {
public void testIdentification() {
compilationHelper
.addSourceLines(
"A.java",
Expand All @@ -17,73 +22,60 @@ public void testNoIdentificationFoundCases() {
"import java.time.Clock;",
"import java.time.Duration;",
"import java.time.Instant;",
"",
"class A {",
"void m() {",
" Clock clock = Clock.fixed(Instant.EPOCH, UTC);",
"clock.instant();",
"clock.millis();",
"Clock.offset(clock, Duration.ZERO);",
"Clock.tick(clock, Duration.ZERO);",
" }",
"}")
.doTest();
}

@Test
public void testIdentifyCases() {
compilationHelper
.addSourceLines(
"A.java",
"import static java.time.ZoneOffset.UTC;",
"",
"import java.time.Clock;",
"import java.time.Instant;",
"import java.time.LocalDate;",
"import java.time.LocalDateTime;",
"import java.time.LocalTime;",
"",
"class A {",
"void m() {",
" // BUG: Diagnostic contains:",
"Clock.systemUTC();",
" // BUG: Diagnostic contains:",
"Clock.systemDefaultZone();",
" // BUG: Diagnostic contains:",
"Clock.system(UTC);",
" // BUG: Diagnostic contains:",
"Clock.tickMillis(UTC);",
" // BUG: Diagnostic contains:",
"Clock.tickMinutes(UTC);",
" // BUG: Diagnostic contains:",
"Clock.tickSeconds(UTC);",
" void m() {",
" Clock clock = Clock.fixed(Instant.EPOCH, UTC);",
" // BUG: Diagnostic contains:",
"clock.getZone();",
" // BUG: Diagnostic contains:",
"clock.withZone(UTC);",
" // BUG: Diagnostic contains:",
"Instant.now();",
" // BUG: Diagnostic contains:",
"Instant.now(clock);",
" // BUG: Diagnostic contains:",
"LocalDate.now();",
" // BUG: Diagnostic contains:",
"LocalDate.now(clock);",
" // BUG: Diagnostic contains:",
"LocalDate.now(UTC);",
" // BUG: Diagnostic contains:",
"LocalDateTime.now();",
" // BUG: Diagnostic contains:",
"LocalDateTime.now(clock);",
" // BUG: Diagnostic contains:",
"LocalDateTime.now(UTC);",
" // BUG: Diagnostic contains:",
"LocalTime.now();",
" // BUG: Diagnostic contains:",
"LocalTime.now(clock);",
" // BUG: Diagnostic contains:",
"LocalTime.now(UTC);",
" clock.instant();",
" clock.millis();",
" Clock.offset(clock, Duration.ZERO);",
" Clock.tick(clock, Duration.ZERO);",
"",
" // BUG: Diagnostic matches: X",
" Clock.systemUTC();",
" // BUG: Diagnostic matches: X",
" Clock.systemDefaultZone();",
" // BUG: Diagnostic matches: X",
" Clock.system(UTC);",
" // BUG: Diagnostic matches: X",
" Clock.tickMillis(UTC);",
" // BUG: Diagnostic matches: X",
" Clock.tickMinutes(UTC);",
" // BUG: Diagnostic matches: X",
" Clock.tickSeconds(UTC);",
" // BUG: Diagnostic matches: X",
" clock.getZone();",
" // BUG: Diagnostic matches: X",
" clock.withZone(UTC);",
"",
" // BUG: Diagnostic matches: X",
" Instant.now();",
" // BUG: Diagnostic matches: X",
" Instant.now(clock);",
"",
" // BUG: Diagnostic matches: X",
" LocalDate.now();",
" // BUG: Diagnostic matches: X",
" LocalDate.now(clock);",
" // BUG: Diagnostic matches: X",
" LocalDate.now(UTC);",
"",
" // BUG: Diagnostic matches: X",
" LocalDateTime.now();",
" // BUG: Diagnostic matches: X",
" LocalDateTime.now(clock);",
" // BUG: Diagnostic matches: X",
" LocalDateTime.now(UTC);",
"",
" // BUG: Diagnostic matches: X",
" LocalTime.now();",
" // BUG: Diagnostic matches: X",
" LocalTime.now(clock);",
" // BUG: Diagnostic matches: X",
" LocalTime.now(UTC);",
" }",
"}")
.doTest();
Expand Down

0 comments on commit d8a721b

Please sign in to comment.