From da3dc03b35b5944e399e998537c3eefde56b0d1c Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Sat, 19 Jan 2013 19:44:29 +0400 Subject: [PATCH] IDEA-99253 Process assignability errors from down to top --- .../codeInspection/ProblemsHolder.java | 6 +- .../groovy/codeInspection/BaseInspection.java | 14 +- .../codeInspection/BaseInspectionVisitor.java | 77 +++----- .../GroovyAssignabilityCheckInspection.java | 171 ++++++++++-------- .../highlighting/GrAssignabilityTest.groovy | 18 +- ...CallMethodWithInapplicableArguments.groovy | 2 +- 6 files changed, 144 insertions(+), 144 deletions(-) diff --git a/platform/lang-api/src/com/intellij/codeInspection/ProblemsHolder.java b/platform/lang-api/src/com/intellij/codeInspection/ProblemsHolder.java index a89139a08f70..027366df9a2a 100644 --- a/platform/lang-api/src/com/intellij/codeInspection/ProblemsHolder.java +++ b/platform/lang-api/src/com/intellij/codeInspection/ProblemsHolder.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2013 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. @@ -209,6 +209,10 @@ public class ProblemsHolder { return !myProblems.isEmpty(); } + public int getResultCount() { + return myProblems.size(); + } + public boolean isOnTheFly() { return myOnTheFly; } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspection.java index 70051c8b2956..772cd0554766 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspection.java @@ -25,9 +25,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase; -import java.util.List; - - public abstract class BaseInspection extends GroovySuppressableInspectionTool { private final String m_shortName = StringUtil.trimEnd(getClass().getSimpleName(), "Inspection"); @@ -54,7 +51,8 @@ public abstract class BaseInspection extends GroovySuppressableInspectionTool { return m_shortName; } - @Nullable BaseInspectionVisitor buildGroovyVisitor(@NotNull ProblemsHolder problemsHolder, boolean onTheFly) { + @NotNull + protected BaseInspectionVisitor buildGroovyVisitor(@NotNull ProblemsHolder problemsHolder, boolean onTheFly) { final BaseInspectionVisitor visitor = buildVisitor(); visitor.setProblemsHolder(problemsHolder); visitor.setOnTheFly(onTheFly); @@ -73,12 +71,12 @@ public abstract class BaseInspection extends GroovySuppressableInspectionTool { } @Nullable - protected GroovyFix buildFix(PsiElement location) { + protected GroovyFix buildFix(@NotNull PsiElement location) { return null; } @Nullable - protected GroovyFix[] buildFixes(PsiElement location) { + protected GroovyFix[] buildFixes(@NotNull PsiElement location) { return null; } @@ -92,10 +90,10 @@ public abstract class BaseInspection extends GroovySuppressableInspectionTool { final ProblemsHolder problemsHolder = new ProblemsHolder(inspectionManager, psiFile, isOnTheFly); final BaseInspectionVisitor visitor = buildGroovyVisitor(problemsHolder, isOnTheFly); groovyFile.accept(visitor); - final List problems = problemsHolder.getResults(); - return problems.toArray(new ProblemDescriptor[problems.size()]); + return problemsHolder.getResultsArray(); } + @NotNull protected abstract BaseInspectionVisitor buildVisitor(); } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java index 45930c21bbe4..6ec3be034fd5 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/BaseInspectionVisitor.java @@ -16,30 +16,25 @@ package org.jetbrains.plugins.groovy.codeInspection; import com.intellij.codeInspection.LocalQuickFix; -import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiModifierList; -import com.intellij.psi.PsiModifierListOwner; -import com.intellij.psi.PsiWhiteSpace; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; -import java.util.List; - public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisitor { private BaseInspection inspection = null; private ProblemsHolder problemsHolder = null; private boolean onTheFly = false; - private final List errors = null; public void setInspection(BaseInspection inspection) { this.inspection = inspection; @@ -63,33 +58,13 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito registerError(statementToken, args); } - - protected void registerVariableError(GrVariable variable) { - final PsiElement nameIdentifier = variable.getNameIdentifierGroovy(); - registerError(nameIdentifier); - } - - protected void registerModifierError(String modifier, - PsiModifierListOwner parameter) { - final PsiModifierList modifiers = parameter.getModifierList(); - if (modifiers == null) { - return; - } - final PsiElement[] children = modifiers.getChildren(); - for (final PsiElement child : children) { - final String text = child.getText(); - if (modifier.equals(text)) { - registerError(child); - } - } - } - protected void registerError(PsiElement location) { if (location == null) { return; } final LocalQuickFix[] fix = createFixes(location); - final String description = inspection.buildErrorString(location); + String description = StringUtil.notNullize(inspection.buildErrorString(location)); + registerError(location, description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } @@ -97,9 +72,10 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito if (method == null) { return; } - final LocalQuickFix[] fix = createFixes(method); - final String description = inspection.buildErrorString(args); - registerError(method.getNameIdentifierGroovy(), description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + final LocalQuickFix[] fixes = createFixes(method); + String description = StringUtil.notNullize(inspection.buildErrorString(args)); + + registerError(method.getNameIdentifierGroovy(), description, fixes, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } protected void registerVariableError(GrVariable variable, Object... args) { @@ -107,7 +83,7 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito return; } final LocalQuickFix[] fix = createFixes(variable); - final String description = inspection.buildErrorString(args); + final String description = StringUtil.notNullize(inspection.buildErrorString(args)); registerError(variable.getNameIdentifierGroovy(), description, fix, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } @@ -115,15 +91,19 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito if (method == null) { return; } - final LocalQuickFix[] fix = createFixes(method); - final String description = inspection.buildErrorString(args); - registerError(((GrReferenceExpression) method.getInvokedExpression()).getReferenceNameElement(), description, fix, - ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + final LocalQuickFix[] fixes = createFixes(method); + final String description = StringUtil.notNullize(inspection.buildErrorString(args)); + + final GrExpression invoked = method.getInvokedExpression(); + assert invoked != null; + final PsiElement nameElement = ((GrReferenceExpression)invoked).getReferenceNameElement(); + assert nameElement != null; + registerError(nameElement, description, fixes, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); } protected void registerError(@NotNull PsiElement location, - String description, - LocalQuickFix[] fixes, + @NotNull String description, + @Nullable LocalQuickFix[] fixes, ProblemHighlightType highlightType) { problemsHolder.registerProblem(location, description, highlightType, fixes); } @@ -136,12 +116,12 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito ProblemHighlightType highlightType, Object... args) { final LocalQuickFix[] fix = createFixes(location); - final String description = inspection.buildErrorString(args); + final String description = StringUtil.notNullize(inspection.buildErrorString(args)); registerError(location, description, fix, highlightType); } @Nullable - private LocalQuickFix[] createFixes(PsiElement location) { + private LocalQuickFix[] createFixes(@NotNull PsiElement location) { if (!onTheFly && inspection.buildQuickFixesOnlyForOnTheFlyErrors()) { return null; @@ -157,18 +137,7 @@ public abstract class BaseInspectionVisitor extends GroovyRecursiveElementVisito return new GroovyFix[]{fix}; } - @Nullable - public ProblemDescriptor[] getErrors() { - if (errors == null) { - return null; - } else { - final int numErrors = errors.size(); - return errors.toArray(new ProblemDescriptor[numErrors]); - } - } - - public void visitWhiteSpace(PsiWhiteSpace space) { - // none of our inspections need to do anything with white space, - // so this is a performance optimization + public int getErrorCount() { + return problemsHolder.getResultCount(); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java index 6b3cc5aa03e2..167ccd04fec8 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2012 JetBrains s.r.o. + * Copyright 2000-2013 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. @@ -17,10 +17,7 @@ package org.jetbrains.plugins.groovy.codeInspection.assignment; import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.codeInspection.InspectionManager; -import com.intellij.codeInspection.LocalQuickFix; -import com.intellij.codeInspection.ProblemDescriptor; -import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.codeInspection.*; import com.intellij.lang.annotation.Annotation; import com.intellij.lang.annotation.AnnotationHolder; import com.intellij.openapi.diagnostic.Logger; @@ -47,13 +44,13 @@ import org.jetbrains.plugins.groovy.extensions.NamedArgumentDescriptor; import org.jetbrains.plugins.groovy.findUsages.LiteralConstructorReference; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.psi.GrControlFlowOwner; +import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrListOrMap; import org.jetbrains.plugins.groovy.lang.psi.api.signatures.GrClosureSignature; import org.jetbrains.plugins.groovy.lang.psi.api.signatures.GrSignature; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrConstructorInvocation; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrField; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable; import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariableDeclaration; import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentList; @@ -66,7 +63,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.literals.GrString; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrIndexProperty; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.path.GrMethodCallExpression; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrTypeDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.*; import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.impl.GrClosureType; @@ -115,6 +111,7 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { return (String)args[0]; } + @NotNull @Override protected BaseInspectionVisitor buildVisitor() { return new MyVisitor(); @@ -163,13 +160,6 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { return true; } - @Override - public void visitMethod(GrMethod method) { - if (!shouldProcess(method)) return; - - super.visitMethod(method); - } - @Override public void visitReturnStatement(GrReturnStatement returnStatement) { super.visitReturnStatement(returnStatement); @@ -196,20 +186,8 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } } - protected boolean shouldProcess(GrMethod method) { - return !GroovyPsiManager.getInstance(method.getProject()).isCompileStatic(method); - } - - @Override - public void visitField(GrField field) { - if (GroovyPsiManager.getInstance(field.getProject()).isCompileStatic(field)) return; - super.visitField(field); - } - - @Override - public void visitTypeDefinition(GrTypeDefinition typeDefinition) { - if (GroovyPsiManager.getInstance(typeDefinition.getProject()).isCompileStatic(typeDefinition)) return; - super.visitTypeDefinition(typeDefinition); + protected boolean shouldProcess(GrMember member) { + return !GroovyPsiManager.getInstance(member.getProject()).isCompileStatic(member); } @Override @@ -537,47 +515,9 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { } @Override - public void visitReferenceExpression(GrReferenceExpression referenceExpression) { - super.visitReferenceExpression(referenceExpression); - - final PsiElement parent = referenceExpression.getParent(); - if (parent instanceof GrCall) { - GrCall call = (GrCall)parent; - GroovyResolveResult resolveResult = call.advancedResolve(); - GroovyResolveResult[] results = call.multiResolve(false); //cached - - PsiElement resolved = resolveResult.getElement(); - if (resolved == null) { - GrExpression qualifier = referenceExpression.getQualifierExpression(); - if (qualifier == null && GrHighlightUtil.isDeclarationAssignment(referenceExpression)) return; - } - - if (!checkCannotInferArgumentTypes(referenceExpression)) return; - - final PsiType type = referenceExpression.getType(); - if (resolved != null) { - if (resolved instanceof PsiMethod && !resolveResult.isInvokedOnProperty()) { - checkMethodApplicability(resolveResult, referenceExpression, true); - } - else { - checkCallApplicability(type, referenceExpression, true); - } - } - else if (results.length > 0) { - for (GroovyResolveResult result : results) { - resolved = result.getElement(); - if (resolved instanceof PsiMethod && !resolveResult.isInvokedOnProperty()) { - if (!checkMethodApplicability(result, referenceExpression, false)) return; - } - else { - if (!checkCallApplicability(type, referenceExpression, false)) return; - } - } - - registerError(getElementToHighlight(referenceExpression, PsiUtil.getArgumentsList(referenceExpression)), - GroovyBundle.message("method.call.is.ambiguous")); - } - } + public void visitIndexProperty(GrIndexProperty expression) { + super.visitIndexProperty(expression); + checkMethodCall(expression, expression.getInvokedExpression()); } private boolean checkCannotInferArgumentTypes(PsiElement referenceExpression) { @@ -591,13 +531,13 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { @Override public void visitMethodCallExpression(GrMethodCallExpression methodCallExpression) { super.visitMethodCallExpression(methodCallExpression); - checkMethodCall(methodCallExpression); + checkMethodCall(methodCallExpression, methodCallExpression.getInvokedExpression()); } @Override public void visitApplicationStatement(GrApplicationStatement applicationStatement) { super.visitApplicationStatement(applicationStatement); - checkMethodCall(applicationStatement); + checkMethodCall(applicationStatement, applicationStatement.getInvokedExpression()); } @Override @@ -650,13 +590,49 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { return false; } - private void checkMethodCall(GrMethodCall call) { + private void checkMethodCall(GrCall call, GrExpression invoked) { if (hasErrorElements(call.getArgumentList())) return; - final GrExpression expression = call.getInvokedExpression(); - if (!(expression instanceof GrReferenceExpression)) { //it checks in visitRefExpr(...) - final PsiType type = expression.getType(); - checkCallApplicability(type, expression, true); + if (invoked instanceof GrReferenceExpression) { + final GrReferenceExpression referenceExpression = (GrReferenceExpression)invoked; + GroovyResolveResult resolveResult = call.advancedResolve(); + GroovyResolveResult[] results = call.multiResolve(false); //cached + + PsiElement resolved = resolveResult.getElement(); + if (resolved == null) { + GrExpression qualifier = referenceExpression.getQualifierExpression(); + if (qualifier == null && GrHighlightUtil.isDeclarationAssignment(referenceExpression)) return; + } + + if (!checkCannotInferArgumentTypes(referenceExpression)) return; + + final PsiType type = referenceExpression.getType(); + if (resolved != null) { + if (resolved instanceof PsiMethod && !resolveResult.isInvokedOnProperty()) { + checkMethodApplicability(resolveResult, referenceExpression, true); + } + else { + checkCallApplicability(type, referenceExpression, true); + } + } + else if (results.length > 0) { + for (GroovyResolveResult result : results) { + resolved = result.getElement(); + if (resolved instanceof PsiMethod && !resolveResult.isInvokedOnProperty()) { + if (!checkMethodApplicability(result, referenceExpression, false)) return; + } + else { + if (!checkCallApplicability(type, referenceExpression, false)) return; + } + } + + registerError(getElementToHighlight(referenceExpression, PsiUtil.getArgumentsList(referenceExpression)), + GroovyBundle.message("method.call.is.ambiguous")); + } + } + else if (invoked != null) { //it checks in visitRefExpr(...) + final PsiType type = invoked.getType(); + checkCallApplicability(type, invoked, true); } checkNamedArgumentsType(call); @@ -908,14 +884,14 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { private AnnotationHolder myHolder; @Override - protected boolean shouldProcess(GrMethod method) { + protected boolean shouldProcess(GrMember member) { return true; } @Override protected void registerError(@NotNull final PsiElement location, - final String description, - final LocalQuickFix[] fixes, + @NotNull final String description, + @NotNull final LocalQuickFix[] fixes, final ProblemHighlightType highlightType) { Annotation annotation = myHolder.createErrorAnnotation(location, description); for (final LocalQuickFix fix : fixes) { @@ -984,4 +960,41 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { annotatingVisitor.myHolder = oldHolder; } } + + @Nullable + public ProblemDescriptor[] checkFile(@NotNull PsiFile psiFile, @NotNull InspectionManager inspectionManager, boolean isOnTheFly) { + if (!(psiFile instanceof GroovyFileBase)) { + return super.checkFile(psiFile, inspectionManager, isOnTheFly); + } + + + final GroovyFileBase groovyFile = (GroovyFileBase)psiFile; + final ProblemsHolder problemsHolder = new ProblemsHolder(inspectionManager, psiFile, isOnTheFly); + final MyVisitor visitor = (MyVisitor)buildGroovyVisitor(problemsHolder, isOnTheFly); + + processElement(groovyFile, visitor); + + return problemsHolder.getResultsArray(); + } + + private static void processElement(@NotNull GroovyPsiElement element, @NotNull MyVisitor visitor) { + if (element instanceof GrMember && !visitor.shouldProcess((GrMember)element)) { + return; + } + + final int count = visitor.getErrorCount(); + + PsiElement child = element.getFirstChild(); + while (child != null) { + if (child instanceof GroovyPsiElement) { + processElement((GroovyPsiElement)child, visitor); + } + child = child.getNextSibling(); + } + + if (count == visitor.getErrorCount()) { + element.accept(visitor); + } + } + } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrAssignabilityTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrAssignabilityTest.groovy index eb872ca5c7ce..4b61775ed25e 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrAssignabilityTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrAssignabilityTest.groovy @@ -1,5 +1,5 @@ /* - * Copyright 2000-2012 JetBrains s.r.o. + * Copyright 2000-2013 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. @@ -469,6 +469,22 @@ def _Boolean(Boolean x) {} _int(null) _boolean(null) _Boolean(null) +''') + } + + void testInnerWarning() { + testHighlighting('''\ +public static void main(String[] args) { + bar (foo(foo(foo('2')))) +} + +static def T foo(T abc) { + abc +} + +static bar(String s) { + +} ''') } } diff --git a/plugins/groovy/testdata/highlighting/ClosureCallMethodWithInapplicableArguments.groovy b/plugins/groovy/testdata/highlighting/ClosureCallMethodWithInapplicableArguments.groovy index be947b78d319..01c677096279 100644 --- a/plugins/groovy/testdata/highlighting/ClosureCallMethodWithInapplicableArguments.groovy +++ b/plugins/groovy/testdata/highlighting/ClosureCallMethodWithInapplicableArguments.groovy @@ -1,6 +1,6 @@ def foo={x, y->} -print foo.call(1) +print foo.call(1) def bar={3} print bar.call()