From efa7ff250985710971b837c52eff1b4d1acde2e2 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 26 Jul 2017 14:04:45 +0700 Subject: [PATCH] CFG inliners: fixes according to review IDEA-CR-22449 1. CFGBuilder moved as top-level class 2. Complex path in DupInstruction removed (replaced with SpliceInstruction) 3. test174759 renamed to testTwoOptionalInteraction 4. PsiUtil.deparenthesizeExpression used 5. OptionalChainInliner: made constants private 6. LightVariableBuilder used to create temporary variables --- .../codeInspection/dataFlow/CFGBuilder.java | 228 +++++++++++++++ .../dataFlow/ControlFlowAnalyzer.java | 264 +++--------------- .../dataFlow/inliner/CallInliner.java | 5 +- .../inliner/CollectionFactoryInliner.java | 9 +- .../dataFlow/inliner/LambdaInliner.java | 5 +- .../inliner/OptionalChainInliner.java | 45 +-- .../dataFlow/instructions/DupInstruction.java | 32 +-- .../dataFlow/fixture/OptionalInlining.java | 5 +- 8 files changed, 304 insertions(+), 289 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java new file mode 100644 index 000000000000..bfe3003a0f88 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java @@ -0,0 +1,228 @@ +/* + * 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; + +import com.intellij.codeInspection.dataFlow.inliner.CallInliner; +import com.intellij.codeInspection.dataFlow.instructions.*; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; +import com.intellij.psi.*; +import com.intellij.psi.impl.light.LightVariableBuilder; +import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayDeque; +import java.util.Deque; + +/** + * A facade for building control flow graph used by {@link CallInliner} implementations + */ +public 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(getFactory().getConstFactory().getNull(), null)); + return this; + } + + public CFGBuilder pushExpression(PsiExpression expression) { + expression.accept(myAnalyzer); + return this; + } + + public CFGBuilder pushVariable(PsiVariable variable) { + myAnalyzer.addInstruction( + new PushInstruction(getFactory().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 dereferenceCheck(PsiReferenceExpression referenceExpression) { + if (referenceExpression != null) { + myAnalyzer.addInstruction(new DupInstruction()); + myAnalyzer.addInstruction(new FieldReferenceInstruction(referenceExpression, null)); + } + 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.getInstructionCount()); + return this; + } + + private CFGBuilder compare(IElementType relation) { + myAnalyzer.addInstruction(new BinopInstruction(relation, null, myAnalyzer.getContext().getProject())); + 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 checkNotNull(PsiExpression expression) { + myAnalyzer.addInstruction(new CheckNotNullInstruction(expression)); + 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.getFactory(); + } + + /** + * 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.deparenthesizeExpression(functionalExpression); + 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); + checkNotNull(functionalExpression); + pop(); + 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) { + myAnalyzer.inlineLambda(lambda); + return this; + } + + public PsiVariable createTempVariable(PsiType type) { + return new LightVariableBuilder<>("tmp$" + myAnalyzer.getInstructionCount(), type, myAnalyzer.getContext()); + } +} 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 679656c97057..f87f8f2a1ce0 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 @@ -39,7 +39,6 @@ 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; @@ -102,6 +101,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { return myCurrentFlow; } + DfaValueFactory getFactory() { + return myFactory; + } + + PsiElement getContext() { + return myCodeFragment; + } private PsiClassType createClassType(GlobalSearchScope scope, String fqn) { PsiClass aClass = JavaPsiFacade.getInstance(myProject).findClass(fqn, scope); @@ -109,11 +115,15 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { return JavaPsiFacade.getElementFactory(myProject).createTypeByFQClassName(fqn, scope); } - private T addInstruction(T i) { + T addInstruction(T i) { myCurrentFlow.addInstruction(i); return i; } + int getInstructionCount() { + return myCurrentFlow.getInstructionCount(); + } + private ControlFlow.ControlFlowOffset getEndOffset(PsiElement element) { return myCurrentFlow.getEndOffset(element); } @@ -1136,11 +1146,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } - private void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression expression, PsiType expectedType) { + void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression expression, PsiType expectedType) { generateBoxingUnboxingInstructionFor(expression, expression.getType(), expectedType); } - private void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression context, PsiType actualType, PsiType expectedType) { + void generateBoxingUnboxingInstructionFor(@NotNull PsiExpression context, PsiType actualType, PsiType expectedType) { if (PsiType.VOID.equals(expectedType)) return; if (TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isPrimitiveWrapper(actualType)) { @@ -1304,7 +1314,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } - private void pushUnknown() { + void pushUnknown() { addInstruction(new PushInstruction(DfaUnknownValue.getInstance(), null)); } @@ -1390,11 +1400,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (i == 0 && isEqualsCall) { // stack: .., qualifier, arg1 - addInstruction(new SwapInstruction()); - // stack: .., arg1, qualifier - addInstruction(new DupInstruction(2, 1)); - // stack: .., arg1, qualifier, arg1, qualifier - addInstruction(new PopInstruction()); + addInstruction(new SpliceInstruction(2, 0, 1, 0)); // stack: .., arg1, qualifier, arg1 } } @@ -1419,7 +1425,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } - private void addBareCall(PsiMethodCallExpression expression) { + void addBareCall(PsiMethodCallExpression expression) { addConditionalRuntimeThrow(); PsiMethod method = expression.resolveMethod(); List contracts = method == null ? Collections.emptyList() : getMethodCallContracts(method, expression); @@ -1684,227 +1690,31 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitClass(PsiClass aClass) { } - static final CallInliner[] INLINERS = {new OptionalChainInliner(), new LambdaInliner(), new CollectionFactoryInliner()}; - - /** - * 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 dereferenceCheck(PsiReferenceExpression referenceExpression) { - if (referenceExpression != null) { - myAnalyzer.addInstruction(new DupInstruction()); - myAnalyzer.addInstruction(new FieldReferenceInstruction(referenceExpression, null)); - } - 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 checkNotNull(PsiExpression expression) { - myAnalyzer.addInstruction(new CheckNotNullInstruction(expression)); - 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); - checkNotNull(functionalExpression); - pop(); - PsiType returnType = LambdaUtil.getFunctionalInterfaceReturnType(functionalExpression.getType()); - if (returnType != null) { - push(getFactory().createTypeValue(returnType, DfaPsiUtil.getTypeNullability(returnType))); - } - else { + void inlineLambda(PsiLambdaExpression lambda) { + PsiLambdaExpression oldLambda = this.myLambdaExpression; + this.myLambdaExpression = lambda; + startElement(lambda); + // Transfer value is pushed to avoid emptying stack beyond this point + addInstruction(new PushInstruction(this.myFactory.controlTransfer(ReturnTransfer.INSTANCE, this.myTrapStack), null)); + try { + PsiElement body = lambda.getBody(); + Objects.requireNonNull(body).accept(this); + if (body instanceof PsiCodeBlock) { + // return value for void or incomplete lambda pushUnknown(); } - return this; - } - - public CFGBuilder inlineLambda(PsiLambdaExpression lambda) { - PsiLambdaExpression oldLambda = myAnalyzer.myLambdaExpression; - myAnalyzer.myLambdaExpression = lambda; - myAnalyzer.startElement(lambda); - // Transfer value is pushed to avoid emptying stack beyond this point - push(getFactory().controlTransfer(ReturnTransfer.INSTANCE, myAnalyzer.myTrapStack)); - 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)); - } + else if (body instanceof PsiExpression) { + generateBoxingUnboxingInstructionFor((PsiExpression)body, LambdaUtil.getFunctionalInterfaceReturnType(lambda)); } - finally { - // Pop transfer value (which is second value in stack now) - splice(2, 0); - myAnalyzer.finishElement(lambda); - myAnalyzer.myLambdaExpression = oldLambda; - } - return this; } - - public PsiParameter createTempVariable(PsiType type) { - return JavaPsiFacade.getElementFactory(myAnalyzer.myProject) - .createParameter("tmp$" + myAnalyzer.myCurrentFlow.getInstructionCount(), type); + finally { + // Pop transfer value (which is second value in stack now) + addInstruction(new SpliceInstruction(2, 0)); + finishElement(lambda); + this.myLambdaExpression = oldLambda; } } + + static final CallInliner[] INLINERS = {new OptionalChainInliner(), new LambdaInliner(), new CollectionFactoryInliner()}; } 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 index 8196af9d5494..d31ff96315b0 100644 --- 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 @@ -15,8 +15,9 @@ */ package com.intellij.codeInspection.dataFlow.inliner; -import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.codeInspection.dataFlow.CFGBuilder; import com.intellij.psi.PsiMethodCallExpression; +import org.jetbrains.annotations.NotNull; /** * A CallInliner can recognize specific method calls and inline their implementation into current CFG @@ -31,5 +32,5 @@ public interface CallInliner { * @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); + boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java index dc355923e203..5644994dd80d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java @@ -15,16 +15,17 @@ */ package com.intellij.codeInspection.dataFlow.inliner; -import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.codeInspection.dataFlow.CFGBuilder; import com.intellij.codeInspection.dataFlow.Nullness; import com.intellij.codeInspection.dataFlow.SpecialField; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiMethodCallExpression; -import com.intellij.psi.PsiParameter; import com.intellij.psi.PsiType; +import com.intellij.psi.PsiVariable; import com.siyeh.ig.callMatcher.CallMapper; +import org.jetbrains.annotations.NotNull; import static com.intellij.codeInspection.dataFlow.SpecialField.COLLECTION_SIZE; import static com.intellij.codeInspection.dataFlow.SpecialField.MAP_SIZE; @@ -49,14 +50,14 @@ public class CollectionFactoryInliner implements CallInliner { .register(staticCall(JAVA_UTIL_COLLECTIONS, "singletonMap").parameterCount(2), new FactoryInfo(1, MAP_SIZE)); @Override - public boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { FactoryInfo factoryInfo = STATIC_FACTORIES.mapFirst(call); if (factoryInfo == null) return false; PsiExpression[] args = call.getArgumentList().getExpressions(); for (PsiExpression arg : args) { builder.pushExpression(arg).pop(); } - PsiParameter variable = builder.createTempVariable(call.getType()); + PsiVariable variable = builder.createTempVariable(call.getType()); DfaValueFactory factory = builder.getFactory(); DfaVariableValue variableValue = factory.getVarFactory().createVariableValue(variable, false); builder.pushVariable(variable) // tmpVar = 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 index fd60506834f9..2e3d2add58ef 100644 --- 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 @@ -15,11 +15,12 @@ */ package com.intellij.codeInspection.dataFlow.inliner; -import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.codeInspection.dataFlow.CFGBuilder; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; import one.util.streamex.EntryStream; +import org.jetbrains.annotations.NotNull; /** * An inliner which is capable to inline a call like ((IntSupplier)(() -> 5)).getAsInt() to the lambda body. @@ -27,7 +28,7 @@ import one.util.streamex.EntryStream; */ public class LambdaInliner implements CallInliner { @Override - public boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { PsiMethod method = call.resolveMethod(); if (method == null || method != LambdaUtil.getFunctionalInterfaceMethod(method.getContainingClass())) return false; PsiTypeCastExpression typeCastExpression = ObjectUtils 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 index c194471538ce..4d1df652e57c 100644 --- 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 @@ -15,7 +15,7 @@ */ package com.intellij.codeInspection.dataFlow.inliner; -import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; +import com.intellij.codeInspection.dataFlow.CFGBuilder; import com.intellij.codeInspection.dataFlow.Nullness; import com.intellij.codeInspection.dataFlow.value.DfaOptionalValue; import com.intellij.psi.*; @@ -25,6 +25,7 @@ import com.siyeh.ig.callMatcher.CallMapper; import com.siyeh.ig.callMatcher.CallMatcher; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; import java.util.function.BiConsumer; @@ -38,18 +39,18 @@ import static com.intellij.psi.CommonClassNames.JAVA_UTIL_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); + private static final CallMatcher OPTIONAL_OR_ELSE = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "orElse").parameterCount(1); + private static final CallMatcher OPTIONAL_OR_ELSE_GET = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "orElseGet").parameterCount(1); + private static final CallMatcher OPTIONAL_OR = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "or").parameterCount(1); // Java 9 + private static final CallMatcher OPTIONAL_IF_PRESENT = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "ifPresent").parameterCount(1); + private static final CallMatcher OPTIONAL_FILTER = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "filter").parameterCount(1); + private static final CallMatcher OPTIONAL_MAP = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "map").parameterCount(1); + private static final CallMatcher OPTIONAL_FLAT_MAP = CallMatcher.instanceCall(JAVA_UTIL_OPTIONAL, "flatMap").parameterCount(1); + private static final CallMatcher OPTIONAL_OF = CallMatcher.staticCall(JAVA_UTIL_OPTIONAL, "of", "ofNullable").parameterCount(1); + private static final CallMatcher OPTIONAL_EMPTY = CallMatcher.staticCall(JAVA_UTIL_OPTIONAL, "empty").parameterCount(0); - static final CallMapper> TERMINAL_MAPPER = - new CallMapper>() + private static final CallMapper> TERMINAL_MAPPER = + new CallMapper>() .register(OPTIONAL_OR_ELSE, (builder, call) -> { PsiExpression argument = call.getArgumentList().getExpressions()[0]; builder.pushExpression(argument) // stack: .. optValue, elseValue @@ -74,8 +75,8 @@ public class OptionalChainInliner implements CallInliner { .endIf()); @Override - public boolean tryInlineCall(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { - BiConsumer terminalInliner = TERMINAL_MAPPER.mapFirst(call); + public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { + BiConsumer terminalInliner = TERMINAL_MAPPER.mapFirst(call); if (terminalInliner != null) { PsiExpression qualifierExpression = call.getMethodExpression().getQualifierExpression(); if (!pushOptionalValue(builder, PsiUtil.skipParenthesizedExprDown(qualifierExpression), call.getMethodExpression())) return false; @@ -99,7 +100,7 @@ public class OptionalChainInliner implements CallInliner { return PsiUtil.substituteTypeParameter(expression.getType(), JAVA_UTIL_OPTIONAL, 0, false); } - private static boolean pushOptionalValue(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression expression, + private static boolean pushOptionalValue(CFGBuilder builder, PsiExpression expression, PsiReferenceExpression dereferencer) { PsiType optionalElementType = getOptionalElementType(expression); if (optionalElementType == null) return false; @@ -133,7 +134,7 @@ public class OptionalChainInliner implements CallInliner { return true; } - private static boolean pushIntermediateOperationValue(ControlFlowAnalyzer.CFGBuilder builder, PsiMethodCallExpression call) { + private static boolean pushIntermediateOperationValue(CFGBuilder builder, PsiMethodCallExpression call) { boolean isFilter = OPTIONAL_FILTER.test(call); boolean isMap = OPTIONAL_MAP.test(call); boolean isFlatMap = OPTIONAL_FLAT_MAP.test(call); @@ -157,7 +158,7 @@ public class OptionalChainInliner implements CallInliner { return true; } - private static void invokeAndUnwrapOptional(ControlFlowAnalyzer.CFGBuilder builder, + private static void invokeAndUnwrapOptional(CFGBuilder builder, int argCount, PsiExpression function) { PsiLambdaExpression lambda = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(function), PsiLambdaExpression.class); @@ -178,7 +179,7 @@ public class OptionalChainInliner implements CallInliner { .pushUnknown(); } - private static void inlineFlatMap(ControlFlowAnalyzer.CFGBuilder builder, + private static void inlineFlatMap(CFGBuilder builder, PsiExpression function) { builder .dup() @@ -187,7 +188,7 @@ public class OptionalChainInliner implements CallInliner { builder.endIf(); } - private static void inlineOr(ControlFlowAnalyzer.CFGBuilder builder, + private static void inlineOr(CFGBuilder builder, PsiExpression function) { builder .dup() @@ -197,7 +198,7 @@ public class OptionalChainInliner implements CallInliner { builder.endIf(); } - private static void inlineMap(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression function) { + private static void inlineMap(CFGBuilder builder, PsiExpression function) { builder .dup() .ifNotNull() @@ -205,7 +206,7 @@ public class OptionalChainInliner implements CallInliner { .endIf(); } - private static void inlineFilter(ControlFlowAnalyzer.CFGBuilder builder, PsiExpression function) { + private static void inlineFilter(CFGBuilder builder, PsiExpression function) { builder.dup() .ifNotNull() .dup() @@ -217,7 +218,7 @@ public class OptionalChainInliner implements CallInliner { .endIf(); } - private static void inlineOf(ControlFlowAnalyzer.CFGBuilder builder, PsiType optionalElementType, PsiMethodCallExpression qualifierCall) { + private static void inlineOf(CFGBuilder builder, PsiType optionalElementType, PsiMethodCallExpression qualifierCall) { PsiExpression argument = qualifierCall.getArgumentList().getExpressions()[0]; builder.pushExpression(argument) .boxUnbox(argument, optionalElementType) 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 213a64b512e1..081f4f965442 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 @@ -19,47 +19,19 @@ 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 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) { - 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 + 1; j++) { - for (int i = values.size() - 1; i >= 0; i--) { - memState.push(values.get(i)); - } - } - } + memState.push(memState.peek()); Instruction nextInstruction = runner.getInstruction(getIndex() + 1); return new DfaInstructionState[]{new DfaInstructionState(nextInstruction, memState)}; } public String toString() { - return "DUP(" + myValueCount + " top stack values, " + myDuplicationCount + " times)"; + return "DUP"; } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java index f7c54b4ff0b6..d54475778b07 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -116,7 +116,8 @@ public class OptionalInlining { } } - void test174759(Optional a, Optional b) { + // IDEA-174759 + void testTwoOptionalInteraction(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)) @@ -125,7 +126,7 @@ public class OptionalInlining { } } - void test174759MethodRef(Optional a, Optional b) { + void testTwoOptionalInteractionMethodRef(Optional a, Optional b) { if (a.isPresent() || b.isPresent()) { // prefer a over b Integer result = a.map(s -> s + "0").map(Integer::parseInt)