From addebb5e34585ff7ce331fbd4176cb0cc6bea708 Mon Sep 17 00:00:00 2001 From: "Maxim.Medvedev" Date: Wed, 21 Mar 2012 14:10:40 +0400 Subject: [PATCH] IDEA-82839 Skip too complex methods in Groovy DFA analysis --- .../plugins/groovy/GroovyBundle.properties | 2 ++ .../groovy/annotator/GroovyAnnotator.java | 21 ++++++++++++++ .../unusedDef/UnusedDefInspection.java | 6 +++- .../utils/ControlFlowUtils.java | 16 +++++----- .../groovy/lang/psi/dataFlow/DFAEngine.java | 23 ++++++++++++++- .../ReachingDefinitionsCollector.java | 29 ++++++++++--------- .../lang/psi/impl/TypeInferenceHelper.java | 11 +++++-- 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties index c3735295a072..57c820df5558 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties @@ -302,3 +302,5 @@ method.with.type.parameters.should.have.return.type=Method with type parameters primitive.type.parameters.are.not.allowed=Primitive type parameters are not allowed primitive.bound.types.are.not.allowed=Primitive bound types are not allowed ellipsis.type.is.not.allowed.here=Ellipsis type is not allowed here +method.0.is.too.complex.too.analyze=Method ''{0}'' is too complex to analyze.\nTypes of local variables are not inferred. +closure.is.too.complex.to.analyze=Closure is complex to analyze.\nTypes of local variables are not inferred. diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java index 3a81c5db8a1d..a9ff9c221209 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -99,6 +99,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.imports.GrImportStatem import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.packaging.GrPackageDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.types.*; import org.jetbrains.plugins.groovy.lang.psi.api.util.GrVariableDeclarationOwner; +import org.jetbrains.plugins.groovy.lang.psi.impl.TypeInferenceHelper; import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GrLightParameter; import org.jetbrains.plugins.groovy.lang.psi.impl.synthetic.GroovyScriptClass; import org.jetbrains.plugins.groovy.lang.psi.impl.types.GrClosureSignatureUtil; @@ -532,6 +533,12 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { checkMethodWithTypeParamsShouldHaveReturnType(myHolder, method); checkInnerMethod(myHolder, method); checkMethodParameters(myHolder, method); + + GrOpenBlock block = method.getBlock(); + if (block != null && TypeInferenceHelper.isTooComplexTooAnalyze(block)) { + myHolder.createWeakWarningAnnotation(method.getNameIdentifierGroovy(), GroovyBundle.message("method.0.is.too.complex.too.analyze", + method.getName())); + } } private static void checkMethodWithTypeParamsShouldHaveReturnType(AnnotationHolder holder, GrMethod method) { @@ -936,6 +943,20 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { if (!closure.hasParametersSection() && isClosureAmbiguous(closure)) { myHolder.createErrorAnnotation(closure, GroovyBundle.message("ambiguous.code.block")); } + + if (TypeInferenceHelper.isTooComplexTooAnalyze(closure)) { + int startOffset = closure.getLBrace().getTextRange().getStartOffset(); + int endOffset; + if (closure.getArrow()!=null) { + endOffset = closure.getArrow().getTextRange().getEndOffset(); + } + else { + String text = + PsiDocumentManager.getInstance(closure.getProject()).getDocument(closure.getContainingFile()).getText(); + endOffset = Math.min(closure.getTextRange().getEndOffset(), text.indexOf('\n', startOffset)); + } + myHolder.createWeakWarningAnnotation(new TextRange(startOffset, endOffset), GroovyBundle.message("closure.is.too.complex.to.analyze")); + } } private static boolean isClosureAmbiguous(GrClosableBlock closure) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/unusedDef/UnusedDefInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/unusedDef/UnusedDefInspection.java index da1bd028bac2..4bb6ad43dfae 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/unusedDef/UnusedDefInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/unusedDef/UnusedDefInspection.java @@ -83,7 +83,11 @@ public class UnusedDefInspection extends GroovyLocalInspectionBase { final ReachingDefinitionsDfaInstance dfaInstance = new ReachingDefinitionsDfaInstance(flow); final ReachingDefinitionsSemilattice lattice = new ReachingDefinitionsSemilattice(); final DFAEngine> engine = new DFAEngine>(flow, dfaInstance, lattice); - final List> dfaResult = engine.performDFA(); + final List> dfaResult = engine.performDFAWithTimeout(); + if (dfaResult == null) { + return; + } + final TIntHashSet unusedDefs = new TIntHashSet(); for (Instruction instruction : flow) { if (instruction instanceof ReadWriteVariableInstruction && ((ReadWriteVariableInstruction) instruction).isWrite()) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java index 18fb9b913a0c..bd663678c098 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java @@ -153,17 +153,13 @@ public class ControlFlowUtils { if (openBlockMayCompleteNormally(tryBlock)) { return true; } - final GrCatchClause[] catchClauses = tryStatement.getCatchClauses(); - if (catchClauses != null) { - - for (GrCatchClause catchClause : catchClauses) { - if (openBlockMayCompleteNormally(catchClause.getBody())) { - return true; - } + for (GrCatchClause catchClause : tryStatement.getCatchClauses()) { + if (openBlockMayCompleteNormally(catchClause.getBody())) { + return true; } - } + return false; } @@ -650,6 +646,7 @@ public class ControlFlowUtils { place = place.getContext(); } while (true) { + assert place != null; place = place.getContext(); if (place == null) return null; if (place instanceof GrClosableBlock) return (GrClosableBlock)place; @@ -740,6 +737,7 @@ public class ControlFlowUtils { }); } + @NotNull public static ArrayList inferWriteAccessMap(final Instruction[] flow, final GrVariable var) { final Semilattice sem = new Semilattice() { @@ -790,7 +788,7 @@ public class ControlFlowUtils { } }; - return new DFAEngine(flow, dfa, sem).performDFA(); + return new DFAEngine(flow, dfa, sem).performForceDFA(); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/DFAEngine.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/DFAEngine.java index 72e975a3b555..0db8157ec869 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/DFAEngine.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/DFAEngine.java @@ -16,6 +16,8 @@ package org.jetbrains.plugins.groovy.lang.psi.dataFlow; import com.intellij.openapi.progress.ProgressManager; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.CallEnvironment; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.CallInstruction; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.ControlFlowBuilderUtil; @@ -27,6 +29,8 @@ import java.util.*; * @author ven */ public class DFAEngine { + private static final long ourTimeLimit = 1000; + private final Instruction[] myFlow; private final DfaInstance myDfa; @@ -57,7 +61,22 @@ public class DFAEngine { } } - public ArrayList performDFA() { + @NotNull + public ArrayList performForceDFA() { + ArrayList result = performDFA(false); + assert result != null; + return result; + } + + @Nullable + public ArrayList performDFAWithTimeout() { + return performDFA(true); + } + + @Nullable + private ArrayList performDFA(boolean timeout) { + long startTime = System.currentTimeMillis(); + ArrayList info = new ArrayList(myFlow.length); CallEnvironment env = new MyCallEnvironment(myFlow.length); for (int i = 0; i < myFlow.length; i++) { @@ -78,6 +97,8 @@ public class DFAEngine { visited[instr.num()] = true; while (!workList.isEmpty()) { + if (timeout && System.currentTimeMillis() - startTime > ourTimeLimit) return null; + ProgressManager.checkCanceled(); final Instruction curr = workList.remove(); final int num = curr.num(); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/reachingDefs/ReachingDefinitionsCollector.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/reachingDefs/ReachingDefinitionsCollector.java index ad34f71449f4..4ebd0621742f 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/reachingDefs/ReachingDefinitionsCollector.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/dataFlow/reachingDefs/ReachingDefinitionsCollector.java @@ -53,6 +53,7 @@ public class ReachingDefinitionsCollector { private ReachingDefinitionsCollector() { } + @NotNull public static FragmentVariableInfos obtainVariableFlowInformation(final GrStatement first, final GrStatement last) { GroovyPsiElement context = PsiTreeUtil.getParentOfType(first, GrMethod.class, GrClosableBlock.class, GroovyFileBase.class, GrClassInitializer.class); GrControlFlowOwner flowOwner; @@ -66,7 +67,7 @@ public class ReachingDefinitionsCollector { final ReachingDefinitionsDfaInstance dfaInstance = new ReachingDefinitionsDfaInstance(flow); final ReachingDefinitionsSemilattice lattice = new ReachingDefinitionsSemilattice(); final DFAEngine> engine = new DFAEngine>(flow, dfaInstance, lattice); - final TIntObjectHashMap dfaResult = postprocess(engine.performDFA(), flow, dfaInstance); + final TIntObjectHashMap dfaResult = postprocess(engine.performForceDFA(), flow, dfaInstance); final LinkedHashSet fragmentInstructions = getFragmentInstructions(first, last, flow); final int[] postorder = ControlFlowBuilderUtil.postorder(flow); @@ -149,16 +150,15 @@ public class ReachingDefinitionsCollector { } String name = variable.getName(); - if (name != null) { - if (!(variable instanceof GrField)) { - if (!isInFragment(first, last, resolved)) { - if (isInFragment(first, last, closure)) { - addVariable(name, imap, variable.getManager(), variable.getType()); - } - } else { - if (!isInFragment(first, last, closure)) { - addVariable(name, omap, variable.getManager(), variable.getType()); - } + if (!(variable instanceof GrField)) { + if (!isInFragment(first, last, resolved)) { + if (isInFragment(first, last, closure)) { + addVariable(name, imap, variable.getManager(), variable.getType()); + } + } + else { + if (!isInFragment(first, last, closure)) { + addVariable(name, omap, variable.getManager(), variable.getType()); } } } @@ -306,7 +306,7 @@ public class ReachingDefinitionsCollector { final StringBuffer buffer = new StringBuffer(); for (int i = 0; i < dfaResult.size(); i++) { TIntObjectHashMap map = dfaResult.get(i); - buffer.append("At " + i + ":\n"); + buffer.append("At ").append(i).append(":\n"); map.forEachEntry(new TIntObjectProcedure() { public boolean execute(int i, TIntHashSet defs) { buffer.append(i).append(" -> "); @@ -366,7 +366,10 @@ public class ReachingDefinitionsCollector { } } - private static TIntObjectHashMap postprocess(final ArrayList> dfaResult, Instruction[] flow, ReachingDefinitionsDfaInstance dfaInstance) { + @NotNull + private static TIntObjectHashMap postprocess(@NotNull final ArrayList> dfaResult, + @NotNull Instruction[] flow, + @NotNull ReachingDefinitionsDfaInstance dfaInstance) { TIntObjectHashMap result = new TIntObjectHashMap(); for (int i = 0; i < flow.length; i++) { Instruction insn = flow[i]; diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/TypeInferenceHelper.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/TypeInferenceHelper.java index edc9c60ee6c4..ddcbd868464c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/TypeInferenceHelper.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/TypeInferenceHelper.java @@ -89,6 +89,10 @@ public class TypeInferenceHelper { return getInferredType(variableName, instruction, flow, scope); } + public static boolean isTooComplexTooAnalyze(GrControlFlowOwner scope) { + return getDefUseMaps(scope).second == null; + } + @Nullable private static Instruction findInstructionAt(PsiElement place, Instruction[] flow) { List applicable = new ArrayList(); @@ -131,7 +135,10 @@ public class TypeInferenceHelper { final Pair>> pair = getDefUseMaps(scope); final int varIndex = pair.first.getVarIndex(varName); - final TIntObjectHashMap allDefs = pair.second.get(instruction.num()); + List> second = pair.second; + if (second == null) return null; + + final TIntObjectHashMap allDefs = second.get(instruction.num()); final TIntHashSet varDefs = allDefs.get(varIndex); if (varDefs == null) return null; @@ -174,7 +181,7 @@ public class TypeInferenceHelper { }; final ReachingDefinitionsSemilattice lattice = new ReachingDefinitionsSemilattice(); final DFAEngine> engine = new DFAEngine>(flow, dfaInstance, lattice); - final List> dfaResult = engine.performDFA(); + final List> dfaResult = engine.performDFAWithTimeout(); return Result.create(Pair.create(dfaInstance, dfaResult), PsiModificationTracker.MODIFICATION_COUNT); } });