From 890a2c90eb9a22d2b04e5ef6476243ce0a67278e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 29 Jun 2017 17:22:07 +0300 Subject: [PATCH] CFG inlining preliminary implementation Lambda calls like ((cast) x -> y).run() inlined j.u.Optional chains inlined (with basic methodRef support) --- .../dataFlow/ControlFlowAnalyzer.java | 299 ++++++++++++++++-- .../dataFlow/inliner/CallInliner.java | 35 ++ .../dataFlow/inliner/LambdaInliner.java | 52 +++ .../inliner/OptionalChainInliner.java | 236 ++++++++++++++ .../ConditionalGotoInstruction.java | 4 +- .../instructions/GotoInstruction.java | 4 +- .../instructions/JumpInstruction.java | 22 ++ .../instructions/SpliceInstruction.java | 51 +++ .../dataFlow/fixture/LambdaInlining.java | 15 + .../dataFlow/fixture/OptionalInlining.java | 131 ++++++++ .../DataFlowInspection8Test.java | 3 + 11 files changed, 819 insertions(+), 33 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CallInliner.java create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/LambdaInliner.java create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/JumpInstruction.java create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/SpliceInstruction.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/LambdaInlining.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java 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 4b03ac9c8202..5d912f080a46 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 @@ -17,6 +17,9 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.ExceptionUtil; +import com.intellij.codeInspection.dataFlow.inliner.CallInliner; +import com.intellij.codeInspection.dataFlow.inliner.LambdaInliner; +import com.intellij.codeInspection.dataFlow.inliner.OptionalChainInliner; import com.intellij.codeInspection.dataFlow.instructions.*; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; @@ -35,6 +38,7 @@ import com.siyeh.ig.numeric.UnnecessaryExplicitNumericCastInspection; import com.siyeh.ig.psiutils.CountingLoop; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.VariableAccessUtils; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -58,6 +62,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private final ExceptionTransfer myRuntimeException; private final ExceptionTransfer myError; private final PsiType myAssertionError; + private PsiLambdaExpression myLambdaExpression = null; ControlFlowAnalyzer(final DfaValueFactory valueFactory, @NotNull PsiElement codeFragment, boolean ignoreAssertions) { myFactory = valueFactory; @@ -644,10 +649,20 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { generateBoxingUnboxingInstructionFor(returnValue, LambdaUtil.getFunctionalInterfaceReturnType(lambdaExpression)); } } - addInstruction(new CheckReturnValueInstruction(returnValue)); } - addInstruction(new ReturnInstruction(myFactory.controlTransfer(ReturnTransfer.INSTANCE, myTrapStack), statement)); + if (myLambdaExpression == null) { + if (returnValue != null) { + addInstruction(new CheckReturnValueInstruction(returnValue)); + } + addInstruction(new ReturnInstruction(myFactory.controlTransfer(ReturnTransfer.INSTANCE, myTrapStack), statement)); + } + else { + if (returnValue == null) { + pushUnknown(); + } + controlTransfer(new InstructionTransfer(getEndOffset(myLambdaExpression), getVariablesInside(myLambdaExpression)), myTrapStack); + } finishElement(statement); } @@ -825,6 +840,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } return DfaInstructionState.EMPTY_ARRAY; } + + @Override + public String toString() { + return "APPLY NOT NULL"; + } } @Override @@ -1116,23 +1136,25 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } private void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression expression, PsiType expectedType) { + generateBoxingUnboxingInstructionFor(expression, expression.getType(), expectedType); + } + + private void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression context, PsiType actualType, PsiType expectedType) { if (PsiType.VOID.equals(expectedType)) return; - PsiType exprType = expression.getType(); - - if (TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isPrimitiveWrapper(exprType)) { - addInstruction(new MethodCallInstruction(expression, MethodCallInstruction.MethodType.UNBOXING, expectedType)); + if (TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isPrimitiveWrapper(actualType)) { + addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.UNBOXING, expectedType)); } - else if (TypeConversionUtil.isPrimitiveAndNotNull(exprType) && TypeConversionUtil.isAssignableFromPrimitiveWrapper(expectedType)) { + else if (TypeConversionUtil.isPrimitiveAndNotNull(actualType) && TypeConversionUtil.isAssignableFromPrimitiveWrapper(expectedType)) { addConditionalRuntimeThrow(); - addInstruction(new MethodCallInstruction(expression, MethodCallInstruction.MethodType.BOXING, expectedType)); + addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.BOXING, expectedType)); } - else if (exprType != expectedType && - TypeConversionUtil.isPrimitiveAndNotNull(exprType) && + else if (actualType != expectedType && + TypeConversionUtil.isPrimitiveAndNotNull(actualType) && TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && - TypeConversionUtil.isNumericType(exprType) && + TypeConversionUtil.isNumericType(actualType) && TypeConversionUtil.isNumericType(expectedType)) { - addInstruction(new MethodCallInstruction(expression, MethodCallInstruction.MethodType.CAST, expectedType) { + addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.CAST, expectedType) { @Override public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) { return visitor.visitCast(this, runner, stateBefore); @@ -1333,6 +1355,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitMethodCallExpression(PsiMethodCallExpression expression) { startElement(expression); + for (CallInliner inliner : INLINERS) { + if (inliner.tryInlineCall(new CFGBuilder(this), expression)) { + finishElement(expression); + return; + } + } + PsiReferenceExpression methodExpression = expression.getMethodExpression(); PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); @@ -1369,9 +1398,30 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } + addBareCall(expression); + + if (isEqualsCall) { + // assume equals argument must be not-null if the result is true + // don't assume the call result to be false if arg1==null + + // stack: .., arg1, call-result + ConditionalGotoInstruction ifFalse = addInstruction(new ConditionalGotoInstruction(null, true, null)); + + addInstruction(new ApplyNotNullInstruction(expression)); + addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null)); + addInstruction(new GotoInstruction(getEndOffset(expression))); + + ifFalse.setOffset(myCurrentFlow.getInstructionCount()); + addInstruction(new PopInstruction()); + addInstruction(new PushInstruction(myFactory.getConstFactory().getFalse(), null)); + } + finishElement(expression); + } + + private void addBareCall(PsiMethodCallExpression expression) { addConditionalRuntimeThrow(); - List contracts = - method instanceof PsiMethod ? getMethodCallContracts((PsiMethod)method, expression) : Collections.emptyList(); + PsiMethod method = expression.resolveMethod(); + List contracts = method == null ? Collections.emptyList() : getMethodCallContracts(method, expression); addInstruction(new MethodCallInstruction(expression, myFactory.createValue(expression), contracts)); if (contracts.stream().anyMatch(c -> c.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION)) { // if a contract resulted in 'fail', handle it @@ -1389,23 +1439,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (!myTrapStack.isEmpty()) { addMethodThrows(expression.resolveMethod(), expression); } - - if (isEqualsCall) { - // assume equals argument must be not-null if the result is true - // don't assume the call result to be false if arg1==null - - // stack: .., arg1, call-result - ConditionalGotoInstruction ifFalse = addInstruction(new ConditionalGotoInstruction(null, true, null)); - - addInstruction(new ApplyNotNullInstruction(expression)); - addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null)); - addInstruction(new GotoInstruction(getEndOffset(expression))); - - ifFalse.setOffset(myCurrentFlow.getInstructionCount()); - addInstruction(new PopInstruction()); - addInstruction(new PushInstruction(myFactory.getConstFactory().getFalse(), null)); - } - finishElement(expression); } public static List getMethodCallContracts(@NotNull final PsiMethod method, @@ -1650,5 +1683,209 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitClass(PsiClass aClass) { } + static final CallInliner[] INLINERS = {new OptionalChainInliner(), new LambdaInliner()}; + + /** + * A facade for building control flow graph used by {@link CallInliner} implementations + */ + public static class CFGBuilder { + private final ControlFlowAnalyzer myAnalyzer; + private final Deque myBranches = new ArrayDeque<>(); + + CFGBuilder(ControlFlowAnalyzer analyzer) { + myAnalyzer = analyzer; + } + + public CFGBuilder pushUnknown() { + myAnalyzer.pushUnknown(); + return this; + } + + public CFGBuilder pushNull() { + myAnalyzer.addInstruction(new PushInstruction(myAnalyzer.myFactory.getConstFactory().getNull(), null)); + return this; + } + + public CFGBuilder pushExpression(PsiExpression expression) { + expression.accept(myAnalyzer); + return this; + } + + public CFGBuilder pushVariable(PsiVariable variable) { + myAnalyzer.addInstruction( + new PushInstruction(myAnalyzer.myFactory.getVarFactory().createVariableValue(variable, false), null, true)); + return this; + } + + public CFGBuilder push(DfaValue value) { + myAnalyzer.addInstruction(new PushInstruction(value, null)); + return this; + } + + public CFGBuilder pop() { + myAnalyzer.addInstruction(new PopInstruction()); + return this; + } + + public CFGBuilder dup() { + myAnalyzer.addInstruction(new DupInstruction()); + return this; + } + + public CFGBuilder splice(int count, int... replacement) { + myAnalyzer.addInstruction(new SpliceInstruction(count, replacement)); + return this; + } + + public CFGBuilder swap() { + myAnalyzer.addInstruction(new SwapInstruction()); + return this; + } + + public CFGBuilder invoke(PsiMethodCallExpression call) { + myAnalyzer.addBareCall(call); + return this; + } + + public CFGBuilder ifConditionIs(boolean value) { + ConditionalGotoInstruction gotoInstruction = new ConditionalGotoInstruction(null, value, null); + myBranches.add(gotoInstruction); + myAnalyzer.addInstruction(gotoInstruction); + return this; + } + + public CFGBuilder endIf() { + myBranches.removeLast().setOffset(myAnalyzer.myCurrentFlow.getInstructionCount()); + return this; + } + + private CFGBuilder compare(IElementType relation) { + myAnalyzer.addInstruction(new BinopInstruction(relation, null, myAnalyzer.myProject)); + return this; + } + + public CFGBuilder elseBranch() { + GotoInstruction gotoInstruction = new GotoInstruction(null); + myAnalyzer.addInstruction(gotoInstruction); + endIf(); + myBranches.add(gotoInstruction); + return this; + } + + public CFGBuilder ifCondition(IElementType relation) { + return compare(relation).ifConditionIs(true); + } + + public CFGBuilder ifNotNull() { + return pushNull().ifCondition(JavaTokenType.NE); + } + + public CFGBuilder ifNull() { + return pushNull().ifCondition(JavaTokenType.EQEQ); + } + + public CFGBuilder boxUnbox(PsiExpression expression, PsiType expectedType) { + myAnalyzer.generateBoxingUnboxingInstructionFor(expression, expectedType); + return this; + } + + public CFGBuilder assign() { + myAnalyzer.addInstruction(new AssignInstruction(null, null)); + return this; + } + + public CFGBuilder assignTo(PsiVariable var) { + return pushVariable(var).swap().assign(); + } + + public DfaValueFactory getFactory() { + return myAnalyzer.myFactory; + } + + /** + * Generates instructions to invoke functional expression (inlining it if possible) which + * consumes given amount of stack arguments + * + * @param argCount number of stack arguments to consume + * @param functionalExpression a functional expression to invoke + * @return this builder + */ + public CFGBuilder invokeFunction(int argCount, @Nullable PsiExpression functionalExpression) { + PsiExpression stripped = PsiUtil.skipParenthesizedExprDown(functionalExpression); + if (stripped instanceof PsiTypeCastExpression) { + stripped = ((PsiTypeCastExpression)stripped).getOperand(); + } + if (stripped instanceof PsiLambdaExpression) { + PsiLambdaExpression lambda = (PsiLambdaExpression)stripped; + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + if (parameters.length == argCount && lambda.getBody() != null) { + StreamEx.ofReversed(parameters).forEach(p -> assignTo(p).pop()); + return inlineLambda(lambda); + } + } + if (stripped instanceof PsiMethodReferenceExpression) { + PsiMethodReferenceExpression methodRef = (PsiMethodReferenceExpression)stripped; + JavaResolveResult resolveResult = methodRef.advancedResolve(false); + PsiMethod method = ObjectUtils.tryCast(resolveResult.getElement(), PsiMethod.class); + if (method != null) { + // TODO: advanced method references support, including contracts + splice(argCount); + pushExpression(methodRef); + pop(); + PsiSubstitutor substitutor = resolveResult.getSubstitutor(); + PsiType returnType = substitutor.substitute(method.getReturnType()); + if (returnType != null) { + push(getFactory().createTypeValue(returnType, DfaPsiUtil.getElementNullability(returnType, method))); + myAnalyzer.generateBoxingUnboxingInstructionFor(methodRef, returnType, LambdaUtil.getFunctionalInterfaceReturnType(methodRef)); + } + else { + pushUnknown(); + } + return this; + } + } + splice(argCount); + if (functionalExpression == null) { + pushUnknown(); + return this; + } + pushExpression(functionalExpression); + pop(); // TODO: handle deference + PsiType returnType = LambdaUtil.getFunctionalInterfaceReturnType(functionalExpression.getType()); + if (returnType != null) { + push(getFactory().createTypeValue(returnType, DfaPsiUtil.getTypeNullability(returnType))); + } + else { + pushUnknown(); + } + return this; + } + + public CFGBuilder inlineLambda(PsiLambdaExpression lambda) { + PsiLambdaExpression oldLambda = myAnalyzer.myLambdaExpression; + myAnalyzer.myLambdaExpression = lambda; + myAnalyzer.startElement(lambda); + try { + PsiElement body = lambda.getBody(); + Objects.requireNonNull(body).accept(myAnalyzer); + if (body instanceof PsiCodeBlock) { + pushUnknown(); // return value for void or incomplete lambda + } + else if (body instanceof PsiExpression) { + boxUnbox((PsiExpression)body, LambdaUtil.getFunctionalInterfaceReturnType(lambda)); + } + } + finally { + myAnalyzer.finishElement(lambda); + myAnalyzer.myLambdaExpression = oldLambda; + } + return this; + } + + public PsiParameter createTempVariable(PsiType type) { + return JavaPsiFacade.getElementFactory(myAnalyzer.myProject) + .createParameter("tmp$" + myAnalyzer.myCurrentFlow.getInstructionCount(), type); + } + } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CallInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CallInliner.java new file mode 100644 index 000000000000..8196af9d5494 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CallInliner.java @@ -0,0 +1,35 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.dataFlow.inliner; + +import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.psi.PsiMethodCallExpression; + +/** + * A CallInliner can recognize specific method calls and inline their implementation into current CFG + */ +public interface CallInliner { + /** + * Try to inline the supplied call + * + * @param builder a builder to use for inlining. Current state is before given method call (call arguments and qualifier are not + * handled yet). + * @param call a call to inline + * @return true if inlining is successful. In this case subsequent inliners are skipped and default processing is omitted. + * If false is returned, inliner must not emit any instructions via builder. + */ + boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call); +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/LambdaInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/LambdaInliner.java new file mode 100644 index 000000000000..fd60506834f9 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/LambdaInliner.java @@ -0,0 +1,52 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.dataFlow.inliner; + +import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; +import one.util.streamex.EntryStream; + +/** + * An inliner which is capable to inline a call like ((IntSupplier)(() -> 5)).getAsInt() to the lambda body. + * Works even if lambda body is complex, has several returns, etc. + */ +public class LambdaInliner implements CallInliner { + @Override + public boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + PsiMethod method = call.resolveMethod(); + if (method == null || method != LambdaUtil.getFunctionalInterfaceMethod(method.getContainingClass())) return false; + PsiTypeCastExpression typeCastExpression = ObjectUtils + .tryCast(PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression()), PsiTypeCastExpression.class); + if (typeCastExpression == null) return false; + PsiLambdaExpression lambda = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(typeCastExpression.getOperand()), PsiLambdaExpression.class); + if (lambda == null || lambda.getBody() == null) return false; + if (method.isVarArgs()) return false; // TODO: support varargs + PsiExpression[] args = call.getArgumentList().getExpressions(); + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + if (args.length != parameters.length) return false; + EntryStream.zip(args, parameters).forKeyValue((arg, parameter) -> + builder.pushVariable(parameter) + .pushExpression(arg) + .boxUnbox(arg, parameter.getType()) + .assign() + .pop()); + builder.inlineLambda(lambda); + return true; + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java new file mode 100644 index 000000000000..c37fda998050 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/OptionalChainInliner.java @@ -0,0 +1,236 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.dataFlow.inliner; + +import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.codeInspection.dataFlow.Nullness; +import com.intellij.codeInspection.dataFlow.value.DfaOptionalValue; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; +import com.siyeh.ig.callMatcher.CallMatcher; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.Contract; + +import static com.intellij.psi.CommonClassNames.JAVA_UTIL_OPTIONAL; + +/** + * An inliner which is capable to inline some Optional chains like + * {@code Optional.of(xyz).map(lambda).filter(lambda).flatMap(lambda).orElseGet(lambda)} + *

+ * TODO support Guava optional + * TODO support primitive Optionals + */ +public class OptionalChainInliner implements CallInliner { + static final CallMatcher OPTIONAL_OR_ELSE = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "orElse").parameterCount(1); + static final CallMatcher OPTIONAL_OR_ELSE_GET = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "orElseGet").parameterCount(1); + static final CallMatcher OPTIONAL_OR = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "or").parameterCount(1); // Java 9 + static final CallMatcher OPTIONAL_IF_PRESENT = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "ifPresent").parameterCount(1); + static final CallMatcher OPTIONAL_FILTER = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "filter").parameterCount(1); + static final CallMatcher OPTIONAL_MAP = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "map").parameterCount(1); + static final CallMatcher OPTIONAL_FLAT_MAP = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "flatMap").parameterCount(1); + static final CallMatcher OPTIONAL_OF = CallMatcher.staticCall(JAVA_UTIL_OPTIONAL, "of", "ofNullable").parameterCount(1); + static final CallMatcher OPTIONAL_EMPTY = CallMatcher.staticCall(JAVA_UTIL_OPTIONAL, "empty").parameterCount(0); + + @Override + public boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + if (OPTIONAL_OR_ELSE.test(call)) { + PsiExpression qualifierExpression = call.getMethodExpression().getQualifierExpression(); + if (!pushOptionalValue(builder, PsiUtil.skipParenthesizedExprDown(qualifierExpression))) return false; + inlineOrElse(builder, call); + return true; + } + if (OPTIONAL_OR_ELSE_GET.test(call)) { + PsiExpression qualifierExpression = call.getMethodExpression().getQualifierExpression(); + if (!pushOptionalValue(builder, PsiUtil.skipParenthesizedExprDown(qualifierExpression))) return false; + builder.dup() + .ifNull() + .pop() + .invokeFunction(0, call.getArgumentList().getExpressions()[0]) + .endIf(); + return true; + } + if (OPTIONAL_IF_PRESENT.test(call)) { + PsiExpression qualifierExpression = call.getMethodExpression().getQualifierExpression(); + if (!pushOptionalValue(builder, PsiUtil.skipParenthesizedExprDown(qualifierExpression))) return false; + builder.dup() + .ifNotNull() + .invokeFunction(0, call.getArgumentList().getExpressions()[0]) + .elseBranch() + .pop() + .pushUnknown() + .endIf(); + } + if (pushIntermediateOperationValue(builder, call)) { + builder.ifNotNull() + .push(builder.getFactory().getOptionalFactory().getOptional(true)) + .elseBranch() + .push(builder.getFactory().getOptionalFactory().getOptional(false)) + .endIf(); + return true; + } + return false; + } + + @Contract("null -> null") + private static PsiType getOptionalElementType(PsiExpression expression) { + if (expression == null) return null; + return PsiUtil.substituteTypeParameter(expression.getType(), JAVA_UTIL_OPTIONAL, 0, false); + } + + private static boolean pushOptionalValue(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression expression) { + PsiType optionalElementType = getOptionalElementType(expression); + if (optionalElementType == null) return false; + if (expression instanceof PsiMethodCallExpression) { + PsiMethodCallExpression qualifierCall = (PsiMethodCallExpression)expression; + if (OPTIONAL_OF.test(qualifierCall)) { + inlineOf(builder, optionalElementType, qualifierCall); + builder.assignTo(builder.createTempVariable(optionalElementType)); + return true; + } + if (OPTIONAL_EMPTY.test(qualifierCall)) { + builder.pushNull(); + return true; + } + if (pushIntermediateOperationValue(builder, qualifierCall)) { + builder.assignTo(builder.createTempVariable(optionalElementType)); + return true; + } + } + // TODO: handle dereference + DfaOptionalValue presentOptional = builder.getFactory().getOptionalFactory().getOptional(true); + builder + .pushExpression(expression) + .push(presentOptional) + .ifCondition(JavaTokenType.INSTANCEOF_KEYWORD) + .push(builder.getFactory().createTypeValue(optionalElementType, Nullness.NOT_NULL)) + .elseBranch() + .pushNull() + .endIf() + .assignTo(builder.createTempVariable(optionalElementType)); + return true; + } + + private static boolean pushIntermediateOperationValue(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + boolean isFilter = OPTIONAL_FILTER.test(call); + boolean isMap = OPTIONAL_MAP.test(call); + boolean isFlatMap = OPTIONAL_FLAT_MAP.test(call); + boolean isOr = OPTIONAL_OR.test(call); + if (!isFilter && !isMap && !isFlatMap && !isOr) return false; + PsiExpression argument = call.getArgumentList().getExpressions()[0]; + PsiExpression qualifierExpression = call.getMethodExpression().getQualifierExpression(); + if (!pushOptionalValue(builder, PsiUtil.skipParenthesizedExprDown(qualifierExpression))) return false; + if (isFlatMap) { + inlineFlatMap(builder, argument); + } + else if (isFilter) { + inlineFilter(builder, argument); + } + else if (isMap) { + inlineMap(builder, argument); + } + else { + inlineOr(builder, argument); + } + return true; + } + + private static void invokeAndUnwrapOptional(ControlFlowAnalyzer.CFGBuilder builder, + int argCount, + PsiExpression function) { + PsiLambdaExpression lambda = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(function), PsiLambdaExpression.class); + if (lambda != null) { + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + PsiExpression lambdaBody = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); + if (parameters.length == argCount && lambdaBody != null) { + StreamEx.ofReversed(parameters).forEach(p -> builder.assignTo(p).pop()); + if(pushOptionalValue(builder, lambdaBody)) { + return; + } + } + } + builder + .pushExpression(function) + .pop() // TODO: handle dereference + .pushUnknown(); + } + + private static void inlineFlatMap(ControlFlowAnalyzer.CFGBuilder builder, + PsiExpression function) { + builder + .dup() + .ifNotNull(); + invokeAndUnwrapOptional(builder, 1, function); + builder.endIf(); + } + + private static void inlineOr(ControlFlowAnalyzer.CFGBuilder builder, + PsiExpression function) { + builder + .dup() + .ifNull() + .pop(); + invokeAndUnwrapOptional(builder, 0, function); + builder.endIf(); + } + + private static void inlineMap(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression function) { + builder + .dup() + .ifNotNull() + .invokeFunction(1, function) + .endIf(); + } + + private static void inlineFilter(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression function) { + builder.dup() + .ifNotNull() + .dup() + .invokeFunction(1, function) + .ifConditionIs(false) + .pop() + .pushNull() + .endIf() + .endIf(); + } + + private static void inlineOf(ControlFlowAnalyzer.CFGBuilder builder, PsiType optionalElementType, PsiMethodCallExpression qualifierCall) { + PsiExpression argument = qualifierCall.getArgumentList().getExpressions()[0]; + builder.pushExpression(argument) + .boxUnbox(argument, optionalElementType) + .pushUnknown() // ... arg, ? + .splice(2, 1, 0, 1) // ... arg, ?, arg + .invoke(qualifierCall) // ... arg, opt -- keep original call in CFG so some warnings like "ofNullable for null" can work + .pop(); // ... arg + if ("of".equals(qualifierCall.getMethodExpression().getReferenceName())) { + builder.dup() + .ifNull() + .pop() + .pushUnknown() + .endIf(); + } + } + private static void inlineOrElse(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + PsiExpression argument = call.getArgumentList().getExpressions()[0]; + builder.pushExpression(argument) // stack: .. optValue, elseValue + .boxUnbox(argument, call.getType()) + .splice(2, 0, 1, 1) // stack: .. elseValue, optValue, optValue + .ifNotNull() + .swap() // stack: .. optValue, elseValue + .endIf() + .pop(); + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ConditionalGotoInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ConditionalGotoInstruction.java index 287b86b89d29..1caa02e3265f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ConditionalGotoInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ConditionalGotoInstruction.java @@ -20,7 +20,7 @@ package com.intellij.codeInspection.dataFlow.instructions; import com.intellij.codeInspection.dataFlow.*; import com.intellij.psi.PsiElement; -public class ConditionalGotoInstruction extends BranchingInstruction { +public class ConditionalGotoInstruction extends BranchingInstruction implements JumpInstruction { private ControlFlow.ControlFlowOffset myOffset; private final boolean myIsNegated; @@ -43,10 +43,12 @@ public class ConditionalGotoInstruction extends BranchingInstruction { return (isNegated() ? "!":"") + "cond?_goto " + getOffset(); } + @Override public int getOffset() { return myOffset.getInstructionOffset(); } + @Override public void setOffset(final int offset) { myOffset = new ControlFlow.ControlFlowOffset() { @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/GotoInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/GotoInstruction.java index b4d2eafd1c61..b09862ef7162 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/GotoInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/GotoInstruction.java @@ -19,13 +19,14 @@ package com.intellij.codeInspection.dataFlow.instructions; import com.intellij.codeInspection.dataFlow.*; -public class GotoInstruction extends Instruction { +public class GotoInstruction extends Instruction implements JumpInstruction { private ControlFlow.ControlFlowOffset myOffset; public GotoInstruction(ControlFlow.ControlFlowOffset myOffset) { this.myOffset = myOffset; } + @Override public int getOffset() { return myOffset.getInstructionOffset(); } @@ -40,6 +41,7 @@ public class GotoInstruction extends Instruction { return "GOTO: " + getOffset(); } + @Override public void setOffset(final int offset) { myOffset = new ControlFlow.ControlFlowOffset() { @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/JumpInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/JumpInstruction.java new file mode 100644 index 000000000000..dd44eb1c3f37 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/JumpInstruction.java @@ -0,0 +1,22 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.dataFlow.instructions; + +public interface JumpInstruction { + int getOffset(); + + void setOffset(final int offset); +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/SpliceInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/SpliceInstruction.java new file mode 100644 index 000000000000..3f84a41446dc --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/SpliceInstruction.java @@ -0,0 +1,51 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.dataFlow.instructions; + +import com.intellij.codeInspection.dataFlow.DataFlowRunner; +import com.intellij.codeInspection.dataFlow.DfaInstructionState; +import com.intellij.codeInspection.dataFlow.DfaMemoryState; +import com.intellij.codeInspection.dataFlow.InstructionVisitor; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import one.util.streamex.IntStreamEx; + +import java.util.Arrays; +import java.util.List; + +/** + * Pop several elements from the stack and replace them with some of them (possibly duplicating, swapping, removing some, etc.) + */ +public class SpliceInstruction extends Instruction { + private final int myCount; + private final int[] myReplacement; + + public SpliceInstruction(int count, int... replacement) { + myCount = count; + myReplacement = replacement; + } + + @Override + public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) { + List removed = IntStreamEx.range(myCount).mapToObj(idx -> stateBefore.pop()).toList(); + IntStreamEx.of(myReplacement).elements(removed).forEach(stateBefore::push); + Instruction nextInstruction = runner.getInstruction(getIndex() + 1); + return new DfaInstructionState[]{new DfaInstructionState(nextInstruction, stateBefore)}; + } + + public String toString() { + return "SPLICE [" + myCount + "] -> " + Arrays.toString(myReplacement); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LambdaInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/LambdaInlining.java new file mode 100644 index 000000000000..44195d512a4a --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LambdaInlining.java @@ -0,0 +1,15 @@ +import java.util.function.IntSupplier; + +public class LambdaInlining { + void testLambdaInline() { + int x = ((IntSupplier) (() -> { + if (Math.random() > 0.5) { + return 4; + } + return 5; + })).getAsInt(); + if (x == 6) { + System.out.println("oops"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java new file mode 100644 index 000000000000..12ff3c5f1c63 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -0,0 +1,131 @@ +import org.jetbrains.annotations.Nullable; +import java.util.Optional; + +public class OptionalInlining { + void testOrElse() { + String s = Optional.ofNullable("foo").orElse("bar"); + if (s.equals("bar")) { + System.out.println("Never"); + } + String s2 = Optional.ofNullable(null).orElse("bar"); + if (s2.equals("bar")) { + System.out.println("Always"); + } + String s3 = Optional.of(Math.random() > 0.5 ? "foo" : "baz").orElse("bar"); + if (s3.equals("foo") || s3.equals("baz")) { + System.out.println("Always"); + } + if (s3.equals("bar")) { + System.out.println("Never"); + } + } + + void testIsPresent(Optional opt) { + if (!opt.isPresent() && opt.orElse("foo").equals("bar")) { + + } + } + + void testDeref(Optional opt) { + if (opt == null) { + System.out.println(opt.orElse("qq")); // Must warn + } + } + + void testOrElseGet(Optional opt) { + String s = opt.orElseGet(() -> { + if (Math.random() > 0.5) { + return "foo"; + } + return "baz"; + }); + if (s.equals("bar") && !opt.isPresent()) { + System.out.println("Impossible"); + } + } + + void testFilter(Optional opt, Optional intOpt) { + Integer integer = intOpt.filter(x -> x > 5).filter(x -> x == 5).orElse(0); + String s1 = opt.filter(s -> false).filter(s -> s.equals("barr")).orElse("baz"); + if (s1.equals("xz")) { + System.out.println("never"); + } + String abc = opt.filter(s -> s == "xyz").orElse("abc"); // s.equals("xyz") does not work yet :( + if (abc.equals("123")) { + System.out.println("never"); + } + if (abc.equals("xyz") && opt.isPresent()) { + System.out.println("always"); + } + } + + @Nullable + String nullableMethod() { + if(Math.random() > 0.5) { + return null; + } + return ""; + } + + @Nullable + Object getObj(String s) { + return new Object(); + } + + void testMap(Optional opt) { + String res = opt.map(s -> null).orElse("abc"); + if (!res.equals("abc")) { + System.out.println("Never"); + } + String trimmed = Optional.ofNullable(nullableMethod()).map(xx -> xx.trim()).orElse(""); + if(trimmed == null) { + System.out.println("impossible"); + } + String xyz = nullableMethod(); + Object n = Optional.ofNullable(xyz).map(String::trim).map(this::getObj).orElse(null); + if(n instanceof Integer) { + // n instanceof Integer -> n is not null -> xyz was not null -> safe to dereference + System.out.println(xyz.trim()); + } + xyz.trim(); + } + + void testFlatMap(Optional opt) { + String s = opt.flatMap(str -> Optional.of(str.length() > 10 ? "foo" : "bar")).orElse("baz"); + if (s.equals("qux")) { + System.out.println("Never"); + } + } + + void testIfPresent(Optional opt) { + opt.map(s -> s.isEmpty() ? 5 : 6).ifPresent(val -> { + if (val == 7) { + System.out.println("oops"); + } + }); + } + + void testIntermediate(Optional opt) { + if ( x == \"bar\").isPresent()' is always 'false'">opt.filter(x -> x == "foo").filter(x -> x == "bar").isPresent()) { + System.out.println("never"); + } + } + + void test174759(Optional a, Optional b) { + if (a.isPresent() || b.isPresent()) { + // prefer a over b + Integer result = a.map(s -> s + "0").map(s -> Integer.parseInt(s)) + .orElseGet(() -> Integer.parseInt(b.get())); // <-- no more warning for b.get() + System.out.println(result); + } + } + + void test174759MethodRef(Optional a, Optional b) { + if (a.isPresent() || b.isPresent()) { + // prefer a over b + Integer result = a.map(s -> s + "0").map(Integer::parseInt) + .orElseGet(() -> Integer.parseInt(b.get())); // <-- no more warning for b.get() + System.out.println(result); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index 44249b44e796..6a5f3ea86b56 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -160,6 +160,9 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { doTest(); } + public void testLambdaInlining() { doTest(); } + public void testOptionalInlining() { doTest(); } + public void testMethodVsExpressionTypeAnnotationConflict() { setupCustomAnnotations("withTypeUse", "{ElementType.METHOD, ElementType.TYPE_USE}", myFixture); doTest();