From d0e4d4fd1bcc994b79b77f90ba2c863bd9ec624e Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 23 Jul 2013 13:08:34 +0200 Subject: [PATCH] support multi-clause contracts (IDEA-93372) --- .../dataFlow/ControlFlowAnalyzer.java | 96 +++++++++++-------- .../dataFlow/instructions/DupInstruction.java | 30 +++++- .../dataFlow/fixture/ContractAnnotation.java | 16 +++- .../org/jetbrains/annotations/Contract.java | 6 +- 4 files changed, 101 insertions(+), 47 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 44e379c8f194..6b84ec4e03ea 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -31,6 +31,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.*; import com.intellij.util.IncorrectOperationException; import com.intellij.util.SmartList; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.Stack; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -1248,9 +1249,9 @@ class ControlFlowAnalyzer extends JavaElementVisitor { try { startElement(expression); - MethodContract contract = getCallContract(expression); - if (contract != null) { - handleContract(expression, contract); + List contracts = getCallContract(expression); + if (!contracts.isEmpty()) { + handleContracts(expression, contracts); return; } @@ -1301,28 +1302,36 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - private void handleContract(PsiMethodCallExpression expression, MethodContract contract) { - PsiExpression[] params = expression.getArgumentList().getExpressions(); - if (params.length != contract.arguments.length) { - return; + private void handleContracts(PsiMethodCallExpression expression, List contracts) { + PsiExpression[] args = expression.getArgumentList().getExpressions(); + for (PsiExpression arg : args) { + arg.accept(this); } + if (contracts.size() > 1) { + addInstruction(new DupInstruction(args.length, contracts.size() - 1)); + } + for (MethodContract contract : contracts) { + if (args.length == contract.arguments.length) { + handleContract(expression, contract); + } + } + pushUnknown(); // goto here if all contracts are false + } + + private void handleContract(PsiMethodCallExpression expression, MethodContract contract) { + PsiExpression[] args = expression.getArgumentList().getExpressions(); final int exitPoint = getEndOffset(expression); List gotoContractFalse = new SmartList(); - for (int i = 0; i < params.length; i++) { - params[i].accept(this); - if (contract.arguments[i] == ValueConstraint.ANY_VALUE) { - addInstruction(new PopInstruction()); - } - } - for (int i = params.length - 1; i >= 0; i--) { + for (int i = args.length - 1; i >= 0; i--) { ValueConstraint arg = contract.arguments[i]; if (arg == ValueConstraint.NULL_VALUE || arg == ValueConstraint.NOT_NULL_VALUE) { addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, expression.getProject())); } else if (arg != ValueConstraint.TRUE_VALUE && arg != ValueConstraint.FALSE_VALUE) { + addInstruction(new PopInstruction()); continue; } @@ -1372,19 +1381,18 @@ class ControlFlowAnalyzer extends JavaElementVisitor { for (ConditionalGotoInstruction instruction : gotoContractFalse) { instruction.setOffset(myCurrentFlow.getInstructionCount()); } - pushUnknown(); } - private static MethodContract getCallContract(PsiMethodCallExpression expression) { + private static List getCallContract(PsiMethodCallExpression expression) { PsiMethod resolved = expression.resolveMethod(); if (resolved != null) { final PsiAnnotation contractAnno = AnnotationUtil.findAnnotation(resolved, "org.jetbrains.annotations.Contract"); if (contractAnno != null) { final Project project = expression.getProject(); - return CachedValuesManager.getManager(project).getCachedValue(contractAnno, new CachedValueProvider() { + return CachedValuesManager.getManager(project).getCachedValue(contractAnno, new CachedValueProvider>() { @Nullable @Override - public Result compute() { + public Result> compute() { PsiAnnotationMemberValue value = contractAnno.findAttributeValue(null); Object text = JavaPsiFacade.getInstance(project).getConstantEvaluationHelper().computeConstantExpression(value); if (text instanceof String) { @@ -1394,7 +1402,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { catch (Exception ignored) { } } - return Result.create(null, contractAnno); + return Result.create(Collections.emptyList(), contractAnno); } }); } @@ -1407,35 +1415,35 @@ class ControlFlowAnalyzer extends JavaElementVisitor { final String className = owner.getQualifiedName(); if ("java.lang.System".equals(className)) { if ("exit".equals(methodName)) { - return new MethodContract(getAnyArgConstraints(params), ValueConstraint.SYSTEM_EXIT); + return Arrays.asList(new MethodContract(getAnyArgConstraints(params), ValueConstraint.SYSTEM_EXIT)); } } else if ("junit.framework.Assert".equals(className) || "org.junit.Assert".equals(className) || "junit.framework.TestCase".equals(className) || "org.testng.Assert".equals(className)) { boolean testng = "org.testng.Assert".equals(className); if ("fail".equals(methodName)) { - return new MethodContract(getAnyArgConstraints(params), ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(getAnyArgConstraints(params), ValueConstraint.THROW_EXCEPTION)); } int checkedParam = testng ? 0 : params.length - 1; ValueConstraint[] constraints = getAnyArgConstraints(params); if ("assertTrue".equals(methodName)) { constraints[checkedParam] = ValueConstraint.FALSE_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } if ("assertFalse".equals(methodName)) { constraints[checkedParam] = ValueConstraint.TRUE_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } if ("assertNull".equals(methodName)) { constraints[checkedParam] = ValueConstraint.NOT_NULL_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } if ("assertNotNull".equals(methodName)) { constraints[checkedParam] = ValueConstraint.NULL_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } - return null; + return Collections.emptyList(); } } @@ -1444,37 +1452,43 @@ class ControlFlowAnalyzer extends JavaElementVisitor { ValueConstraint[] constraints = getAnyArgConstraints(params); int checkedParam = checker.getCheckedParameterIndex(); if (checkedParam >= constraints.length) { - return null; + return Collections.emptyList(); } ConditionChecker.Type type = checker.getConditionCheckType(); if (type == ASSERT_IS_NULL_METHOD || type == ASSERT_IS_NOT_NULL_METHOD) { constraints[checkedParam] = type == ASSERT_IS_NOT_NULL_METHOD ? ValueConstraint.NULL_VALUE : ValueConstraint.NOT_NULL_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } else if (type == IS_NULL_METHOD || type == IS_NOT_NULL_METHOD) { constraints[checkedParam] = type == IS_NULL_METHOD ? ValueConstraint.NOT_NULL_VALUE : ValueConstraint.NULL_VALUE; - return new MethodContract(constraints, ValueConstraint.FALSE_VALUE); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.FALSE_VALUE)); } else { //assertTrue or assertFalse constraints[checkedParam] = type == ASSERT_FALSE_METHOD ? ValueConstraint.TRUE_VALUE : ValueConstraint.FALSE_VALUE; - return new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION); + return Arrays.asList(new MethodContract(constraints, ValueConstraint.THROW_EXCEPTION)); } } } - return null; + return Collections.emptyList(); } - private static MethodContract parseContract(String text) throws ParseException { - text = StringUtil.replace(text, " ", ""); - String arrow = "->"; - int arrowIndex = text.indexOf(arrow); - if (arrowIndex < 0) throw new ParseException("A contract must be in form arg1, ..., argN -> return-value"); - String[] argStrings = text.substring(0, arrowIndex).split(","); - ValueConstraint[] args = new ValueConstraint[argStrings.length]; - for (int i = 0; i < args.length; i++) { - args[i] = parseConstraint(argStrings[i]); + private static List parseContract(String text) throws ParseException { + List result = ContainerUtil.newArrayList(); + for (String clause : StringUtil.replace(text, " ", "").split(";")) { + String arrow = "->"; + int arrowIndex = clause.indexOf(arrow); + if (arrowIndex < 0) { + throw new ParseException("A contract clause must be in form arg1, ..., argN -> return-value"); + } + + String[] argStrings = clause.substring(0, arrowIndex).split(","); + ValueConstraint[] args = new ValueConstraint[argStrings.length]; + for (int i = 0; i < args.length; i++) { + args[i] = parseConstraint(argStrings[i]); + } + result.add(new MethodContract(args, parseConstraint(clause.substring(arrowIndex + arrow.length())))); } - return new MethodContract(args, parseConstraint(text.substring(arrowIndex + arrow.length()))); + return result; } private static ValueConstraint parseConstraint(String name) throws ParseException { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/DupInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/DupInstruction.java index ab0e969cd803..7a77a2242c38 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/DupInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/DupInstruction.java @@ -21,16 +21,40 @@ import com.intellij.codeInspection.dataFlow.DfaMemoryState; import com.intellij.codeInspection.dataFlow.InstructionVisitor; import com.intellij.codeInspection.dataFlow.value.DfaValue; +import java.util.ArrayList; +import java.util.List; + /** * @author max */ public class DupInstruction extends Instruction { + private final int myValueCount; + private final int myDuplicationCount; + + public DupInstruction() { + this(1, 1); + } + + public DupInstruction(int valueCount, int duplicationCount) { + myValueCount = valueCount; + myDuplicationCount = duplicationCount; + } @Override public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState memState, InstructionVisitor visitor) { - final DfaValue a = memState.pop(); - memState.push(a); - memState.push(a); + if (myDuplicationCount == 1 && myValueCount == 1) { + memState.push(memState.peek()); + } else { + List values = new ArrayList(myValueCount); + for (int i = 0; i < myValueCount; i++) { + values.add(memState.pop()); + } + for (int j = 0; j < myDuplicationCount; j++) { + for (int i = values.size() - 1; i >= 0; i--) { + memState.push(values.get(i)); + } + } + } Instruction nextInstruction = runner.getInstruction(getIndex() + 1); return new DfaInstructionState[]{new DfaInstructionState(nextInstruction, memState)}; } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java index 22866486e898..f86d79733c73 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractAnnotation.java @@ -1,10 +1,16 @@ import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.Nullable; import java.lang.*; +import java.lang.AssertionError; import java.lang.IllegalArgumentException; public class AssertIsNotNull { - void bar() { + void bar(String s) { + if (s == null && trimIfNotNull(s) != null) { + throw new AssertionError(); + } + final Object o = call(); assertIsNotNull(o); if(o == null) {} @@ -16,6 +22,14 @@ public class AssertIsNotNull { throw new IllegalArgumentException(); } } + + @Contract("null -> null; !null -> !null") + @Nullable static String trimIfNotNull(@Nullable String s) { + if (s == null) { + return null; + } + return s.trim(); + } Object call() {return new Object();} } diff --git a/platform/annotations/src/org/jetbrains/annotations/Contract.java b/platform/annotations/src/org/jetbrains/annotations/Contract.java index 000428ba6145..29d70dadfbbb 100644 --- a/platform/annotations/src/org/jetbrains/annotations/Contract.java +++ b/platform/annotations/src/org/jetbrains/annotations/Contract.java @@ -21,8 +21,9 @@ import java.lang.annotation.*; * Specifies some aspects of the method behavior depending on the arguments. Can be used by tools for advanced data flow analysis.

* * Method contract has the following syntax:
- * contract ::= args '->' effect
- * args ::= ((arg ,)* arg )?
+ * contract ::= (clause ';')* clause
+ * clause ::= args '->' effect
+ * args ::= ((arg ',')* arg )?
* arg ::= value-constraint
* value-constraint ::= 'any' | 'null' | '!null' | 'false' | 'true'
* effect ::= value-constraint | 'fail' | 'exit'

@@ -39,6 +40,7 @@ import java.lang.annotation.*; * * Examples:

* @Contract("any, null -> null") - method returns null if its second argument is null
+ * @Contract("any, null -> null; any, !null -> !null") - method returns null if its second argument is null and not-null otherwise
* @Contract("true -> fail") - a typical assertFalse method which throws an exception if true is passed to it
* * @author peter