diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java b/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java index c72b6c3ac81..6986bed107d 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ImpossibleNullComparison.java @@ -145,12 +145,14 @@ private static boolean isNull(ExpressionTree tree) { private final boolean matchTestAssertions; private final boolean checkPrimitives; + private final boolean checkValueOf; @Inject ImpossibleNullComparison(ErrorProneFlags flags) { this.matchTestAssertions = flags.getBoolean("ProtoFieldNullComparison:MatchTestAssertions").orElse(true); this.checkPrimitives = flags.getBoolean("ImmutableNullComparison:CheckPrimitives").orElse(true); + this.checkValueOf = flags.getBoolean("ImpossibleNullComparison:CheckValueOf").orElse(true); } @Override @@ -262,6 +264,7 @@ private Optional getFixer(ExpressionTree tree, VisitorState state) { } return stream(GetterTypes.values()) .filter(gt -> !gt.equals(GetterTypes.PRIMITIVE) || checkPrimitives) + .filter(gt -> !gt.equals(GetterTypes.VALUE_OF) || checkValueOf) .map(type -> type.match(resolvedTree, state)) .filter(Objects::nonNull) .findFirst(); @@ -311,6 +314,12 @@ private interface Fixer { private static final Matcher TABLE_COLUMN_MATCHER = instanceMethod().onDescendantOf("com.google.common.collect.Table").named("column"); + private static final Matcher NON_NULL_VALUE_OF = + staticMethod() + .onDescendantOfAny("java.lang.Enum", "java.lang.Number") + .named("valueOf") + .withParameters("java.lang.String"); + private enum GetterTypes { OPTIONAL_GET { @Nullable @@ -394,6 +403,17 @@ Fixer match(ExpressionTree tree, VisitorState state) { return type != null && type.isPrimitive() ? GetterTypes::emptyFix : null; } }, + VALUE_OF { + @Nullable + @Override + Fixer match(ExpressionTree tree, VisitorState state) { + if (!NON_NULL_VALUE_OF.matches(tree, state)) { + return null; + } + // TODO(cpovirk): Suggest Enums.getIfPresent, Ints.tryParse, etc. + return GetterTypes::emptyFix; + } + }, /** {@code proto.getFoo()} */ SCALAR { @Nullable diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ImpossibleNullComparisonTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ImpossibleNullComparisonTest.java index 61b418155d0..2e2e8017c03 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/ImpossibleNullComparisonTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/ImpossibleNullComparisonTest.java @@ -497,4 +497,22 @@ public void primitives() { "}") .doTest(); } + + @Test + public void valueOf() { + compilationHelper + .addSourceLines( + "Test.java", + "import static com.google.common.truth.Truth.assertThat;", + "import java.util.concurrent.TimeUnit;", + "public class Test {", + " public void o(String s) {", + " // BUG: Diagnostic contains:", + " assertThat(TimeUnit.valueOf(s)).isNotNull();", + " // BUG: Diagnostic contains:", + " assertThat(Integer.valueOf(s)).isNotNull();", + " }", + "}") + .doTest(); + } }