From c320ee7bbc2f8c79e53ed1d85fbb682e78ff2422 Mon Sep 17 00:00:00 2001 From: Error Prone Team Date: Tue, 15 Oct 2024 15:10:39 -0700 Subject: [PATCH] GetSeconds to ToSeconds error prone Per b/201676318, this converts `getSeconds` without corresponding `getNano` to `toSeconds. The bulk of the codebase (java) has been migrated. Only a warning for now (though I supposed GoodTime will make it an error). Will suppress in 3P on a need-to-basis. PiperOrigin-RevId: 686254153 --- .../JavaDurationGetSecondsToToSeconds.java | 61 +++++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + ...JavaDurationGetSecondsToToSecondsTest.java | 86 +++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSeconds.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSecondsTest.java diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSeconds.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSeconds.java new file mode 100644 index 000000000000..22b8207b8f11 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSeconds.java @@ -0,0 +1,61 @@ +/* + * Copyright 2018 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.errorprone.bugpatterns.time; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.bugpatterns.time.NearbyCallers.containsCallToSameReceiverNearby; +import static com.google.errorprone.matchers.Matchers.allOf; +import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.fixes.SuggestedFixes; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; + +/** Error prone checking for {@code Duration.getSeconds()} without {@code Duration.getNano()}. */ +@BugPattern( + summary = "Prefer duration.toSeconds() over duration.getSeconds()", + explanation = + "duration.getSeconds() is a decomposition API which should always be used alongside" + + " duration.getNano(). duration.toSeconds() is a conversion API, and the preferred way" + + " to convert to seconds.", + severity = WARNING) +public final class JavaDurationGetSecondsToToSeconds extends BugChecker + implements MethodInvocationTreeMatcher { + + private static final Matcher GET_SECONDS = + instanceMethod().onExactClass("java.time.Duration").named("getSeconds"); + private static final Matcher GET_NANO = + allOf( + instanceMethod().onExactClass("java.time.Duration").named("getNano"), + Matchers.not(Matchers.packageStartsWith("java."))); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (GET_SECONDS.matches(tree, state)) { + if (!containsCallToSameReceiverNearby(tree, GET_NANO, state, /* checkProtoChains= */ false)) { + return describeMatch(tree, SuggestedFixes.renameMethodInvocation(tree, "toSeconds", state)); + } + } + return Description.NO_MATCH; + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 9806e788dd4d..a01309f8afcb 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -586,6 +586,7 @@ import com.google.errorprone.bugpatterns.time.InstantTemporalUnit; import com.google.errorprone.bugpatterns.time.InvalidJavaTimeConstant; import com.google.errorprone.bugpatterns.time.JavaDurationGetSecondsGetNano; +import com.google.errorprone.bugpatterns.time.JavaDurationGetSecondsToToSeconds; import com.google.errorprone.bugpatterns.time.JavaDurationWithNanos; import com.google.errorprone.bugpatterns.time.JavaDurationWithSeconds; import com.google.errorprone.bugpatterns.time.JavaInstantGetSecondsGetNano; @@ -970,6 +971,7 @@ public static ScannerSupplier warningChecks() { JUnitAmbiguousTestClass.class, JUnitIncompatibleType.class, JavaDurationGetSecondsGetNano.class, + JavaDurationGetSecondsToToSeconds.class, JavaDurationWithNanos.class, JavaDurationWithSeconds.class, JavaInstantGetSecondsGetNano.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSecondsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSecondsTest.java new file mode 100644 index 000000000000..5d6e10c8d79f --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/time/JavaDurationGetSecondsToToSecondsTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2018 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.errorprone.bugpatterns.time; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link JavaDurationGetSecondsToToSeconds}. */ +@RunWith(JUnit4.class) +public class JavaDurationGetSecondsToToSecondsTest { + + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(JavaDurationGetSecondsToToSeconds.class, getClass()); + + private final BugCheckerRefactoringTestHelper refactorHelper = + BugCheckerRefactoringTestHelper.newInstance( + JavaDurationGetSecondsToToSeconds.class, getClass()); + + @Test + public void testNegative() { + compilationHelper + .addSourceLines( + "test/TestCase.java", + """ + package test; + + import java.time.Duration; + + public class TestCase { + public static void foo(Duration duration) { + long seconds = duration.getSeconds(); + int nanos = duration.getNano(); + } + } + """) + .doTest(); + } + + @Test + public void testPositive() { + refactorHelper + .addInputLines( + "test/TestCase.java", + """ + package test; + + import java.time.Duration; + + public class TestCase { + public static void foo(Duration duration) { + long seconds = duration.getSeconds(); + } + } + """) + .addOutputLines( + "out/TestCase.java", + """ + package test; + + import java.time.Duration; + + public class TestCase { + public static void foo(Duration duration) { + long seconds = duration.toSeconds(); + } + } + """) + .doTest(); + } +}