From 2a034eb0bbe03495b4c218a7643d676765afd456 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 3 Apr 2019 10:41:24 +0700 Subject: [PATCH] IDEA-210298 Result of method call ignored false-positive on Validate.nonNull check from commons-lang3 --- .../dataFlow/HardcodedContracts.java | 8 +- .../ig/bugs/IgnoreResultOfCallInspection.java | 9 +- .../IgnoreResultOfCallInspectionTest.groovy | 89 ++++++++++++------- 3 files changed, 66 insertions(+), 40 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java index bffbe182697f..5074c1bd20fc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java @@ -182,7 +182,13 @@ public class HardcodedContracts { if ("notNull".equals(methodName) && paramCount > 0) { ValueConstraint[] constraints = createConstraintArray(paramCount); constraints[0] = NULL_VALUE; - return Collections.singletonList(new StandardMethodContract(constraints, fail())); + StandardMethodContract contract = new StandardMethodContract(constraints, fail()); + if (PsiType.VOID.equals(method.getReturnType())) { + return Collections.singletonList(contract); + } + else { + return Arrays.asList(contract, new StandardMethodContract(createConstraintArray(paramCount), returnParameter(0))); + } } } else if (isJunit(className) || isTestng(className) || diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/IgnoreResultOfCallInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/IgnoreResultOfCallInspection.java index 0f6b66b7e1e1..7c35c39d84eb 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/IgnoreResultOfCallInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/bugs/IgnoreResultOfCallInspection.java @@ -285,11 +285,10 @@ public class IgnoreResultOfCallInspection extends BaseInspection { private boolean hasTrivialReturnValue(PsiMethod method) { List contracts = JavaMethodContractUtil.getMethodCallContracts(method, null); - return !contracts.isEmpty() && - contracts.stream() - .map(MethodContract::getReturnValue) - .allMatch(returnValue -> returnValue.equals(ContractReturnValue.returnThis()) || - returnValue instanceof ContractReturnValue.ParameterReturnValue); + ContractReturnValue nonFailingReturnValue = JavaMethodContractUtil.getNonFailingReturnValue(contracts); + return nonFailingReturnValue != null && + (nonFailingReturnValue.equals(ContractReturnValue.returnThis()) || + nonFailingReturnValue instanceof ContractReturnValue.ParameterReturnValue); } private void registerMethodCallOrRefError(PsiExpression call, PsiClass aClass) { diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/IgnoreResultOfCallInspectionTest.groovy b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/IgnoreResultOfCallInspectionTest.groovy index 4ca7eeded9ae..416b4cb2610e 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/IgnoreResultOfCallInspectionTest.groovy +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/IgnoreResultOfCallInspectionTest.groovy @@ -38,43 +38,54 @@ class IgnoreResultOfCallInspectionTest extends LightInspectionTestCase { @Override protected String[] getEnvironmentClasses() { return [ - "package java.util.regex; public class Pattern {" + - " public static Pattern compile(String regex) {return null;}" + - " public Matcher matcher(CharSequence input) {return null;}" + - "}", - "package java.util.regex; public class Matcher {" + - " public boolean find() {return true;}" + - "}", + """package java.util.regex; - "package javax.annotation;\n" + - "\n" + - "import java.lang.annotation.Documented;\n" + - "import java.lang.annotation.ElementType;\n" + - "import java.lang.annotation.Retention;\n" + - "import java.lang.annotation.RetentionPolicy;\n" + - "import java.lang.annotation.Target;\n" + - "\n" + - "import javax.annotation.meta.When;\n" + - "\n" + - "@Documented\n" + - "@Target( { ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE,\n" + - " ElementType.PACKAGE })\n" + - "@Retention(RetentionPolicy.RUNTIME)\n" + - "public @interface CheckReturnValue {\n" + - " When when() default When.ALWAYS;\n" + - "}", +public class Pattern { + public static Pattern compile(String regex) {return null;} - "package a;\n" + - " public @interface CheckReturnValue {}", + public Matcher matcher(CharSequence input) {return null;} +}""", - "package com.google.errorprone.annotations;" + - "import java.lang.annotation.ElementType;\n" + - "import java.lang.annotation.Retention;\n" + - "import java.lang.annotation.RetentionPolicy;\n" + - "import java.lang.annotation.Target;\n" + - "@Target(value={ElementType.METHOD, ElementType.TYPE})\n" + - "@Retention(value=RetentionPolicy.CLASS)\n" + - "public @interface CanIgnoreReturnValue {}" + """package java.util.regex; + +public class Matcher { + public boolean find() {return true;} +}""", + + """package javax.annotation; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import javax.annotation.meta.When; + +@Documented +@Target( { ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE, + ElementType.PACKAGE }) +@Retention(RetentionPolicy.RUNTIME) +public @interface CheckReturnValue { + When when() default When.ALWAYS; +}""", + + """package a; + public @interface CheckReturnValue {}""", + + """package com.google.errorprone.annotations; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +@Target(value={ElementType.METHOD, ElementType.TYPE}) +@Retention(value=RetentionPolicy.CLASS) +public @interface CanIgnoreReturnValue {}""", + + """package org.apache.commons.lang3; +public class Validate { + public native static T notNull(T object); +}""" ] as String[] } @@ -374,6 +385,16 @@ public static int atLeast(int min, int actual, String varName) { opt.get(); if (opt.isPresent()) opt./*Result of 'Optional.get()' is ignored*/get/**/(); } +}""" + } + + void testCommonsLang3NotNull() { + doTest """import org.apache.commons.lang3.Validate; +class X{ + void test(String foo) { + if (foo == null) return; + Validate.notNull(foo); + } }""" } } \ No newline at end of file