From aa4a91edae715c3bcace1fda1387269a28790422 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Wed, 7 Sep 2011 12:06:38 +0400 Subject: [PATCH] highlighting optimizations --- .../analysis/HighlightControlFlowUtil.java | 9 +-- .../impl/analysis/HighlightNamesUtil.java | 8 +-- .../daemon/impl/analysis/HighlightUtil.java | 44 +++++------- .../impl/analysis/HighlightVisitorImpl.java | 72 ++++++++++++++----- .../refactoring/util/RefactoringUtil.java | 2 +- 5 files changed, 80 insertions(+), 55 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java index 34db9fd2be34..7289bee48651 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightControlFlowUtil.java @@ -422,8 +422,7 @@ public class HighlightControlFlowUtil { } public static boolean isReassigned(PsiVariable variable, - Map> finalVarProblems, - Map parameterIsReassigned) { + Map> finalVarProblems) { if (variable instanceof PsiLocalVariable) { final PsiElement parent = variable.getParent(); if (parent == null) return false; @@ -433,11 +432,7 @@ public class HighlightControlFlowUtil { } else if (variable instanceof PsiParameter) { final PsiParameter parameter = (PsiParameter)variable; - final Boolean isReassigned = parameterIsReassigned.get(parameter); - if (isReassigned != null) return isReassigned.booleanValue(); - boolean isAssigned = PsiUtil.isAssigned(parameter); - parameterIsReassigned.put(parameter, Boolean.valueOf(isAssigned)); - return isAssigned; + return PsiUtil.isAssigned(parameter); } else { return false; diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightNamesUtil.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightNamesUtil.java index ddc62701ed18..48263de0188a 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightNamesUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightNamesUtil.java @@ -143,17 +143,15 @@ public class HighlightNamesUtil { || var instanceof PsiParameter && ((PsiParameter)var).getDeclarationScope() instanceof PsiForeachStatement) { return HighlightInfoType.LOCAL_VARIABLE; } - else if (var instanceof PsiField) { + if (var instanceof PsiField) { return var.hasModifierProperty(PsiModifier.STATIC) ? HighlightInfoType.STATIC_FIELD : HighlightInfoType.INSTANCE_FIELD; } - else if (var instanceof PsiParameter) { + if (var instanceof PsiParameter) { return HighlightInfoType.PARAMETER; } - else { //? - return null; - } + return null; } private static HighlightInfoType getClassNameHighlightType(PsiClass aClass) { diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index d167189d35ca..6a271c9c64a7 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java @@ -884,14 +884,13 @@ public class HighlightUtil { @Nullable - static HighlightInfo checkMustBeBoolean(PsiExpression expr) { + static HighlightInfo checkMustBeBoolean(@NotNull PsiExpression expr, PsiType type) { PsiElement parent = expr.getParent(); if (parent instanceof PsiIfStatement || parent instanceof PsiWhileStatement || parent instanceof PsiForStatement && expr.equals(((PsiForStatement)parent).getCondition()) || parent instanceof PsiDoWhileStatement && expr.equals(((PsiDoWhileStatement)parent).getCondition())) { if (expr.getNextSibling() instanceof PsiErrorElement) return null; - PsiType type = expr.getType(); if (!TypeConversionUtil.isBooleanType(type)) { final HighlightInfo info = createIncompatibleTypeHighlightInfo(PsiType.BOOLEAN, type, expr.getTextRange()); if (expr instanceof PsiMethodCallExpression) { @@ -1225,7 +1224,7 @@ public class HighlightUtil { } @Nullable - static HighlightInfo checkValidArrayAccessExpression(PsiExpression arrayExpression, PsiExpression indexExpression) { + static HighlightInfo checkValidArrayAccessExpression(PsiExpression arrayExpression, PsiExpression indexExpression, PsiType type) { PsiType arrayExpressionType = arrayExpression == null ? null : arrayExpression.getType(); if (arrayExpressionType != null && !(arrayExpressionType instanceof PsiArrayType)) { String description = JavaErrorMessages.message("array.type.expected", formatType(arrayExpressionType)); @@ -1255,13 +1254,11 @@ public class HighlightUtil { } @Nullable - public static Collection checkArrayInitializer(final PsiExpression initializer) { - if (! (initializer instanceof PsiArrayInitializerExpression)) return null; + public static Collection checkArrayInitializer(final PsiExpression initializer, PsiType type) { + if (!(initializer instanceof PsiArrayInitializerExpression)) return null; + if (!(type instanceof PsiArrayType)) return null; - final PsiType arrayInitializerType = initializer.getType(); - if (! (arrayInitializerType instanceof PsiArrayType)) return null; - - final PsiType componentType = ((PsiArrayType) arrayInitializerType).getComponentType(); + final PsiType componentType = ((PsiArrayType) type).getComponentType(); final PsiArrayInitializerExpression arrayInitializer = (PsiArrayInitializerExpression) initializer; boolean arrayTypeFixChecked = false; @@ -1895,12 +1892,10 @@ public class HighlightUtil { @Nullable - public static HighlightInfo checkTernaryOperatorConditionIsBoolean(PsiExpression expression) { + public static HighlightInfo checkTernaryOperatorConditionIsBoolean(PsiExpression expression, PsiType type) { if (expression.getParent() instanceof PsiConditionalExpression && - ((PsiConditionalExpression)expression.getParent()).getCondition() == expression && expression.getType() != null && - !TypeConversionUtil.isBooleanType(expression.getType())) { - PsiType foundType = expression.getType(); - return createIncompatibleTypeHighlightInfo(PsiType.BOOLEAN, foundType, expression.getTextRange()); + ((PsiConditionalExpression)expression.getParent()).getCondition() == expression && !TypeConversionUtil.isBooleanType(type)) { + return createIncompatibleTypeHighlightInfo(PsiType.BOOLEAN, type, expression.getTextRange()); } return null; } @@ -1920,18 +1915,17 @@ public class HighlightUtil { @Nullable - public static HighlightInfo checkAssertOperatorTypes(PsiExpression expression) { + public static HighlightInfo checkAssertOperatorTypes(PsiExpression expression, PsiType type) { + if (type == null) return null; if (!(expression.getParent() instanceof PsiAssertStatement)) { return null; } PsiAssertStatement assertStatement = (PsiAssertStatement)expression.getParent(); - PsiType type = expression.getType(); - if (type == null) return null; if (expression == assertStatement.getAssertCondition() && !TypeConversionUtil.isBooleanType(type)) { // addTypeCast quickfix is not applicable here since no type can be cast to boolean return createIncompatibleTypeHighlightInfo(PsiType.BOOLEAN, type, expression.getTextRange()); } - else if (expression == assertStatement.getAssertDescription() && TypeConversionUtil.isVoidType(type)) { + if (expression == assertStatement.getAssertDescription() && TypeConversionUtil.isVoidType(type)) { String description = JavaErrorMessages.message("void.type.is.not.allowed"); return HighlightInfo.createHighlightInfo(HighlightInfoType.ERROR, expression, description); } @@ -1940,10 +1934,9 @@ public class HighlightUtil { @Nullable - public static HighlightInfo checkSynchronizedExpressionType(PsiExpression expression) { + public static HighlightInfo checkSynchronizedExpressionType(PsiExpression expression, PsiType type) { + if (type == null) return null; if (expression.getParent() instanceof PsiSynchronizedStatement) { - PsiType type = expression.getType(); - if (type == null) return null; PsiSynchronizedStatement synchronizedStatement = (PsiSynchronizedStatement)expression.getParent(); if (expression == synchronizedStatement.getLockExpression() && (type instanceof PsiPrimitiveType || TypeConversionUtil.isNullType(type))) { @@ -1956,17 +1949,18 @@ public class HighlightUtil { @Nullable - public static HighlightInfo checkConditionalExpressionBranchTypesMatch(PsiExpression expression) { - if (!(expression.getParent() instanceof PsiConditionalExpression)) { + public static HighlightInfo checkConditionalExpressionBranchTypesMatch(final PsiExpression expression, PsiType type) { + PsiElement parent = expression.getParent(); + if (!(parent instanceof PsiConditionalExpression)) { return null; } - PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)expression.getParent(); + PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)parent; // check else branches only if (conditionalExpression.getElseExpression() != expression) return null; final PsiExpression thenExpression = conditionalExpression.getThenExpression(); assert thenExpression != null; PsiType thenType = thenExpression.getType(); - PsiType elseType = expression.getType(); + PsiType elseType = type; if (thenType == null || elseType == null) return null; if (conditionalExpression.getType() == null) { // cannot derive type of conditional expression diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 014b08a4774e..a0b1e75a9dbd 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -44,6 +44,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.xml.XmlAttributeValue; import gnu.trove.THashMap; +import gnu.trove.TObjectIntHashMap; import org.jetbrains.annotations.NotNull; import java.util.Collection; @@ -64,7 +65,9 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh private final Map> myUninitializedVarProblems = new THashMap>(); // map codeBlock->List of PsiReferenceExpression of extra initialization of final variable private final Map> myFinalVarProblems = new THashMap>(); - private final Map myParameterIsReassigned = new THashMap(); + + // value==1: no info if the parameter was reassigned (but the parameter is present in current file), value==2: parameter was reassigned + private final TObjectIntHashMap myReassignedParameters = new TObjectIntHashMap(); private final Map> mySingleImportedClasses = new THashMap>(); private final Map> mySingleImportedFields = new THashMap>(); @@ -144,7 +147,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh myFinalVarProblems.clear(); mySingleImportedClasses.clear(); mySingleImportedFields.clear(); - myParameterIsReassigned.clear(); + myReassignedParameters.clear(); myRefCountHolder = null; myFile = null; @@ -329,29 +332,31 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh ProgressManager.checkCanceled(); // visitLiteralExpression is invoked very often in array initializers super.visitExpression(expression); - if (myHolder.add(HighlightUtil.checkMustBeBoolean(expression))) return; + PsiType type = expression.getType(); + if (myHolder.add(HighlightUtil.checkMustBeBoolean(expression, type))) return; + PsiExpression indexExpression; + if (expression instanceof PsiArrayAccessExpression - && ((PsiArrayAccessExpression)expression).getIndexExpression() != null) { - myHolder.add(HighlightUtil.checkValidArrayAccessExpression(((PsiArrayAccessExpression)expression).getArrayExpression(), - ((PsiArrayAccessExpression)expression).getIndexExpression())); + && (indexExpression = ((PsiArrayAccessExpression)expression).getIndexExpression()) != null) { + PsiExpression arrayExpression = ((PsiArrayAccessExpression)expression).getArrayExpression(); + myHolder.add(HighlightUtil.checkValidArrayAccessExpression(arrayExpression, indexExpression, indexExpression.getType())); } if (expression.getParent() instanceof PsiNewExpression && ((PsiNewExpression)expression.getParent()).getQualifier() != expression && ((PsiNewExpression)expression.getParent()).getArrayInitializer() != expression) { // like in 'new String["s"]' - myHolder.add(HighlightUtil.checkValidArrayAccessExpression(null, expression)); + myHolder.add(HighlightUtil.checkValidArrayAccessExpression(null, expression, type)); } if (!myHolder.hasErrorResults()) myHolder.add(HighlightControlFlowUtil.checkCannotWriteToFinal(expression)); if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkVariableExpected(expression)); - if (!myHolder.hasErrorResults()) myHolder.addAll(HighlightUtil.checkArrayInitializer(expression)); - if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkTernaryOperatorConditionIsBoolean(expression)); - if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkAssertOperatorTypes(expression)); - if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkSynchronizedExpressionType(expression)); - if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkConditionalExpressionBranchTypesMatch(expression)); + if (!myHolder.hasErrorResults()) myHolder.addAll(HighlightUtil.checkArrayInitializer(expression, type)); + if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkTernaryOperatorConditionIsBoolean(expression, type)); + if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkAssertOperatorTypes(expression, type)); + if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkSynchronizedExpressionType(expression, type)); + if (!myHolder.hasErrorResults()) myHolder.add(HighlightUtil.checkConditionalExpressionBranchTypesMatch(expression, type)); if (!myHolder.hasErrorResults() && expression.getParent() instanceof PsiThrowStatement && ((PsiThrowStatement)expression.getParent()).getException() == expression) { - PsiType type = expression.getType(); myHolder.add(HighlightUtil.checkMustBeThrowable(type, expression, true)); } @@ -392,11 +397,17 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh final PsiElement child = variable.getLastChild(); if (child instanceof PsiErrorElement && child.getPrevSibling() == identifier) return; } - if (isReassigned(variable)) { - myHolder.add(HighlightNamesUtil.highlightReassignedVariable(variable, variable.getNameIdentifier())); + boolean isMethodParameter = variable instanceof PsiParameter && ((PsiParameter)variable).getDeclarationScope() instanceof PsiMethod; + if (!isMethodParameter) { // method params are highlighted in visitMethod since we should make sure the method body was visited before + if (HighlightControlFlowUtil.isReassigned(variable, myFinalVarProblems)) { + myHolder.add(HighlightNamesUtil.highlightReassignedVariable(variable, identifier)); + } + else { + myHolder.add(HighlightNamesUtil.highlightVariableName(variable, identifier, colorsScheme)); + } } else { - myHolder.add(HighlightNamesUtil.highlightVariableName(variable, identifier, colorsScheme)); + myReassignedParameters.put((PsiParameter)variable, 1); // mark param as present in current file } } else if (parent instanceof PsiClass) { @@ -566,6 +577,21 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh if (!myHolder.hasErrorResults() && method.isConstructor()) { myHolder.add(HighlightClassUtil.checkThingNotAllowedInInterface(method, method.getContainingClass())); } + + // method params are highlighted in visitMethod since we should make sure the method body was visited before + PsiParameter[] parameters = method.getParameterList().getParameters(); + final EditorColorsScheme colorsScheme = myHolder.getColorsScheme(); + + for (PsiParameter parameter : parameters) { + int info = myReassignedParameters.get(parameter); + if (info == 0) continue; // out of this file + if (info == 2) {// reassigned + myHolder.add(HighlightNamesUtil.highlightReassignedVariable(parameter, parameter.getNameIdentifier())); + } + else { + myHolder.add(HighlightNamesUtil.highlightVariableName(parameter, parameter.getNameIdentifier(), colorsScheme)); + } + } } private void highlightReferencedMethodOrClassName(PsiJavaCodeReferenceElement element, PsiElement resolved) { @@ -792,6 +818,10 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh } } + if (variable instanceof PsiParameter && ref instanceof PsiExpression && PsiUtil.isAccessedForWriting((PsiExpression)ref)) { + myReassignedParameters.put((PsiParameter)variable, 2); + } + final EditorColorsScheme colorsScheme = myHolder.getColorsScheme(); if (!variable.hasModifierProperty(PsiModifier.FINAL) && isReassigned(variable)) { myHolder.add(HighlightNamesUtil.highlightReassignedVariable(variable, ref)); @@ -976,7 +1006,15 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh private boolean isReassigned(PsiVariable variable) { try { - return HighlightControlFlowUtil.isReassigned(variable, myFinalVarProblems, myParameterIsReassigned); + boolean reassigned; + if (variable instanceof PsiParameter) { + reassigned = myReassignedParameters.get((PsiParameter)variable) == 2; + } + else { + reassigned = HighlightControlFlowUtil.isReassigned(variable, myFinalVarProblems); + } + + return reassigned; } catch (IndexNotReadyException e) { return false; diff --git a/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java b/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java index ff1fac6b3417..13c4e442c659 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java +++ b/java/java-impl/src/com/intellij/refactoring/util/RefactoringUtil.java @@ -418,7 +418,7 @@ public class RefactoringUtil { public static boolean canBeDeclaredFinal(PsiVariable variable) { LOG.assertTrue(variable instanceof PsiLocalVariable || variable instanceof PsiParameter); final boolean isReassigned = HighlightControlFlowUtil - .isReassigned(variable, new THashMap>(), new THashMap()); + .isReassigned(variable, new THashMap>()); return !isReassigned; }