From 9c3064008ea4c7c1e467dcccdb5ba166221ec03c Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 6 Mar 2013 18:12:10 +0100 Subject: [PATCH] make custom condition checking methods and the hardcoded ones work the same way (CR-IC-238) --- .../codeInsight/ConditionCheckManager.java | 78 +++-------- .../dataFlow/ControlFlowAnalyzer.java | 122 ++++++------------ 2 files changed, 58 insertions(+), 142 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/ConditionCheckManager.java b/java/java-impl/src/com/intellij/codeInsight/ConditionCheckManager.java index c23706c40bef..f7edb1e3b250 100644 --- a/java/java-impl/src/com/intellij/codeInsight/ConditionCheckManager.java +++ b/java/java-impl/src/com/intellij/codeInsight/ConditionCheckManager.java @@ -20,6 +20,7 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiMethod; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.List; @@ -146,7 +147,7 @@ public class ConditionCheckManager implements PersistentStateComponent listToLoadTo, List listToLoadFrom, ConditionChecker.Type type){ + private static void loadMethods(List listToLoadTo, List listToLoadFrom, ConditionChecker.Type type){ listToLoadTo.clear(); for (String setting : listToLoadFrom) { try { @@ -157,70 +158,25 @@ public class ConditionCheckManager implements PersistentStateComponent checkers) { + @Nullable + public static ConditionChecker findConditionChecker(@NotNull PsiMethod method) { + ConditionCheckManager instance = getInstance(method.getProject()); + ConditionChecker checker = methodMatches(method, instance.getIsNullCheckMethods()); + if (checker == null) checker = methodMatches(method, instance.getIsNotNullCheckMethods()); + if (checker == null) checker = methodMatches(method, instance.getAssertIsNullMethods()); + if (checker == null) checker = methodMatches(method, instance.getAssertIsNotNullMethods()); + if (checker == null) checker = methodMatches(method, instance.getAssertTrueMethods()); + if (checker == null) checker = methodMatches(method, instance.getAssertFalseMethods()); + return checker; + } + + private static ConditionChecker methodMatches(PsiMethod psiMethod, List checkers) { for (ConditionChecker checker : checkers) { if (checker.matchesPsiMethod(psiMethod)) { - return true; + return checker; } } - return false; + return null; } - public static boolean isNullCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getIsNullCheckMethods()); - } - - public static boolean isNotNullCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getIsNotNullCheckMethods()); - } - - public static boolean isAssertIsNullCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getAssertIsNullMethods()); - } - - public static boolean isAssertIsNotNullCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getAssertIsNotNullMethods()); - } - - public static boolean isAssertTrueCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getAssertTrueMethods()); - } - - public static boolean isAssertFalseCheckMethod(PsiMethod psiMethod) { - return methodMatches(psiMethod, getInstance(psiMethod.getProject()).getAssertFalseMethods()); - } - - public static boolean isNullCheckMethod(PsiMethod psiMethod, int paramIndex) { - ConditionCheckManager instance = getInstance(psiMethod.getProject()); - return methodMatches(psiMethod, paramIndex, instance.getIsNullCheckMethods()) || - methodMatches(psiMethod, paramIndex, instance.getIsNotNullCheckMethods()); - } - - public static boolean isNullabilityAssertionMethod(PsiMethod psiMethod, int paramIndex) { - ConditionCheckManager instance = getInstance(psiMethod.getProject()); - return methodMatches(psiMethod, paramIndex, instance.getAssertIsNullMethods()) || - methodMatches(psiMethod, paramIndex, instance.getAssertIsNotNullMethods()); - } - - public static boolean isBooleanAssertMethod(PsiMethod psiMethod, int paramIndex) { - ConditionCheckManager instance = getInstance(psiMethod.getProject()); - return methodMatches(psiMethod, paramIndex, instance.getAssertTrueMethods()) || - methodMatches(psiMethod, paramIndex, instance.getAssertFalseMethods()); - } - - public static boolean methodMatches(PsiMethod psiMethod, List checkers) { - for (ConditionChecker checker : checkers) { - if (checker.matchesPsiMethod(psiMethod)) - return true; - } - return false; - } - - public static boolean methodMatches(PsiMethod psiMethod, int paramIndex, List checkers) { - for (ConditionChecker checker : checkers) { - if (checker.matchesPsiMethod(psiMethod, paramIndex)) - return true; - } - return false; - } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 27537fb812d3..ca53a86ae615 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -16,6 +16,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.ConditionCheckManager; +import com.intellij.codeInsight.ConditionChecker; import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInspection.dataFlow.instructions.*; import com.intellij.codeInspection.dataFlow.value.*; @@ -1044,7 +1045,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { rExpr.accept(this); if (!comparingRef) { - generateBoxingUnboxingInstructionFor(rExpr,castType); + generateBoxingUnboxingInstructionFor(rExpr, castType); } } @@ -1291,17 +1292,16 @@ class ControlFlowAnalyzer extends JavaElementVisitor { PsiMethod resolved = expression.resolveMethod(); if (resolved != null) { - final PsiExpressionList argList = expression.getArgumentList(); @NonNls String methodName = resolved.getName(); - PsiExpression[] params = argList.getExpressions(); + PsiExpression[] params = expression.getArgumentList().getExpressions(); PsiClass owner = resolved.getContainingClass(); final int exitPoint = getEndOffset(expression) - 1; if (owner != null) { final String className = owner.getQualifiedName(); if ("java.lang.System".equals(className)) { if ("exit".equals(methodName)) { - pushParameters(params, false, false); + pushParameters(params, -1); addInstruction(new ReturnInstruction()); return true; } @@ -1310,84 +1310,48 @@ class ControlFlowAnalyzer extends JavaElementVisitor { "junit.framework.TestCase".equals(className) || "org.testng.Assert".equals(className)) { boolean testng = "org.testng.Assert".equals(className); if ("fail".equals(methodName)) { - pushParameters(params, false, !testng); + pushParameters(params, -1); returnCheckingFinally(); return true; } - else if ("assertTrue".equals(methodName)) { - pushParameters(params, true, !testng); + + int checkedParam = testng ? 0 : params.length - 1; + if ("assertTrue".equals(methodName)) { + pushParameters(params, checkedParam); conditionalExit(exitPoint, false); return true; } - else if ("assertFalse".equals(methodName)) { - pushParameters(params, true, !testng); + if ("assertFalse".equals(methodName)) { + pushParameters(params, checkedParam); conditionalExit(exitPoint, true); return true; } - else if ("assertNull".equals(methodName)) { - pushParameters(params, true, !testng); - - addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, expression.getProject())); - conditionalExit(exitPoint, false); + if ("assertNull".equals(methodName)) { + pushParameters(params, checkedParam); + handleAssertNullityMethod(expression, exitPoint, false); return true; } - else if ("assertNotNull".equals(methodName)) { - pushParameters(params, true, !testng); - - addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, expression.getProject())); - conditionalExit(exitPoint, true); + if ("assertNotNull".equals(methodName)) { + pushParameters(params, checkedParam); + handleAssertNullityMethod(expression, exitPoint, true); return true; } return false; } } - boolean assertNull = ConditionCheckManager.isAssertIsNullCheckMethod(resolved); - boolean assertNotNull = ConditionCheckManager.isAssertIsNotNullCheckMethod(resolved); - if (assertNull || assertNotNull) { - for (int i = 0; i < params.length; i++) { - params[i].accept(this); - if (ConditionCheckManager.isNullabilityAssertionMethod(resolved, i)) { - addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); - addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, expression.getProject())); - conditionalExit(exitPoint, assertNotNull); // Exit if ==null for assertNull and != null for assertNotNull - } - else { - addInstruction(new PopInstruction()); - } - } - return true; - } + ConditionChecker checker = ConditionCheckManager.findConditionChecker(resolved); + if (checker != null) { + pushParameters(params, checker.getCheckedParameterIndex()); - boolean isNull = ConditionCheckManager.isNullCheckMethod(resolved); - boolean isNotNull = ConditionCheckManager.isNotNullCheckMethod(resolved); - if (isNull || isNotNull) { - for (int i = 0; i < params.length; i++) { - params[i].accept(this); - if (ConditionCheckManager.isNullCheckMethod(resolved, i)) { - addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); - addInstruction(new BinopInstruction(isNull ? JavaTokenType.EQEQ : JavaTokenType.NE, null, expression.getProject())); - } - else { - addInstruction(new PopInstruction()); - } - } - return true; - } - - boolean assertTrue = ConditionCheckManager.isAssertTrueCheckMethod(resolved); - boolean assertFalse = ConditionCheckManager.isAssertFalseCheckMethod(resolved); - if (assertTrue || assertFalse) { - for (int i = 0; i < params.length; i++) { - params[i].accept(this); - if (ConditionCheckManager.isBooleanAssertMethod(resolved, i)) { - conditionalExit(exitPoint, assertFalse); - } - else { - addInstruction(new PopInstruction()); - } + ConditionChecker.Type type = checker.getConditionCheckType(); + if (type == ConditionChecker.Type.ASSERT_IS_NULL_METHOD || type == ConditionChecker.Type.ASSERT_IS_NOT_NULL_METHOD) { + handleAssertNullityMethod(expression, exitPoint, type == ConditionChecker.Type.ASSERT_IS_NOT_NULL_METHOD); + } else if (type == ConditionChecker.Type.IS_NULL_METHOD || type == ConditionChecker.Type.IS_NOT_NULL_METHOD) { + addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); + addInstruction(new BinopInstruction(type == ConditionChecker.Type.IS_NULL_METHOD ? JavaTokenType.EQEQ : JavaTokenType.NE, null, expression.getProject())); + } else { //assertTrue or assertFalse + conditionalExit(exitPoint, type == ConditionChecker.Type.ASSERT_FALSE_METHOD); } return true; } @@ -1398,19 +1362,12 @@ class ControlFlowAnalyzer extends JavaElementVisitor { final PsiType qualifierType = qualifierExpression.getType(); if (qualifierType != null && qualifierType.equalsToText("com.intellij.openapi.diagnostic.Logger")) { if ("error".equals(methodName)) { - for (PsiExpression param : params) { - param.accept(this); - addInstruction(new PopInstruction()); - } + pushParameters(params, -1); returnCheckingFinally(); return true; } - else if ("assertTrue".equals(methodName)) { - params[0].accept(this); - for (int i = 1; i < params.length; i++) { - params[i].accept(this); - addInstruction(new PopInstruction()); - } + if ("assertTrue".equals(methodName)) { + pushParameters(params, 0); conditionalExit(exitPoint, false); return true; } @@ -1422,21 +1379,24 @@ class ControlFlowAnalyzer extends JavaElementVisitor { return false; } + private void handleAssertNullityMethod(PsiMethodCallExpression expression, int exitPoint, boolean assertNotNull) { + addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); + addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, expression.getProject())); + conditionalExit(exitPoint, assertNotNull); // Exit if ==null for assertNull and != null for assertNotNull + } + private void conditionalExit(final int continuePoint, final boolean exitIfTrue) { addInstruction(new ConditionalGotoInstruction(continuePoint, exitIfTrue, null)); addInstruction(new ReturnInstruction()); pushUnknown(); } - private void pushParameters(final PsiExpression[] params, final boolean leaveOnStack, boolean lastParameterIsSignificant) { + private void pushParameters(final PsiExpression[] params, final int leaveOnStack) { for (int i = 0; i < params.length; i++) { - PsiExpression param = params[i]; - param.accept(this); - if (leaveOnStack) { - if (lastParameterIsSignificant && i == params.length - 1 || !lastParameterIsSignificant && i == 0) continue; + params[i].accept(this); + if (leaveOnStack != i) { + addInstruction(new PopInstruction()); } - - addInstruction(new PopInstruction()); } }