diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index d4fa6772c7..784d0d0806 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -46,6 +46,7 @@ import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; import javax.lang.model.element.VariableElement; +import org.checkerframework.nullaway.dataflow.cfg.node.ClassNameNode; import org.checkerframework.nullaway.dataflow.cfg.node.FieldAccessNode; import org.checkerframework.nullaway.dataflow.cfg.node.IntegerLiteralNode; import org.checkerframework.nullaway.dataflow.cfg.node.LocalVariableNode; @@ -477,6 +478,19 @@ && isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) { result = new AccessPath( ((LocalVariableNode) node).getElement(), ImmutableList.copyOf(elements), mapKey); + } else if (node instanceof ClassNameNode) { + // It is useful to make an access path if elements.size() > 1 and elements.getFirst() is + // "this". In this case, we may have an access of a field of an enclosing class from a nested + // class. Tracking an access path for "Foo.this" alone is not useful, since that can never be + // null. Also, we do not attempt to canonicalize cases where "Foo.this" is used to refer to + // the receiver of the current method instead of "this". + if (elements.size() > 1 + && elements.getFirst().getJavaElement().getSimpleName().contentEquals("this")) { + Element rootElement = elements.pop().getJavaElement(); + result = new AccessPath(rootElement, ImmutableList.copyOf(elements), mapKey); + } else { + result = null; + } } else if (node instanceof ThisNode || node instanceof SuperNode) { result = new AccessPath(null, ImmutableList.copyOf(elements), mapKey); } else { diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java index f3f0c07b2f..7282e4e043 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayAccessPathsTests.java @@ -379,4 +379,59 @@ public void testFinalFieldNullabilityPropagatesToInnerContexts() { "}") .doTest(); } + + @Test + public void testAccessUsingExplicitThis() { + defaultCompilationHelper + .addSourceLines( + "Foo.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "public class Foo {", + " @Nullable public Object bar;", + " public class Nested {", + " @Nullable public Object bar;", + " public void testNegative1() {", + " if (Foo.this.bar != null) {", + " Foo.this.bar.toString();", + " }", + " }", + " public void testNegative2() {", + " // Foo.this can never be null", + " Foo.this.toString();", + " }", + " public void testPositive() {", + " if (Foo.this.bar != null) {", + " // BUG: Diagnostic contains: dereferenced expression this.bar is @Nullable", + " this.bar.toString();", + " }", + " if (this.bar != null) {", + " // BUG: Diagnostic contains: dereferenced expression Foo.this.bar is @Nullable", + " Foo.this.bar.toString();", + " }", + " }", + " }", + " public void testUnhandled1() {", + " if (bar != null) {", + " // This is safe but we don't currently handle it", + " // BUG: Diagnostic contains: dereferenced expression Foo.this.bar is @Nullable", + " Foo.this.bar.toString();", + " }", + " }", + " public void testUnhandled2() {", + " if (Foo.this.bar != null) {", + " // This is safe but we don't currently handle it", + " // BUG: Diagnostic contains: dereferenced expression bar is @Nullable", + " bar.toString();", + " }", + " }", + " public void testNegative1() {", + " if (bar != null) {", + " // This is safe and handled", + " this.bar.toString();", + " }", + " }", + "}") + .doTest(); + } }