From d29aa4f8ddd4212bfad112d10a511c082df13c1c Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 11 Jul 2019 10:58:34 +0700 Subject: [PATCH] IDEA-218061 Better support for string concatenation in dataflow GitOrigin-RevId: 8c29e3469c062b9125a3fe8f8e45d96a9390c67e --- .../dataFlow/CustomMethodHandlers.java | 2 +- .../dataFlow/StandardInstructionVisitor.java | 45 ++++++++++++++++--- .../dataFlow/fixture/StringConcat.java | 16 +++++++ .../DataFlowRangeAnalysisTest.java | 1 + 4 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/StringConcat.java 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 1c9b59a9931e..e6fea10ba4f2 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 @@ -40,7 +40,7 @@ class CustomMethodHandlers { staticCall(JAVA_LANG_BYTE, "toString").parameterTypes("byte"), staticCall(JAVA_LANG_SHORT, "toString").parameterTypes("short") ); - private static final int MAX_STRING_CONSTANT_LENGTH_TO_TRACK = 1024; + static final int MAX_STRING_CONSTANT_LENGTH_TO_TRACK = 256; interface CustomMethodHandler { 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 ddf50e9d3c9f..7b5feebd9f3b 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 @@ -12,7 +12,6 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.ObjectUtils; import com.intellij.util.ThreeState; import com.siyeh.ig.psiutils.MethodUtils; import com.siyeh.ig.psiutils.TypeUtils; @@ -22,6 +21,8 @@ import org.jetbrains.annotations.Nullable; import java.util.*; +import static com.intellij.util.ObjectUtils.tryCast; + /** * @author peter */ @@ -199,7 +200,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { PsiMethod sam = LambdaUtil.getFunctionalInterfaceMethod(functionalInterfaceType); if (sam == null || PsiType.VOID.equals(sam.getReturnType())) return; JavaResolveResult resolveResult = methodRef.advancedResolve(false); - PsiMethod method = ObjectUtils.tryCast(resolveResult.getElement(), PsiMethod.class); + PsiMethod method = tryCast(resolveResult.getElement(), PsiMethod.class); if (method == null || !JavaMethodContractUtil.isPure(method)) return; List contracts = JavaMethodContractUtil.getMethodCallContracts(method, null); PsiSubstitutor substitutor = resolveResult.getSubstitutor(); @@ -283,8 +284,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { beforeMethodCall(instruction.getExpression(), callArguments, runner, memState); } - Set finalStates = new LinkedHashSet<>(); - finalStates.addAll(handleKnownMethods(instruction, runner, memState, callArguments)); + Set finalStates = new LinkedHashSet<>(handleKnownMethods(instruction, runner, memState, callArguments)); if (finalStates.isEmpty()) { Set currentStates = Collections.singleton(new DfaCallState(memState, callArguments)); @@ -673,7 +673,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { result = runner.getFactory().getBinOpFactory().create(dfaLeft, dfaRight, memState, isLong, opSign); } if (result == DfaUnknownValue.getInstance() && JavaTokenType.PLUS == opSign && TypeUtils.isJavaLangString(type)) { - result = runner.getFactory().createTypeValue(type, Nullability.NOT_NULL); + result = concatStrings(dfaLeft, dfaRight, memState, type, runner.getFactory()); } pushExpressionResult(result, instruction, memState); @@ -683,6 +683,39 @@ public class StandardInstructionVisitor extends InstructionVisitor { return nextInstruction(instruction, runner, memState); } + @NotNull + private static DfaValue concatStrings(DfaValue left, + DfaValue right, + DfaMemoryState memState, + PsiType stringType, + DfaValueFactory factory) { + DfaConstValue leftConst = memState.getConstantValue(left); + DfaConstValue rightConst = memState.getConstantValue(right); + if (leftConst != null && rightConst != null) { + String leftString = tryCast(leftConst.getValue(), String.class); + String rightString = tryCast(rightConst.getValue(), String.class); + if (leftString != null && rightString != null && + leftString.length() + rightString.length() <= CustomMethodHandlers.MAX_STRING_CONSTANT_LENGTH_TO_TRACK) { + return factory.getConstFactory().createFromValue(leftString + rightString, stringType); + } + } + DfaValue leftLength = SpecialField.STRING_LENGTH.createValue(factory, left); + DfaValue rightLength = SpecialField.STRING_LENGTH.createValue(factory, right); + LongRangeSet leftRange = memState.getValueFact(leftLength, DfaFactType.RANGE); + LongRangeSet rightRange = memState.getValueFact(rightLength, DfaFactType.RANGE); + DfaFactMap map = DfaFactMap.EMPTY + .with(DfaFactType.TYPE_CONSTRAINT, factory.createDfaType(stringType).asConstraint()) + .with(DfaFactType.NULLABILITY, DfaNullability.NOT_NULL); + if (leftRange != null && rightRange != null) { + LongRangeSet resultRange = leftRange.plus(rightRange, false).intersect(LongRangeSet.indexRange()); + if (!resultRange.equals(LongRangeSet.indexRange())) { + map = map.with(DfaFactType.SPECIAL_FIELD_VALUE, + SpecialField.STRING_LENGTH.withValue(factory.getFactValue(DfaFactType.RANGE, resultRange))); + } + } + return factory.getFactFactory().createValue(map); + } + @Nullable private DfaInstructionState[] handleRelationBinop(BinopInstruction instruction, DataFlowRunner runner, @@ -772,7 +805,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { DfaValue condition = null; if (instruction.isClassObjectCheck()) { DfaConstValue constant = memState.getConstantValue(dfaRight); - PsiType type = constant == null ? null : ObjectUtils.tryCast(constant.getValue(), PsiType.class); + PsiType type = constant == null ? null : tryCast(constant.getValue(), PsiType.class); if (type == null || type instanceof PsiPrimitiveType) { // Unknown/primitive class: just execute contract "null -> false" DfaConstValue aNull = factory.getConstFactory().getNull(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StringConcat.java b/java/java-tests/testData/inspection/dataFlow/fixture/StringConcat.java new file mode 100644 index 000000000000..b31c61336843 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StringConcat.java @@ -0,0 +1,16 @@ +import java.util.*; + +public class StringConcat { + void test(String s1, String s2) { + if (s1.equals("foo") && s2.equals("bar")) { + String res = s1 + s2; + if (res.equals("foobar")) { + + } + } + if (s1.startsWith("foo") && s2.startsWith("bar")) { + String res = s1 + s2; + if (res.length() < 6) {} + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java index 6b9b93268f1b..9d7a217ba4f3 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java @@ -61,4 +61,5 @@ public class DataFlowRangeAnalysisTest extends DataFlowInspectionTestCase { public void testModRange() { doTest(); } public void testBackPropagationMod() { doTest(); } public void testArithmeticNoOp() { doTest(); } + public void testStringConcat() { doTest(); } }