Skip to content

Commit

Permalink
Rename ProtoFieldNullComparison to something less intrinsically proto…
Browse files Browse the repository at this point in the history
…-ish, in preparation for generalizing it a bit.

PiperOrigin-RevId: 487541689
  • Loading branch information
graememorgan authored and Error Prone Team committed Nov 10, 2022
1 parent 4d5918f commit 5b56e10
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@
import javax.annotation.Nullable;

/** Matches comparison of proto fields to {@code null}. */
@BugPattern(summary = "Protobuf fields cannot be null.", severity = ERROR)
public class ProtoFieldNullComparison extends BugChecker implements CompilationUnitTreeMatcher {
@BugPattern(
summary = "Protobuf fields cannot be null.",
altNames = "ProtoFieldNullComparison",
severity = ERROR)
public final class ImpossibleNullComparison extends BugChecker
implements CompilationUnitTreeMatcher {

// TODO(b/111109484): Try to consolidate these with NullnessPropagationTransfer.
private static final Matcher<ExpressionTree> CHECK_NOT_NULL =
Expand Down Expand Up @@ -138,23 +142,23 @@ private static boolean isNull(ExpressionTree tree) {

private final boolean matchTestAssertions;

public ProtoFieldNullComparison(ErrorProneFlags flags) {
public ImpossibleNullComparison(ErrorProneFlags flags) {
this.matchTestAssertions =
flags.getBoolean("ProtoFieldNullComparison:MatchTestAssertions").orElse(true);
}

@Override
public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
ProtoNullComparisonScanner scanner = new ProtoNullComparisonScanner(state);
NullComparisonScanner scanner = new NullComparisonScanner(state);
scanner.scan(state.getPath(), null);
return Description.NO_MATCH;
}

private class ProtoNullComparisonScanner extends TreePathScanner<Void, Void> {
private class NullComparisonScanner extends TreePathScanner<Void, Void> {
private final Map<Symbol, ExpressionTree> effectivelyFinalValues = new HashMap<>();
private final VisitorState state;

private ProtoNullComparisonScanner(VisitorState state) {
private NullComparisonScanner(VisitorState state) {
this.state = state;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
import com.google.errorprone.bugpatterns.ImmutableMemberCollection;
import com.google.errorprone.bugpatterns.ImmutableSetForContains;
import com.google.errorprone.bugpatterns.ImplementAssertionWithChaining;
import com.google.errorprone.bugpatterns.ImpossibleNullComparison;
import com.google.errorprone.bugpatterns.Incomparable;
import com.google.errorprone.bugpatterns.IncompatibleModifiersChecker;
import com.google.errorprone.bugpatterns.InconsistentCapitalization;
Expand Down Expand Up @@ -285,7 +286,6 @@
import com.google.errorprone.bugpatterns.PrivateSecurityContractProtoAccess;
import com.google.errorprone.bugpatterns.ProtectedMembersInFinalClass;
import com.google.errorprone.bugpatterns.ProtoBuilderReturnValueIgnored;
import com.google.errorprone.bugpatterns.ProtoFieldNullComparison;
import com.google.errorprone.bugpatterns.ProtoRedundantSet;
import com.google.errorprone.bugpatterns.ProtoStringFieldReferenceEquality;
import com.google.errorprone.bugpatterns.ProtoTruthMixedDescriptors;
Expand Down Expand Up @@ -669,6 +669,7 @@ public static ScannerSupplier errorChecks() {
IdentityHashMapBoxing.class,
IgnoredPureGetter.class,
ImmutableChecker.class,
ImpossibleNullComparison.class,
Incomparable.class,
IncompatibleArgumentType.class,
IncompatibleModifiersChecker.class,
Expand Down Expand Up @@ -736,7 +737,6 @@ public static ScannerSupplier errorChecks() {
PreconditionsInvalidPlaceholder.class,
PrivateSecurityContractProtoAccess.class,
ProtoBuilderReturnValueIgnored.class,
ProtoFieldNullComparison.class,
ProtoStringFieldReferenceEquality.class,
ProtoTruthMixedDescriptors.class,
ProtocolBufferOrdinal.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
*/
@RunWith(JUnit4.class)
@Ignore("b/130670448")
public final class ProtoFieldNullComparisonTest {
public final class ImpossibleNullComparisonTest {

private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(ProtoFieldNullComparison.class, getClass());
CompilationTestHelper.newInstance(ImpossibleNullComparison.class, getClass());

@Test
public void scalarCases() {
Expand Down Expand Up @@ -144,7 +144,7 @@ public void testMessageOrBuilderGetField() {
"import com.google.errorprone.bugpatterns.proto.ProtoTest.TestProtoMessage;",
"public class Test {",
" public boolean doIt(TestProtoMessage mob, FieldDescriptor f) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" return mob.getField(f) == null;",
" }",
"}")
Expand All @@ -161,7 +161,7 @@ public void testMessageOrBuilderGetFieldCast() {
"public class Test {",
" public boolean doIt(TestProtoMessage mob, FieldDescriptor f) {",
" String s = ((String) mob.getField(f));",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" return s == null;",
" }",
"}")
Expand All @@ -177,7 +177,7 @@ public void testExtendableMessageGetExtension1param() {
"import com.google.errorprone.bugpatterns.proto.ProtoTest.TestProtoMessage;",
"public class Test {",
" public void test(TestProtoMessage e, ExtensionLite extensionLite) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" boolean a = e.getExtension(extensionLite) == null;",
" }",
"}")
Expand All @@ -193,7 +193,7 @@ public void testMessageOrBuilderGetRepeatedField() {
"import com.google.errorprone.bugpatterns.proto.ProtoTest.TestProtoMessage;",
"public class Test {",
" public void doIt(TestProtoMessage mob, FieldDescriptor f) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" boolean a = mob.getRepeatedField(f, 0) == null;",
" }",
"}")
Expand All @@ -209,7 +209,7 @@ public void testExtendableMessageGetExtension2param() {
"import com.google.errorprone.bugpatterns.proto.ProtoTest.TestProtoMessage;",
"public class Test {",
" public void test(TestProtoMessage e, ExtensionLite extensionLite) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" boolean a = e.getExtension(extensionLite, 0) == null;",
" }",
"}")
Expand All @@ -228,7 +228,7 @@ public void repeated() {
"public class Test {",
" public void test(ExtensionLite<TestProtoMessage,",
" List<TestFieldProtoMessage>> e, TestProtoMessage message) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" boolean y = message.getExtension(e) == null;",
" }",
"}")
Expand All @@ -255,7 +255,7 @@ public void repeated2() {
" ImmutableList.Builder extensionList = ImmutableList.builder();",
" int extensionCount = mob.getExtensionCount(extension);",
" for (int extensionIndex = 0; extensionIndex < extensionCount; ++extensionIndex) {",
" // BUG: Diagnostic contains: ProtoFieldNullComparison",
" // BUG: Diagnostic contains:",
" boolean y = mob.getExtension(extension) == null;",
" extensionList.add(mob.getExtension(extension));",
" }",
Expand Down
File renamed without changes.

0 comments on commit 5b56e10

Please sign in to comment.