From 3b5bd1761ba7cc4bafdc1b57250d73157585b6e1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Sat, 9 May 2020 14:07:50 +0700 Subject: [PATCH] NPE navigation: better support compiler-generated null-checks GitOrigin-RevId: 6f611a250f2fe0a2e1e0532b0abdca5045e5ec53 --- .../DataflowExceptionAnalysisProvider.java | 3 + .../filters/ExceptionWorkerTest.java | 22 +++++++ .../execution/filters/ExceptionWorker.java | 7 ++ .../filters/NullPointerExceptionInfo.java | 65 +++++++++++-------- 4 files changed, 70 insertions(+), 27 deletions(-) diff --git a/java/java-impl/src/com/intellij/slicer/DataflowExceptionAnalysisProvider.java b/java/java-impl/src/com/intellij/slicer/DataflowExceptionAnalysisProvider.java index b3d345d4ab70..ea10af9bd34d 100644 --- a/java/java-impl/src/com/intellij/slicer/DataflowExceptionAnalysisProvider.java +++ b/java/java-impl/src/com/intellij/slicer/DataflowExceptionAnalysisProvider.java @@ -49,6 +49,9 @@ public class DataflowExceptionAnalysisProvider implements ExceptionAnalysisProvi } private static @Nullable Analysis getIntermediateRowAnalysis(@NotNull PsiElement anchor) { + if (anchor instanceof PsiExpression) { + return new Analysis(DfTypes.NULL, (PsiExpression)anchor); + } if (!(anchor instanceof PsiIdentifier)) return null; PsiReferenceExpression ref = tryCast(anchor.getParent(), PsiReferenceExpression.class); if (ref == null) return null; diff --git a/java/java-tests/testSrc/com/intellij/java/execution/filters/ExceptionWorkerTest.java b/java/java-tests/testSrc/com/intellij/java/execution/filters/ExceptionWorkerTest.java index 4b491ebbe632..d127de8b4d57 100644 --- a/java/java-tests/testSrc/com/intellij/java/execution/filters/ExceptionWorkerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/execution/filters/ExceptionWorkerTest.java @@ -24,8 +24,10 @@ import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.Trinity; import com.intellij.psi.PsiClass; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; import org.intellij.lang.annotations.Language; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Arrays; @@ -35,6 +37,10 @@ import java.util.List; * @author gregsh */ public class ExceptionWorkerTest extends LightJavaCodeInsightFixtureTestCase { + @Override + protected @NotNull LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } public void testParsing() { myFixture.addClass("package com.sample;\n" + @@ -620,6 +626,22 @@ public class ExceptionWorkerTest extends LightJavaCodeInsightFixtureTestCase { checkColumnFinder(classText, traceAndPositions); } + public void testNpeRequireNonNullOnSwitch() { + @Language("JAVA") String classText = + "/** @noinspection ALL*/\n" + + "public class MainTest {\n" + + " public static void main(String[] args) {\n" + + " String test = null;\n" + + " switch(test) {}\n" + + " }\n" + + "}"; + List> traceAndPositions = Arrays.asList( + Trinity.create("Exception in thread \"main\" java.lang.NullPointerException\n", null, null), + Trinity.create("\tat java.base/java.util.Objects.requireNonNull(Objects.java:222)\n", null, null), + Trinity.create("\tat MainTest.main(MainTest.java:5)\n", 5, 16)); + checkColumnFinder(classText, traceAndPositions); + } + public void testNpePoorMan() { @Language("JAVA") String classText = "/** @noinspection ALL*/\n" + diff --git a/java/openapi/src/com/intellij/execution/filters/ExceptionWorker.java b/java/openapi/src/com/intellij/execution/filters/ExceptionWorker.java index 30733003ad39..81fc1032b508 100644 --- a/java/openapi/src/com/intellij/execution/filters/ExceptionWorker.java +++ b/java/openapi/src/com/intellij/execution/filters/ExceptionWorker.java @@ -344,6 +344,13 @@ public class ExceptionWorker { @Override public PsiElement matchElement(@NotNull PsiElement element) { + if (myMethodName.equals("requireNonNull") && myClassName.equals(CommonClassNames.JAVA_UTIL_OBJECTS)) { + // Since Java 9 Objects.requireNonNull(x) is used by javac instead of x.getClass() for generated null-check (JDK-8074306) + PsiExpression expression = NullPointerExceptionInfo.matchCompilerGeneratedNullCheck(element); + if (expression != null) { + return expression; + } + } if (!(element instanceof PsiIdentifier)) return null; if (myMethodName.equals("")) { if (myHasDollarInName || element.textMatches(StringUtil.getShortName(myClassName))) { diff --git a/java/openapi/src/com/intellij/execution/filters/NullPointerExceptionInfo.java b/java/openapi/src/com/intellij/execution/filters/NullPointerExceptionInfo.java index f8518abee257..ea37c4f52bb9 100644 --- a/java/openapi/src/com/intellij/execution/filters/NullPointerExceptionInfo.java +++ b/java/openapi/src/com/intellij/execution/filters/NullPointerExceptionInfo.java @@ -54,9 +54,7 @@ public class NullPointerExceptionInfo extends ExceptionInfo { if (result != null) return result; result = fromSynchronized(e); if (result != null) return result; - result = fromQualifiedNew(e); - if (result != null) return result; - result = fromMethodReference(e); + result = matchCompilerGeneratedNullCheck(e); if (result != null) return result; result = fromUnboxing(e, null); if (result != null) return result; @@ -127,26 +125,6 @@ public class NullPointerExceptionInfo extends ExceptionInfo { } return null; } - - private static PsiExpression fromMethodReference(PsiElement e) { - if (e instanceof PsiJavaToken && ((PsiJavaToken)e).getTokenType().equals(JavaTokenType.DOUBLE_COLON)) { - PsiMethodReferenceExpression methodRef = tryCast(e.getParent(), PsiMethodReferenceExpression.class); - if (methodRef == null) return null; - PsiExpression qualifier = methodRef.getQualifierExpression(); - if (mayBeNull(qualifier)) return qualifier; - } - return null; - } - - private static PsiExpression fromQualifiedNew(PsiElement e) { - if (e instanceof PsiKeyword && e.textMatches(PsiKeyword.NEW)) { - PsiNewExpression newExpression = tryCast(e.getParent(), PsiNewExpression.class); - if (newExpression == null) return null; - PsiExpression qualifier = newExpression.getQualifier(); - if (mayBeNull(qualifier)) return qualifier; - } - return null; - } private static UnaryOperator getJep358Extractor(String message) { if (!message.startsWith("Cannot ")) return null; @@ -191,10 +169,10 @@ public class NullPointerExceptionInfo extends ExceptionInfo { String methodName = method.substring(dotPos + 1); PsiPrimitiveType type = UNBOXING_METHODS.get(methodName); return e -> { - if (methodName.equals("getClass")) { - PsiExpression result = fromMethodReference(e); - if (result != null) return result; - result = fromQualifiedNew(e); + if (methodName.equals("getClass") || methodName.equals("ordinal")) { + // x.getClass() was generated as a null-check by javac until Java 9 + // it's still possible that such a code is executed under Java 14+ + PsiElement result = matchCompilerGeneratedNullCheck(e); if (result != null) return result; } if (type != null) { @@ -214,6 +192,39 @@ public class NullPointerExceptionInfo extends ExceptionInfo { return null; } + @Nullable + static PsiExpression matchCompilerGeneratedNullCheck(PsiElement e) { + PsiExpression dereferenced = null; + if (e instanceof PsiJavaToken && ((PsiJavaToken)e).getTokenType().equals(JavaTokenType.DOUBLE_COLON)) { + // method reference qualifier + PsiMethodReferenceExpression methodRef = tryCast(e.getParent(), PsiMethodReferenceExpression.class); + if (methodRef != null) { + dereferenced = methodRef.getQualifierExpression(); + } + } + else if (e instanceof PsiKeyword && e.textMatches(PsiKeyword.SWITCH)) { + // switch on string or enum + PsiSwitchBlock switchBlock = tryCast(e.getParent(), PsiSwitchBlock.class); + if (switchBlock != null) { + PsiExpression selector = switchBlock.getExpression(); + if (selector != null) { + PsiClass psiClass = PsiUtil.resolveClassInClassTypeOnly(selector.getType()); + if (psiClass != null && (psiClass.isEnum() || CommonClassNames.JAVA_LANG_STRING.equals(psiClass.getQualifiedName()))) { + dereferenced = selector; + } + } + } + } + else if (e instanceof PsiKeyword && e.textMatches(PsiKeyword.NEW)) { + // qualified new + PsiNewExpression newExpression = tryCast(e.getParent(), PsiNewExpression.class); + if (newExpression != null) { + dereferenced = newExpression.getQualifier(); + } + } + return mayBeNull(dereferenced) ? dereferenced : null; + } + private static boolean mayBeNull(PsiExpression qualifier) { return qualifier != null && !(qualifier instanceof PsiNewExpression) && !(qualifier instanceof PsiLiteralExpression) && !(qualifier instanceof PsiPolyadicExpression);