diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AttemptedNegativeZero.java b/core/src/main/java/com/google/errorprone/bugpatterns/AttemptedNegativeZero.java new file mode 100644 index 00000000000..b6f7aef32b3 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AttemptedNegativeZero.java @@ -0,0 +1,64 @@ +/* + * Copyright 2023 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; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.constValue; +import static com.google.errorprone.util.ASTHelpers.targetType; +import static com.sun.source.tree.Tree.Kind.UNARY_MINUS; +import static com.sun.tools.javac.code.TypeTag.DOUBLE; +import static com.sun.tools.javac.code.TypeTag.FLOAT; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.UnaryTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.UnaryTree; + +/** A BugPattern; see the summary. */ +@BugPattern( + severity = WARNING, + summary = "-0 is the same as 0. For the floating-point negative zero, use -0.0.") +public class AttemptedNegativeZero extends BugChecker implements UnaryTreeMatcher { + @Override + public Description matchUnary(UnaryTree tree, VisitorState state) { + if (tree.getKind() != UNARY_MINUS) { + return NO_MATCH; + } + Object negatedConstValue = constValue(tree.getExpression()); + if (negatedConstValue == null) { + return NO_MATCH; + } + if (!negatedConstValue.equals(0) && !negatedConstValue.equals(0L)) { + return NO_MATCH; + } + String replacement; + switch (targetType(state).type().getTag()) { + case DOUBLE: + replacement = "-0.0"; + break; + case FLOAT: + replacement = "-0.0f"; + break; + default: + return NO_MATCH; + } + return describeMatch(tree, SuggestedFix.builder().replace(tree, replacement).build()); + } +} 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 e87e78d43a2..04d1dcefef4 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -40,6 +40,7 @@ import com.google.errorprone.bugpatterns.AssertionFailureIgnored; import com.google.errorprone.bugpatterns.AsyncCallableReturnsNull; import com.google.errorprone.bugpatterns.AsyncFunctionReturnsNull; +import com.google.errorprone.bugpatterns.AttemptedNegativeZero; import com.google.errorprone.bugpatterns.AutoValueBuilderDefaultsInConstructor; import com.google.errorprone.bugpatterns.AutoValueFinalMethods; import com.google.errorprone.bugpatterns.AutoValueImmutableFields; @@ -823,6 +824,7 @@ public static ScannerSupplier warningChecks() { AssertThrowsMultipleStatements.class, AssertionFailureIgnored.class, AssistedInjectAndInjectOnSameConstructor.class, + AttemptedNegativeZero.class, AutoValueFinalMethods.class, AutoValueImmutableFields.class, AutoValueSubclassLeaked.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/AttemptedNegativeZeroTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/AttemptedNegativeZeroTest.java new file mode 100644 index 00000000000..4b9d2c149a7 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/AttemptedNegativeZeroTest.java @@ -0,0 +1,57 @@ +/* + * Copyright 2023 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; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** {@link AttemptedNegativeZero}Test. */ +@RunWith(JUnit4.class) +public final class AttemptedNegativeZeroTest { + private final CompilationTestHelper compilationHelper = + CompilationTestHelper.newInstance(AttemptedNegativeZero.class, getClass()); + + @Test + public void positive() { + compilationHelper + .addSourceLines( + "Foo.java", + "class Foo {", + " // BUG: Diagnostic contains: ", + " double x = -0;", + " // BUG: Diagnostic contains: ", + " double y = -0L;", // I didn't see the `long` case in the wild. + "}") + .doTest(); + } + + @Test + public void negative() { + compilationHelper + .addSourceLines( + "Foo.java", + "class Foo {", + " int w = -0;", // weird but not likely to be hiding any bugs + " double x = -0.0;", + " double y = 0;", + " double z = +0;", + "}") + .doTest(); + } +} diff --git a/docs/bugpattern/AttemptedNegativeZero.md b/docs/bugpattern/AttemptedNegativeZero.md new file mode 100644 index 00000000000..ab89251a3aa --- /dev/null +++ b/docs/bugpattern/AttemptedNegativeZero.md @@ -0,0 +1,9 @@ +Because `0` is an integer constant, `-0` is an integer, too. Integers have no +concept of "negative zero," so it is the same as plain `0`. + +The value is then widened to a floating-point number. And while floating-point +numbers have a concept of "negative zero," the integral `0` is widened to the +floating-point "positive" zero. + +To write a negative zero, you have to write a constant that is a floating-point +number. One simple way to do that is to write `-0.0`.