From 130a641011bc1e07a0a77a8719f19a7c2e92d02b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 11 Aug 2017 15:44:16 +0700 Subject: [PATCH] Better methodref support in DFA; Stream.generate() inlining support --- .../codeInspection/dataFlow/CFGBuilder.java | 12 +++-- .../dataFlow/CustomMethodHandlers.java | 13 ++++- .../dataFlow/DataFlowInspectionBase.java | 3 +- .../dataFlow/StandardInstructionVisitor.java | 8 +++- .../dataFlow/inliner/StreamChainInliner.java | 48 +++++++++++++------ .../dataFlow/value/DfaVariableValue.java | 4 +- .../fixture/LongRangeKnownMethods.java | 4 +- .../dataFlow/fixture/OptionalInlining.java | 4 +- .../dataFlow/fixture/StreamInlining.java | 12 ++++- .../com/siyeh/ig/callMatcher/CallHandler.java | 5 ++ .../com/siyeh/ig/callMatcher/CallMapper.java | 20 ++++++-- 11 files changed, 98 insertions(+), 35 deletions(-) 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 index 20cf48748526..07864c3e16c1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java @@ -449,12 +449,10 @@ public class CFGBuilder { PsiVariable qualifierBinding = createTempVariable(qualifier.getType()); pushVariable(qualifierBinding) .pushExpression(qualifier) - .dup(); - myAnalyzer.addInstruction(new FieldReferenceInstruction(qualifier, ControlFlowAnalyzer.METHOD_REFERENCE_QUALIFIER_SYNTHETIC_FIELD)); - assign().pop(); + .checkNotNull(qualifier, NullabilityProblem.fieldAccessNPE) + .assign() + .pop(); myMethodRefQualifiers.put(methodRef, qualifierBinding); - } else { - pushExpression(methodRef).pop(); } return this; } @@ -599,4 +597,8 @@ public class CFGBuilder { operation.accept(this); return this; } + + public static boolean isTempVariable(PsiModifierListOwner variable) { + return variable instanceof LightVariableBuilder && ((LightVariableBuilder)variable).getName().startsWith("tmp$"); + } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java index 64747a49dc3d..21c418092902 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CustomMethodHandlers.java @@ -15,10 +15,13 @@ */ package com.intellij.codeInspection.dataFlow; +import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction; import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; +import com.intellij.psi.PsiElement; import com.intellij.psi.PsiMethodCallExpression; +import com.intellij.psi.PsiMethodReferenceExpression; import com.intellij.util.ArrayUtil; import com.intellij.util.ObjectUtils; import com.siyeh.ig.callMatcher.CallMapper; @@ -67,8 +70,14 @@ public class CustomMethodHandlers { .register(staticCall(JAVA_LANG_MATH, "abs").parameterTypes("long"), (args, memState, factory) -> mathAbs(args.myArguments, memState, factory, true)); - public static CustomMethodHandler find(PsiMethodCallExpression call) { - return CUSTOM_METHOD_HANDLERS.mapFirst(call); + public static CustomMethodHandler find(MethodCallInstruction instruction) { + PsiElement context = instruction.getContext(); + if(context instanceof PsiMethodCallExpression) { + return CUSTOM_METHOD_HANDLERS.mapFirst((PsiMethodCallExpression)context); + } else if(context instanceof PsiMethodReferenceExpression) { + return CUSTOM_METHOD_HANDLERS.mapFirst((PsiMethodReferenceExpression)context); + } + return null; } private static List stringStartsEnds(DfaCallArguments args, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 9f8dcbee6a6f..85c457eb2077 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -1043,7 +1043,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { protected void processMethodReferenceResult(PsiMethodReferenceExpression methodRef, List contracts, DfaValue res) { - if(contracts.stream().anyMatch(c -> !c.isTrivial())) { + if(contracts.isEmpty() || !contracts.get(0).isTrivial()) { // Do not track if method reference may have different results myMethodReferenceResults.merge(methodRef, res, (a, b) -> a == b ? a : DfaUnknownValue.getInstance()); } @@ -1056,6 +1056,7 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } private static boolean hasNonTrivialBooleanContracts(MethodCallInstruction instruction) { + if (CustomMethodHandlers.find(instruction) != null) return true; List contracts = instruction.getContracts(); return !contracts.isEmpty() && contracts.stream().anyMatch( contract -> (contract.getReturnValue() == MethodContract.ValueConstraint.FALSE_VALUE || diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f51ec9dcdffc..daf76c2797d4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -320,12 +320,17 @@ public class StandardInstructionVisitor extends InstructionVisitor { } } + PsiMethodReferenceExpression methodRef = instruction.getMethodType() == MethodCallInstruction.MethodType.METHOD_REFERENCE_CALL ? + (PsiMethodReferenceExpression)instruction.getContext() : null; DfaInstructionState[] result = new DfaInstructionState[finalStates.size()]; int i = 0; for (DfaMemoryState state : finalStates) { if (instruction.shouldFlushFields()) { state.flushFields(); } + if (methodRef != null) { + processMethodReferenceResult(methodRef, instruction.getContracts(), state.peek()); + } result[i++] = new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), state); } return result; @@ -333,8 +338,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { @NotNull private List handleKnownMethods(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class); - CustomMethodHandlers.CustomMethodHandler handler = CustomMethodHandlers.find(call); + CustomMethodHandlers.CustomMethodHandler handler = CustomMethodHandlers.find(instruction); if (handler == null) return Collections.emptyList(); DfaCallArguments callArguments = popCall(instruction, runner, memState, false); List states = diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java index 4f753a861118..163b30e4b904 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java @@ -31,10 +31,8 @@ import org.jetbrains.annotations.NotNull; import java.util.function.UnaryOperator; -import static com.intellij.psi.CommonClassNames.JAVA_UTIL_STREAM_BASE_STREAM; -import static com.intellij.psi.CommonClassNames.JAVA_UTIL_STREAM_STREAM; -import static com.siyeh.ig.callMatcher.CallMatcher.anyOf; -import static com.siyeh.ig.callMatcher.CallMatcher.instanceCall; +import static com.intellij.psi.CommonClassNames.*; +import static com.siyeh.ig.callMatcher.CallMatcher.*; public class StreamChainInliner implements CallInliner { private static final String[] TERMINALS = @@ -58,6 +56,12 @@ public class StreamChainInliner implements CallInliner { instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "flatMap", "flatMapToInt", "flatMapToLong", "flatMapToDouble").parameterCount(1); private static final CallMatcher PEEK = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "peek").parameterCount(1); + private static final CallMatcher STREAM_GENERATE = anyOf( + staticCall(JAVA_UTIL_STREAM_STREAM, "generate").parameterCount(1), + staticCall(JAVA_UTIL_STREAM_INT_STREAM, "generate").parameterCount(1), + staticCall(JAVA_UTIL_STREAM_LONG_STREAM, "generate").parameterCount(1), + staticCall(JAVA_UTIL_STREAM_DOUBLE_STREAM, "generate").parameterCount(1)); + private static final CallMapper> INTERMEDIATE_STEP_MAPPER = new CallMapper>() .register(FILTER, (PsiMethodCallExpression call) -> (Step next) -> new FilterStep(call, next)) .register(MAP, (PsiMethodCallExpression call) -> (Step next) -> new MapStep(call, next)) @@ -310,17 +314,31 @@ public class StreamChainInliner implements CallInliner { static void buildStreamCFG(CFGBuilder builder, Step firstStep, PsiExpression originalQualifier) { PsiType inType = StreamApiUtil.getStreamElementType(originalQualifier.getType()); - builder - .pushExpression(originalQualifier) - .checkNotNull(firstStep.myCall, NullabilityProblem.callNPE) - .pop() - .chain(firstStep::before) - .doWhile() - .pushVariable(builder.createTempVariable(inType)) - .push(builder.getFactory().createTypeValue(inType, DfaPsiUtil.getTypeNullability(inType))) - .assign() - .chain(firstStep::iteration) - .endWhileUnknown(); + PsiMethodCallExpression sourceCall = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(originalQualifier), PsiMethodCallExpression.class); + if(STREAM_GENERATE.test(sourceCall)) { + PsiExpression fn = sourceCall.getArgumentList().getExpressions()[0]; + builder + .evaluateFunction(fn) + .chain(firstStep::before) + .doWhile() + .pushVariable(builder.createTempVariable(inType)) + .invokeFunction(0, fn) + .assign() + .chain(firstStep::iteration) + .endWhileUnknown(); + } else { + builder + .pushExpression(originalQualifier) + .checkNotNull(firstStep.myCall, NullabilityProblem.callNPE) + .pop() + .chain(firstStep::before) + .doWhile() + .pushVariable(builder.createTempVariable(inType)) + .push(builder.getFactory().createTypeValue(inType, DfaPsiUtil.getTypeNullability(inType))) + .assign() + .chain(firstStep::iteration) + .endWhileUnknown(); + } } static Step buildChain(PsiMethodCallExpression qualifierCall, Step terminalStep) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index 790801df6ab7..e0407bdb9583 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -157,7 +157,9 @@ public class DfaVariableValue extends DfaValue { } public boolean isFlushableByCalls() { - if (myVariable instanceof PsiLocalVariable || myVariable instanceof PsiParameter) return false; + if (myVariable instanceof PsiLocalVariable || myVariable instanceof PsiParameter || CFGBuilder.isTempVariable(myVariable)) { + return false; + } boolean finalField = myVariable instanceof PsiVariable && myVariable.hasModifierProperty(PsiModifier.FINAL); boolean specialFinalField = myVariable instanceof PsiMethod && Arrays.stream(SpecialField.values()).anyMatch(sf -> sf.isFinal() && sf.isMyAccessor(myVariable)); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java index c76ecc37c07b..54d1a5634956 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LongRangeKnownMethods.java @@ -153,8 +153,8 @@ public class LongRangeKnownMethods { void testStringComparison(String name) { // Parentheses misplaced -- found in AndroidStudio - if (!(name.equals("layout_width") && !(name.equals("layout_height")) && - !(name.equals("id")))) { + if (!(name.equals("layout_width") && !(name.equals("layout_height")) && + !(name.equals("id")))) { System.out.println("ok"); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java index e99427140f83..21d57a61fbf7 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -12,7 +12,7 @@ public class OptionalInlining { System.out.println("Always"); } String s3 = Optional.of(Math.random() > 0.5 ? "foo" : "baz").orElse("bar"); - if (s3.equals("foo") || s3.equals("baz")) { + if (s3.equals("foo") || s3.equals("baz")) { System.out.println("Always"); } if (s3.equals("bar")) { @@ -111,7 +111,7 @@ public class OptionalInlining { void testMap(Optional opt) { opt.map(null); String res = opt.map(s -> null).orElse("abc"); - if (!res.equals("abc")) { + if (!res.equals("abc")) { System.out.println("Never"); } String trimmed = Optional.ofNullable(nullableMethod()).map(xx -> xx.trim()).orElse(""); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java index 21b1558f12be..45632aed7f55 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamInlining.java @@ -2,8 +2,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; -import java.util.stream.IntStream; -import java.util.stream.Stream; +import java.util.stream.*; public class StreamInlining { void testNulls(List list) { @@ -26,6 +25,7 @@ public class StreamInlining { if(list.stream().filter(Objects::nonNull).anyMatch(x -> x == null)) { System.out.println("never"); } + list.stream().map(Integer::valueOf).filter(Objects::nonNull).forEach(System.out::println); } void filterDistinctLimitSkipFilter(List list) { @@ -118,4 +118,12 @@ public class StreamInlining { Optional testOptionalOfNullable(List list) { return list.stream().filter(Objects::isNull).map(Optional::ofNullable).findFirst().orElse(Optional.empty()); } + + void testGenerate() { + List list1 = Stream.generate(() -> Math.random() > 0.5 ? "foo" : "baz") + .limit(10).filter((xyz -> "bar".equals(xyz))).collect(Collectors.toList()); + List list2 = Stream.generate(() -> "xyz").limit(20).filter("bar"::equals).collect(Collectors.toList()); + Stream.generate(() -> Optional.of("xyz")).filter(Optional::isPresent).forEach(System.out::println); + LongStream.generate(() -> 5).limit(10).filter(x -> x > 6).forEach(s -> System.out.println(s)); + } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallHandler.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallHandler.java index edaba0d0e2eb..408ea108dcf6 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallHandler.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallHandler.java @@ -16,6 +16,7 @@ package com.siyeh.ig.callMatcher; import com.intellij.psi.PsiMethodCallExpression; +import com.intellij.psi.PsiMethodReferenceExpression; import java.util.function.Function; @@ -46,6 +47,10 @@ public class CallHandler implements Function { return matcher().test(call) ? myTransformer.apply(call) : null; } + public T applyMethodReference(PsiMethodReferenceExpression ref) { + return matcher().methodReferenceMatches(ref) ? myTransformer.apply(null) : null; + } + /** * Creates a new CallHandler with specific matcher and specific transformer function * @param matcher a matcher to be applied to the elements diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallMapper.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallMapper.java index 468f2bd123c4..3eacaa8e6d19 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallMapper.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/callMatcher/CallMapper.java @@ -16,6 +16,7 @@ package com.siyeh.ig.callMatcher; import com.intellij.psi.PsiMethodCallExpression; +import com.intellij.psi.PsiMethodReferenceExpression; import one.util.streamex.StreamEx; import java.util.ArrayList; @@ -31,7 +32,7 @@ import java.util.stream.Stream; * @author Tagir Valeev */ public class CallMapper { - private Map>> myMap = new HashMap<>(); + private Map>> myMap = new HashMap<>(); public CallMapper() {} @@ -61,7 +62,7 @@ public class CallMapper { public T mapFirst(PsiMethodCallExpression call) { if (call == null) return null; - List> functions = myMap.get(call.getMethodExpression().getReferenceName()); + List> functions = myMap.get(call.getMethodExpression().getReferenceName()); if (functions == null) return null; for (Function function : functions) { T t = function.apply(call); @@ -72,9 +73,22 @@ public class CallMapper { return null; } + public T mapFirst(PsiMethodReferenceExpression methodRef) { + if (methodRef == null) return null; + List> functions = myMap.get(methodRef.getReferenceName()); + if (functions == null) return null; + for (CallHandler function : functions) { + T t = function.applyMethodReference(methodRef); + if (t != null) { + return t; + } + } + return null; + } + public Stream mapAll(PsiMethodCallExpression call) { if (call == null) return null; - List> functions = myMap.get(call.getMethodExpression().getReferenceName()); + List> functions = myMap.get(call.getMethodExpression().getReferenceName()); if (functions == null) return StreamEx.empty(); return StreamEx.of(functions).map(fn -> fn.apply(call)).nonNull(); }