From a311d24d28c1b6e73d7471da7eb4241bb3c6618a Mon Sep 17 00:00:00 2001 From: Darshan Vijay Date: Thu, 8 Aug 2024 11:12:54 -0400 Subject: [PATCH 1/4] toString on Optional should throw error --- .../baseline/errorprone/OptionalToString.java | 53 ++++++++++++++++ .../errorprone/OptionalToStringUsageTest.java | 61 +++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java new file mode 100644 index 000000000..55304f285 --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java @@ -0,0 +1,53 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import java.util.Optional; + +@AutoService(BugChecker.class) +@BugPattern( + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + linkType = BugPattern.LinkType.CUSTOM, + severity = SeverityLevel.ERROR, + summary = "Optional.toString() does not stringifies the value contained by the Optional" + + " object. Did you mean Optional.get().toString() instead?") +public final class OptionalToString extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { + + private static final Matcher OPTIONAL_TO_STRING_METHOD = MethodMatchers.instanceMethod() + .onExactClassAny(Optional.class.getName(), com.google.common.base.Optional.class.getName()) + .named("toString") + .withNoParameters(); + + @Override + public Description matchMethodInvocation(MethodInvocationTree methodInvocationTree, VisitorState visitorState) { + if (OPTIONAL_TO_STRING_METHOD.matches(methodInvocationTree, visitorState)) { + return describeMatch(methodInvocationTree); + } + + return Description.NO_MATCH; + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java new file mode 100644 index 000000000..129b53ce9 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java @@ -0,0 +1,61 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * 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.palantir.baseline.errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class OptionalToStringUsageTest { + + private CompilationTestHelper compilationHelper; + + @BeforeEach + public void before() { + compilationHelper = CompilationTestHelper.newInstance(OptionalToString.class, getClass()); + } + + @Test + public void should_throw_error_if_to_string_is_invoked_on_java_optional() { + compilationHelper + .addSourceLines( + "Test.java", + "import java.util.Optional;", + "class Test {", + " String f() {", + " // BUG: Diagnostic contains: Optional.toString() does not stringifies the value", + " return Optional.of(\"This is an optional value\").toString();", + " }", + "}") + .doTest(); + } + + @Test + public void should_throw_error_if_to_string_is_invoked_on_guava_optional() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.base.Optional;", + "class Test {", + " String f() {", + " // BUG: Diagnostic contains: Optional.toString() does not stringifies the value", + " return Optional.of(\"This is an optional value\").toString();", + " }", + "}") + .doTest(); + } +} From efa6cc77869bf65e2a911c294e0669855d048f21 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 8 Aug 2024 15:17:25 +0000 Subject: [PATCH 2/4] Add generated changelog entries --- changelog/@unreleased/pr-2833.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-2833.v2.yml diff --git a/changelog/@unreleased/pr-2833.v2.yml b/changelog/@unreleased/pr-2833.v2.yml new file mode 100644 index 000000000..974bd2581 --- /dev/null +++ b/changelog/@unreleased/pr-2833.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Optional.toString() should throw error + links: + - https://github.com/palantir/gradle-baseline/pull/2833 From a5328183d24ea0a8a31cbab4ced575d39ac35f3e Mon Sep 17 00:00:00 2001 From: Darshan Vijay Date: Thu, 8 Aug 2024 11:21:08 -0400 Subject: [PATCH 3/4] Add another test case --- .../errorprone/OptionalToStringUsageTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java index 129b53ce9..2030917a9 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/OptionalToStringUsageTest.java @@ -58,4 +58,18 @@ public void should_throw_error_if_to_string_is_invoked_on_guava_optional() { "}") .doTest(); } + + @Test + public void should_not_throw_error_if_to_string_is_invoked_on_optional_get() { + compilationHelper + .addSourceLines( + "Test.java", + "import com.google.common.base.Optional;", + "class Test {", + " String f() {", + " return Optional.of(\"This is an optional value\").get().toString();", + " }", + "}") + .doTest(); + } } From 247f1f007f68169ca2ffcaa20a660c13c06740ac Mon Sep 17 00:00:00 2001 From: Darshan Vijay Date: Thu, 8 Aug 2024 11:40:08 -0400 Subject: [PATCH 4/4] remove suggestion from warning + replace ERROR with WARNING --- .../com/palantir/baseline/errorprone/OptionalToString.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java index 55304f285..67127d02e 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/OptionalToString.java @@ -32,9 +32,8 @@ @BugPattern( link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", linkType = BugPattern.LinkType.CUSTOM, - severity = SeverityLevel.ERROR, - summary = "Optional.toString() does not stringifies the value contained by the Optional" - + " object. Did you mean Optional.get().toString() instead?") + severity = SeverityLevel.WARNING, + summary = "Optional.toString() does not stringifies the value contained by the Optional object.") public final class OptionalToString extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { private static final Matcher OPTIONAL_TO_STRING_METHOD = MethodMatchers.instanceMethod()