Skip to content

Commit

Permalink
Have TimeZoneUsage check also flag illegal method references (#1507)
Browse files Browse the repository at this point in the history
While there, reorder a few things.
  • Loading branch information
Stephan202 authored Jan 11, 2025
1 parent a42353b commit 3e4a01a
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MemberReferenceTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MethodInvocationTree;
import java.time.Clock;
import java.time.Instant;
Expand All @@ -41,7 +43,8 @@
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class TimeZoneUsage extends BugChecker implements MethodInvocationTreeMatcher {
public final class TimeZoneUsage extends BugChecker
implements MethodInvocationTreeMatcher, MemberReferenceTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> BANNED_TIME_METHOD =
anyOf(
Expand All @@ -59,6 +62,10 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
"tickMillis",
"tickMinutes",
"tickSeconds"),
staticMethod()
.onClassAny(Instant.class.getCanonicalName())
.named("now")
.withNoParameters(),
staticMethod()
.onClassAny(
LocalDate.class.getCanonicalName(),
Expand All @@ -67,17 +74,22 @@ public final class TimeZoneUsage extends BugChecker implements MethodInvocationT
OffsetDateTime.class.getCanonicalName(),
OffsetTime.class.getCanonicalName(),
ZonedDateTime.class.getCanonicalName())
.named("now"),
staticMethod()
.onClassAny(Instant.class.getCanonicalName())
.named("now")
.withNoParameters());
.named("now"));

/** Instantiates a new {@link TimeZoneUsage} instance. */
public TimeZoneUsage() {}

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return getDescription(tree, state);
}

@Override
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
return getDescription(tree, state);
}

private Description getDescription(ExpressionTree tree, VisitorState state) {
return BANNED_TIME_METHOD.matches(tree, state)
? buildDescription(tree).build()
: Description.NO_MATCH;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ void identification() {
"import java.time.OffsetTime;",
"import java.time.ZoneId;",
"import java.time.ZonedDateTime;",
"import reactor.core.publisher.Mono;",
"",
"class A {",
" void m() {",
Expand All @@ -31,68 +32,122 @@ void identification() {
" Clock.tick(clock, Duration.ZERO);",
"",
" // BUG: Diagnostic contains:",
" Clock.systemUTC();",
" clock.getZone();",
" // BUG: Diagnostic contains:",
" Clock.systemDefaultZone();",
" Mono.fromSupplier(clock::getZone);",
" // BUG: Diagnostic contains:",
" clock.withZone(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(clock::withZone);",
" // BUG: Diagnostic contains:",
" Clock.system(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(Clock::system);",
" // BUG: Diagnostic contains:",
" Clock.systemDefaultZone();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(Clock::systemDefaultZone);",
" // BUG: Diagnostic contains:",
" Clock.systemUTC();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(Clock::systemUTC);",
" // BUG: Diagnostic contains:",
" Clock.tickMillis(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(Clock::tickMillis);",
" // BUG: Diagnostic contains:",
" Clock.tickMinutes(UTC);",
" // BUG: Diagnostic contains:",
" Clock.tickSeconds(UTC);",
" Mono.<ZoneId>empty().map(Clock::tickMinutes);",
" // BUG: Diagnostic contains:",
" clock.getZone();",
" Clock.tickSeconds(UTC);",
" // BUG: Diagnostic contains:",
" clock.withZone(UTC);",
" Mono.<ZoneId>empty().map(Clock::tickSeconds);",
"",
" Instant.now(clock);",
" Mono.<Clock>empty().map(Instant::now);",
" // BUG: Diagnostic contains:",
" Instant.now();",
" // This is equivalent to `clock.instant()`, which is fine.",
" Instant.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(Instant::now);",
"",
" // BUG: Diagnostic contains:",
" LocalDate.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(LocalDate::now);",
" // BUG: Diagnostic contains:",
" LocalDate.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(LocalDate::now);",
" // BUG: Diagnostic contains:",
" LocalDate.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(LocalDate::now);",
"",
" // BUG: Diagnostic contains:",
" LocalDateTime.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(LocalDateTime::now);",
" // BUG: Diagnostic contains:",
" LocalDateTime.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(LocalDateTime::now);",
" // BUG: Diagnostic contains:",
" LocalDateTime.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(LocalDateTime::now);",
"",
" // BUG: Diagnostic contains:",
" LocalTime.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(LocalTime::now);",
" // BUG: Diagnostic contains:",
" LocalTime.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(LocalTime::now);",
" // BUG: Diagnostic contains:",
" LocalTime.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(LocalTime::now);",
"",
" // BUG: Diagnostic contains:",
" OffsetDateTime.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(OffsetDateTime::now);",
" // BUG: Diagnostic contains:",
" OffsetDateTime.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(OffsetDateTime::now);",
" // BUG: Diagnostic contains:",
" OffsetDateTime.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(OffsetDateTime::now);",
"",
" // BUG: Diagnostic contains:",
" OffsetTime.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(OffsetTime::now);",
" // BUG: Diagnostic contains:",
" OffsetTime.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(OffsetTime::now);",
" // BUG: Diagnostic contains:",
" OffsetTime.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(OffsetTime::now);",
"",
" // BUG: Diagnostic contains:",
" ZonedDateTime.now();",
" // BUG: Diagnostic contains:",
" Mono.fromSupplier(ZonedDateTime::now);",
" // BUG: Diagnostic contains:",
" ZonedDateTime.now(clock);",
" // BUG: Diagnostic contains:",
" Mono.<Clock>empty().map(ZonedDateTime::now);",
" // BUG: Diagnostic contains:",
" ZonedDateTime.now(UTC);",
" // BUG: Diagnostic contains:",
" Mono.<ZoneId>empty().map(ZonedDateTime::now);",
" }",
"",
" abstract class ForwardingClock extends Clock {",
Expand Down

0 comments on commit 3e4a01a

Please sign in to comment.