From 4671577a1a50e01bd26bb7c19122a001236d1fc7 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 12 Sep 2012 10:25:17 +0200 Subject: [PATCH 1/9] fixed wrong dfa for integer comparisons (IDEA-91380) --- .../dataFlow/ControlFlowAnalyzer.java | 3 -- .../dataFlow/DfaMemoryStateImpl.java | 17 +++++-- .../instructions/BinopInstruction.java | 11 +++-- .../instructions/BranchingInstruction.java | 6 +-- .../dataFlow/value/DfaRelationValue.java | 44 +++++++++---------- .../fixture/NotGreaterIsNotEquals.java | 38 ++++++++++++++++ .../DataFlowInspectionFixtureTest.java | 1 + 7 files changed, 83 insertions(+), 37 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 9ec332a8405b..9e6ea841a280 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -952,9 +952,6 @@ class ControlFlowAnalyzer extends JavaElementVisitor { if (JavaTokenType.PLUS == op && (type == null || !type.equalsToText(CommonClassNames.JAVA_LANG_STRING))) { return null; } - else if (op == JavaTokenType.LT || op == JavaTokenType.GT) { - return JavaTokenType.NE; - } return op; } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 3ba99012a81e..2ba233ea60ab 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -553,7 +553,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaCond instanceof DfaUnboxedValue) { DfaVariableValue dfaVar = ((DfaUnboxedValue)dfaCond).getVariable(); boolean isNegated = dfaVar.isNegated(); - DfaVariableValue dfaNormalVar = isNegated ? (DfaVariableValue)dfaVar.createNegated() : dfaVar; + DfaVariableValue dfaNormalVar = isNegated ? dfaVar.createNegated() : dfaVar; DfaConstValue dfaTrue = myFactory.getConstFactory().getTrue(); final DfaValue boxedTrue = myFactory.getBoxedFactory().createBoxed(dfaTrue); DfaRelationValue dfaEqualsTrue = myFactory.getRelationFactory().createRelation(dfaNormalVar, boxedTrue, JavaTokenType.EQEQ, isNegated); @@ -563,7 +563,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaCond instanceof DfaVariableValue) { DfaVariableValue dfaVar = (DfaVariableValue)dfaCond; boolean isNegated = dfaVar.isNegated(); - DfaVariableValue dfaNormalVar = isNegated ? (DfaVariableValue)dfaVar.createNegated() : dfaVar; + DfaVariableValue dfaNormalVar = isNegated ? dfaVar.createNegated() : dfaVar; DfaConstValue dfaTrue = myFactory.getConstFactory().getTrue(); DfaRelationValue dfaEqualsTrue = myFactory.getRelationFactory().createRelation(dfaNormalVar, dfaTrue, JavaTokenType.EQEQ, isNegated); @@ -576,7 +576,10 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (!(dfaCond instanceof DfaRelationValue)) return true; - DfaRelationValue dfaRelation = (DfaRelationValue)dfaCond; + return applyRelationCondition((DfaRelationValue)dfaCond); + } + + private boolean applyRelationCondition(DfaRelationValue dfaRelation) { DfaValue dfaLeft = dfaRelation.getLeftOperand(); DfaValue dfaRight = dfaRelation.getRightOperand(); if (dfaRight == null || dfaLeft == null) return false; @@ -610,6 +613,14 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaLeft instanceof DfaUnknownValue || dfaRight instanceof DfaUnknownValue) return true; + return applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight); + } + + private boolean applyEquivalenceRelation(DfaRelationValue dfaRelation, DfaValue dfaLeft, DfaValue dfaRight) { + boolean isNegated = dfaRelation.isNonEquality(); + if (!isNegated && !dfaRelation.isEquality()) { + return true; + } boolean result = applyRelation(dfaLeft, dfaRight, isNegated); if (dfaRight instanceof DfaConstValue) { Object constVal = ((DfaConstValue)dfaRight).getValue(); diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java index 5e13d70118f2..195f538da745 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BinopInstruction.java @@ -34,20 +34,19 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.tree.TokenSet; import org.jetbrains.annotations.NotNull; +import static com.intellij.psi.JavaTokenType.*; + public class BinopInstruction extends BranchingInstruction { + private static final TokenSet ourSignificantOperations = TokenSet.create(EQEQ, NE, LT, GT, LE, GE, INSTANCEOF_KEYWORD, PLUS); private final IElementType myOperationSign; private final Project myProject; public BinopInstruction(IElementType opSign, PsiElement psiAnchor, @NotNull Project project) { myProject = project; - if (JavaTokenType.EQEQ == opSign || JavaTokenType.NE == opSign || JavaTokenType.INSTANCEOF_KEYWORD == opSign || JavaTokenType.PLUS == opSign) { - myOperationSign = opSign; - } - else { - myOperationSign = null; - } + myOperationSign = ourSignificantOperations.contains(opSign) ? opSign : null; setPsiAnchor(psiAnchor); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BranchingInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BranchingInstruction.java index 2a036c3256d8..d1524fc42092 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BranchingInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/BranchingInstruction.java @@ -70,8 +70,8 @@ public abstract class BranchingInstruction extends Instruction { return "true".equals(text) || "false".equals(text); } - protected void setPsiAnchor(PsiElement psiAcnchor) { - myExpression = psiAcnchor; - isConstTrue = psiAcnchor != null && isBoolConst(psiAcnchor); + protected void setPsiAnchor(PsiElement psiAnchor) { + myExpression = psiAnchor; + isConstTrue = psiAnchor != null && isBoolConst(psiAnchor); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaRelationValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaRelationValue.java index 6b96b0bba706..2e09163b00ed 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaRelationValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaRelationValue.java @@ -25,7 +25,6 @@ package com.intellij.codeInspection.dataFlow.value; import com.intellij.openapi.util.Comparing; -import com.intellij.psi.JavaTokenType; import com.intellij.psi.tree.IElementType; import com.intellij.util.containers.HashMap; import org.jetbrains.annotations.NonNls; @@ -33,6 +32,8 @@ import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import static com.intellij.psi.JavaTokenType.*; + public class DfaRelationValue extends DfaValue { private DfaValue myLeftOperand; private DfaValue myRightOperand; @@ -52,8 +53,8 @@ public class DfaRelationValue extends DfaValue { @Nullable public DfaRelationValue createRelation(DfaValue dfaLeft, DfaValue dfaRight, IElementType relation, boolean negated) { - if (dfaRight instanceof DfaTypeValue && JavaTokenType.INSTANCEOF_KEYWORD != relation) return null; - if (JavaTokenType.PLUS == relation) return null; + if (dfaRight instanceof DfaTypeValue && INSTANCEOF_KEYWORD != relation) return null; + if (PLUS == relation) return null; if (dfaLeft instanceof DfaVariableValue || dfaLeft instanceof DfaBoxedValue || dfaLeft instanceof DfaUnboxedValue || dfaRight instanceof DfaVariableValue || dfaRight instanceof DfaBoxedValue || dfaRight instanceof DfaUnboxedValue) { @@ -79,16 +80,16 @@ public class DfaRelationValue extends DfaValue { final DfaValue dfaLeft, final DfaValue dfaRight) { // To canonical form. - if (JavaTokenType.NE == relation) { - relation = JavaTokenType.EQEQ; + if (NE == relation) { + relation = EQEQ; negated = !negated; } - else if (JavaTokenType.LT == relation) { - relation = JavaTokenType.GE; + else if (LT == relation) { + relation = GE; negated = !negated; } - else if (JavaTokenType.LE == relation) { - relation = JavaTokenType.GT; + else if (LE == relation) { + relation = GT; negated = !negated; } @@ -115,19 +116,10 @@ public class DfaRelationValue extends DfaValue { } private static IElementType getSymmetricOperation(IElementType sign) { - if (JavaTokenType.LT == sign) { - return JavaTokenType.GT; - } - else if (JavaTokenType.GE == sign) { - return JavaTokenType.LE; - } - else if (JavaTokenType.GT == sign) { - return JavaTokenType.LT; - } - else if (JavaTokenType.LE == sign) { - return JavaTokenType.GE; - } - + if (LT == sign) return GT; + if (GE == sign) return LE; + if (GT == sign) return LT; + if (LE == sign) return GE; return sign; } } @@ -168,6 +160,14 @@ public class DfaRelationValue extends DfaValue { rel.myIsNegated == myIsNegated; } + public boolean isEquality() { + return myRelation == EQEQ && !myIsNegated; + } + + public boolean isNonEquality() { + return myRelation == EQEQ && myIsNegated || myRelation == GT && !myIsNegated || myRelation == GE && myIsNegated; + } + @NonNls public String toString() { return (isNegated() ? "not " : "") + myLeftOperand + " " + myRelation + " " + myRightOperand; } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java b/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java new file mode 100644 index 000000000000..6528fcd5396d --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NotGreaterIsNotEquals.java @@ -0,0 +1,38 @@ +import org.jetbrains.annotations.NotNull; + +import java.io.File; + +class Zoo2 { + public static boolean startsWith(@NotNull String path, @NotNull String start, final boolean caseSensitive) { + final int length1 = path.length(); + final int length2 = start.length(); + if (length2 == 0) return true; + if (length2 > length1) return false; + if (!path.regionMatches(!caseSensitive, 0, start, 0, length2)) return false; + if (length1 == length2) return true; + char last2 = start.charAt(length2 - 1); + char next1; + if (last2 == '/' || last2 == File.separatorChar) { + next1 = path.charAt(length2 - 1); + } + else { + next1 = path.charAt(length2); + } + return next1 == '/' || next1 == File.separatorChar; + } + void foo(Some me, Some other) { + if (me.depth < other.depth) { + System.out.println("less"); + } else if (other.depth > me.depth) { + System.out.println("more"); + } + } +} + +class Some { + final int depth; + + Some(int depth) { + this.depth = depth; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index 0e7f4ec099c0..a5f80d7392b2 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -77,6 +77,7 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testEqualsConstant() throws Throwable { doTest(); } public void testFinalLoopVariableInstanceof() throws Throwable { doTest(); } public void testGreaterIsNotEquals() throws Throwable { doTest(); } + public void testNotGreaterIsNotEquals() throws Throwable { doTest(); } public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } public void testChainedFinalFieldAccessorsDfa() throws Throwable { doTest(); } From 94dd1967d11eb5efb786010c6f9dcdd9ab747052 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 12 Sep 2012 12:58:28 +0400 Subject: [PATCH 2/9] lambda: check return compatibility according to expected lambda type; test fixed --- .../src/com/intellij/psi/LambdaUtil.java | 38 +++++++++++-------- .../highlighting/ReturnTypeCompatibility.java | 2 +- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java b/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java index 30e0ebc579cb..a9016759a16b 100644 --- a/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java @@ -17,6 +17,7 @@ package com.intellij.psi; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Computable; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.infos.MethodCandidateInfo; import com.intellij.psi.util.*; import org.jetbrains.annotations.NotNull; @@ -215,11 +216,16 @@ public class LambdaUtil { } } if (checkReturnType) { + final String uniqueVarName = + JavaCodeStyleManager.getInstance(lambdaExpression.getProject()).suggestUniqueVariableName("l", lambdaExpression, true); + final PsiStatement assignmentFromText = JavaPsiFacade.getElementFactory(lambdaExpression.getProject()) + .createStatementFromText(leftType.getCanonicalText() + " " + uniqueVarName + " = " + lambdaExpression.getText(), lambdaExpression); + final PsiLocalVariable localVariable = (PsiLocalVariable)((PsiDeclarationStatement)assignmentFromText).getDeclaredElements()[0]; LOG.assertTrue(psiClass != null); PsiType methodReturnType = getReturnType(psiClass, methodSignature); if (methodReturnType != null) { methodReturnType = resolveResult.getSubstitutor().substitute(methodSignature.getSubstitutor().substitute(methodReturnType)); - return checkReturnTypeCompatible(lambdaExpression, methodReturnType) == null; + return checkReturnTypeCompatible((PsiLambdaExpression)localVariable.getInitializer(), methodReturnType) == null; } } return true; @@ -459,21 +465,21 @@ public class LambdaUtil { final PsiElement gParent = expressionList.getParent(); if (gParent instanceof PsiCallExpression) { final PsiCallExpression contextCall = (PsiCallExpression)gParent; - return PsiResolveHelper.ourGuard.doPreventingRecursion(expression, true, new Computable() { - @Override - public PsiType compute() { - final JavaResolveResult resolveResult = contextCall.resolveMethodGenerics(); - final PsiElement resolve = resolveResult.getElement(); - if (resolve instanceof PsiMethod) { - final PsiParameter[] parameters = ((PsiMethod)resolve).getParameterList().getParameters(); - if (lambdaIdx < parameters.length) { - if (!tryToSubstitute) return parameters[lambdaIdx].getType(); - return resolveResult.getSubstitutor().substitute(parameters[lambdaIdx].getType()); - } + final JavaResolveResult resolveResult = contextCall.resolveMethodGenerics(); + final PsiElement resolve = resolveResult.getElement(); + if (resolve instanceof PsiMethod) { + final PsiParameter[] parameters = ((PsiMethod)resolve).getParameterList().getParameters(); + if (lambdaIdx < parameters.length) { + if (!tryToSubstitute) return parameters[lambdaIdx].getType(); + return PsiResolveHelper.ourGuard.doPreventingRecursion(expression, true, new Computable() { + @Override + public PsiType compute() { + return resolveResult.getSubstitutor().substitute(parameters[lambdaIdx].getType()); + } + }); } - return null; } - }); + return null; } } } @@ -507,8 +513,8 @@ public class LambdaUtil { } final PsiParameterList parameterList = lambdaExpression.getParameterList(); + final boolean add = currentStack.add(parameterList); try { - currentStack.add(parameterList); PsiType type = getFunctionalInterfaceType(lambdaExpression, true); if (type == null) { type = getFunctionalInterfaceType(lambdaExpression, false); @@ -528,7 +534,7 @@ public class LambdaUtil { } } finally { - currentStack.remove(parameterList); + if (add) currentStack.remove(parameterList); } } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/highlighting/ReturnTypeCompatibility.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/highlighting/ReturnTypeCompatibility.java index f558cf725f99..4cc91541a566 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/highlighting/ReturnTypeCompatibility.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/highlighting/ReturnTypeCompatibility.java @@ -57,7 +57,7 @@ class ReturnTypeCompatibility { } public static void main(String[] args) { - call(i-> {return i;}); + call(i-> {return i;}); } } From 9167cc6b5bbc4ef5bc70c3c3caa112164fb10975 Mon Sep 17 00:00:00 2001 From: Konstantin Bulenkov Date: Wed, 12 Sep 2012 13:03:57 +0400 Subject: [PATCH 3/9] IDEA-91351 Strange way to invoke color picker --- images/src/META-INF/ImagesPlugin.xml | 4 +++ .../actions/ColorPickerForImageAction.java | 29 +++++++++++++++++++ .../images/actions/EditExternallyAction.java | 4 +++ .../images/editor/impl/ImageEditorUI.java | 10 +++++++ 4 files changed, 47 insertions(+) create mode 100644 images/src/org/intellij/images/actions/ColorPickerForImageAction.java diff --git a/images/src/META-INF/ImagesPlugin.xml b/images/src/META-INF/ImagesPlugin.xml index c11f25f8120a..7a7bf2602548 100644 --- a/images/src/META-INF/ImagesPlugin.xml +++ b/images/src/META-INF/ImagesPlugin.xml @@ -25,6 +25,9 @@ + + + + diff --git a/images/src/org/intellij/images/actions/ColorPickerForImageAction.java b/images/src/org/intellij/images/actions/ColorPickerForImageAction.java new file mode 100644 index 000000000000..ea70d4eb9cb8 --- /dev/null +++ b/images/src/org/intellij/images/actions/ColorPickerForImageAction.java @@ -0,0 +1,29 @@ +/* + * Copyright 2000-2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.intellij.images.actions; + +import com.intellij.openapi.actionSystem.AnActionEvent; +import com.intellij.ui.ShowColorPickerAction; + +/** + * @author Konstantin Bulenkov + */ +public class ColorPickerForImageAction extends ShowColorPickerAction { + @Override + public void update(AnActionEvent e) { + EditExternallyAction.doUpdate(e); + } +} diff --git a/images/src/org/intellij/images/actions/EditExternallyAction.java b/images/src/org/intellij/images/actions/EditExternallyAction.java index 83c3ef7c8c94..e9d627737cba 100644 --- a/images/src/org/intellij/images/actions/EditExternallyAction.java +++ b/images/src/org/intellij/images/actions/EditExternallyAction.java @@ -106,6 +106,10 @@ public final class EditExternallyAction extends AnAction { public void update(AnActionEvent e) { super.update(e); + doUpdate(e); + } + + static void doUpdate(AnActionEvent e) { VirtualFile[] files = e.getData(PlatformDataKeys.VIRTUAL_FILE_ARRAY); final boolean isEnabled = isImages(files); if (e.getPlace().equals(ActionPlaces.PROJECT_VIEW_POPUP)) { diff --git a/images/src/org/intellij/images/editor/impl/ImageEditorUI.java b/images/src/org/intellij/images/editor/impl/ImageEditorUI.java index 359b7b383c92..0cd9f275cdc2 100644 --- a/images/src/org/intellij/images/editor/impl/ImageEditorUI.java +++ b/images/src/org/intellij/images/editor/impl/ImageEditorUI.java @@ -26,6 +26,7 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiManager; import com.intellij.ui.PopupHandler; import com.intellij.ui.ScrollPaneFactory; +import com.intellij.util.ui.UIUtil; import org.intellij.images.ImagesBundle; import org.intellij.images.editor.ImageDocument; import org.intellij.images.editor.ImageEditor; @@ -206,6 +207,15 @@ final class ImageEditorUI extends JPanel implements DataProvider { public Dimension getPreferredSize() { return imageComponent.getSize(); } + + @Override + protected void paintComponent(Graphics g) { + super.paintComponent(g); + if (UIUtil.isUnderDarcula()) { + g.setColor(UIUtil.getControlColor().brighter()); + g.fillRect(0,0,getWidth(), getHeight()); + } + } } private final class ImageWheelAdapter implements MouseWheelListener { From 136cf32a799efc51ed18b62bed4b819dd60fba04 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 12 Sep 2012 11:05:47 +0200 Subject: [PATCH 4/9] IDEA-90685 (Access to static field locked on instance data shall ignore accessing the logger) --- ...StaticFieldLockedOnInstanceInspection.java | 93 ++++++++++--------- .../AccessToStaticFieldLockedOnInstance.html | 2 + ...essToStaticFieldLockedOnInstanceData.java} | 8 +- .../expected.xml | 31 +++++++ ...icFieldLockedOnInstanceInspectionTest.java | 12 +++ 5 files changed, 100 insertions(+), 46 deletions(-) rename plugins/InspectionGadgets/test/com/siyeh/igtest/threading/{AccessToStaticFieldLockedOnInstanceDataInspection.java => access_to_static_field_locked_on_instance_data/AccessToStaticFieldLockedOnInstanceData.java} (77%) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/expected.xml create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspectionTest.java diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspection.java index 3dfb527277d2..aff3c413c736 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 Dave Griffith, Bas Leijdekkers + * Copyright 2006-2012 Dave Griffith, Bas Leijdekkers * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,78 +17,77 @@ package com.siyeh.ig.threading; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.OrderedSet; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.ui.UiUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; -public class AccessToStaticFieldLockedOnInstanceInspection - extends BaseInspection { +import javax.swing.*; +public class AccessToStaticFieldLockedOnInstanceInspection extends BaseInspection { + + @SuppressWarnings("PublicField") public OrderedSet ignoredClasses = new OrderedSet(); + + @Override @NotNull public String getDisplayName() { - return InspectionGadgetsBundle.message( - "access.to.static.field.locked.on.instance.display.name"); + return InspectionGadgetsBundle.message("access.to.static.field.locked.on.instance.display.name"); } + @Override @NotNull protected String buildErrorString(Object... infos) { - return InspectionGadgetsBundle.message( - "access.to.static.field.locked.on.instance.problem.descriptor"); + return InspectionGadgetsBundle.message("access.to.static.field.locked.on.instance.problem.descriptor"); } + @Nullable + @Override + public JComponent createOptionsPanel() { + return UiUtils.createTreeClassChooserList(ignoredClasses, "Ignored Classes", "Choose class to ignore"); + } + + @Override public BaseInspectionVisitor buildVisitor() { return new AccessToStaticFieldLockedOnInstanceVisitor(); } - private static class AccessToStaticFieldLockedOnInstanceVisitor - extends BaseInspectionVisitor { + private class AccessToStaticFieldLockedOnInstanceVisitor extends BaseInspectionVisitor { @Override - public void visitReferenceExpression( - @NotNull PsiReferenceExpression expression) { + public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) { super.visitReferenceExpression(expression); boolean isLockedOnInstance = false; boolean isLockedOnClass = false; - final PsiMethod containingMethod = - PsiTreeUtil.getParentOfType(expression, PsiMethod.class); - if (containingMethod != null && - containingMethod.hasModifierProperty( - PsiModifier.SYNCHRONIZED)) { - if (containingMethod.hasModifierProperty( - PsiModifier.STATIC)) { + final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class); + if (containingMethod != null && containingMethod.hasModifierProperty(PsiModifier.SYNCHRONIZED)) { + if (containingMethod.hasModifierProperty(PsiModifier.STATIC)) { isLockedOnClass = true; } else { isLockedOnInstance = true; } } - final PsiClass expressionClass = - PsiTreeUtil.getParentOfType(expression, PsiClass.class); + final PsiClass expressionClass = PsiTreeUtil.getParentOfType(expression, PsiClass.class); if (expressionClass == null) { return; } PsiElement elementToCheck = expression; while (true) { - final PsiSynchronizedStatement synchronizedStatement = - PsiTreeUtil.getParentOfType(elementToCheck, - PsiSynchronizedStatement.class); - if (synchronizedStatement == null || - !PsiTreeUtil.isAncestor(expressionClass, - synchronizedStatement, true)) { + final PsiSynchronizedStatement synchronizedStatement = PsiTreeUtil.getParentOfType(elementToCheck, PsiSynchronizedStatement.class); + if (synchronizedStatement == null || !PsiTreeUtil.isAncestor(expressionClass, synchronizedStatement, true)) { break; } - final PsiExpression lockExpression = - synchronizedStatement.getLockExpression(); + final PsiExpression lockExpression = synchronizedStatement.getLockExpression(); if (lockExpression instanceof PsiReferenceExpression) { - final PsiReferenceExpression reference = - (PsiReferenceExpression)lockExpression; - final PsiElement referent = reference.resolve(); - if (referent instanceof PsiField) { - final PsiField referentField = (PsiField)referent; - if (referentField.hasModifierProperty( - PsiModifier.STATIC)) { + final PsiReferenceExpression reference = (PsiReferenceExpression)lockExpression; + final PsiElement target = reference.resolve(); + if (target instanceof PsiField) { + final PsiField lockField = (PsiField)target; + if (lockField.hasModifierProperty(PsiModifier.STATIC)) { isLockedOnClass = true; } else { @@ -99,8 +98,7 @@ public class AccessToStaticFieldLockedOnInstanceInspection else if (lockExpression instanceof PsiThisExpression) { isLockedOnInstance = true; } - else if (lockExpression instanceof - PsiClassObjectAccessExpression) { + else if (lockExpression instanceof PsiClassObjectAccessExpression) { isLockedOnClass = true; } elementToCheck = synchronizedStatement; @@ -108,19 +106,28 @@ public class AccessToStaticFieldLockedOnInstanceInspection if (!isLockedOnInstance || isLockedOnClass) { return; } - final PsiElement referent = expression.resolve(); - if (!(referent instanceof PsiField)) { + final PsiElement target = expression.resolve(); + if (!(target instanceof PsiField)) { return; } - final PsiField referredField = (PsiField)referent; - if (!referredField.hasModifierProperty(PsiModifier.STATIC) || - ExpressionUtils.isConstant(referredField)) { + final PsiField lockedField = (PsiField)target; + if (!lockedField.hasModifierProperty(PsiModifier.STATIC) || ExpressionUtils.isConstant(lockedField)) { return; } - final PsiClass containingClass = referredField.getContainingClass(); + final PsiClass containingClass = lockedField.getContainingClass(); if (!PsiTreeUtil.isAncestor(containingClass, expression, false)) { return; } + if (!ignoredClasses.isEmpty()) { + final PsiType type = lockedField.getType(); + if (type instanceof PsiClassType) { + final PsiClassType classType = (PsiClassType)type; + final PsiClass aClass = classType.resolve(); + if (aClass != null && ignoredClasses.contains(aClass.getQualifiedName())) { + return; + } + } + } registerError(expression); } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/AccessToStaticFieldLockedOnInstance.html b/plugins/InspectionGadgets/src/inspectionDescriptions/AccessToStaticFieldLockedOnInstance.html index f25162ddaf7e..b80c141def96 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/AccessToStaticFieldLockedOnInstance.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/AccessToStaticFieldLockedOnInstance.html @@ -6,6 +6,8 @@ Locking a static field on instance data does not prevent the field from b modified by other instances, and thus may result in surprising race conditions.

+Use the table below to specify classes to ignore. Any static fields of the types specified will be ignored by this inspection. +

Powered by InspectionGadgets \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/AccessToStaticFieldLockedOnInstanceDataInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/AccessToStaticFieldLockedOnInstanceData.java similarity index 77% rename from plugins/InspectionGadgets/test/com/siyeh/igtest/threading/AccessToStaticFieldLockedOnInstanceDataInspection.java rename to plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/AccessToStaticFieldLockedOnInstanceData.java index f7f09ce41541..68e891b2fd23 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/AccessToStaticFieldLockedOnInstanceDataInspection.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/AccessToStaticFieldLockedOnInstanceData.java @@ -1,10 +1,12 @@ -package com.siyeh.igtest.threading; +package com.siyeh.igtest.threading.access_to_static_field_locked_on_instance_data; +import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.Executors; -public class AccessToStaticFieldLockedOnInstanceDataInspection { +public class AccessToStaticFieldLockedOnInstanceData { private static int foo; + private static final List LIST = new java.util.ArrayList(); // pretend this is immutable public static synchronized void test1() { @@ -25,13 +27,13 @@ public class AccessToStaticFieldLockedOnInstanceDataInspection { { foo = 3; System.out.println(foo); + LIST.get(0); } } } class StaticFieldNotLockedOnInstanceData { private static final Object printer_ = new Object(); - ; private final Executor executor_ = Executors.newCachedThreadPool(); private final Object lock_ = new Object(); diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/expected.xml new file mode 100644 index 000000000000..e59245054b0e --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data/expected.xml @@ -0,0 +1,31 @@ + + + + AccessToStaticFieldLockedOnInstanceData.java + 19 + Access to static field locked on instance data + Access to static field <code>foo</code> locked on instance data #loc + + + + AccessToStaticFieldLockedOnInstanceData.java + 20 + Access to static field locked on instance data + Access to static field <code>foo</code> locked on instance data #loc + + + + AccessToStaticFieldLockedOnInstanceData.java + 28 + Access to static field locked on instance data + Access to static field <code>foo</code> locked on instance data #loc + + + + AccessToStaticFieldLockedOnInstanceData.java + 29 + Access to static field locked on instance data + Access to static field <code>foo</code> locked on instance data #loc + + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspectionTest.java new file mode 100644 index 000000000000..f1ec35f22993 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/AccessToStaticFieldLockedOnInstanceInspectionTest.java @@ -0,0 +1,12 @@ +package com.siyeh.ig.threading; + +import com.siyeh.ig.IGInspectionTestCase; + +public class AccessToStaticFieldLockedOnInstanceInspectionTest extends IGInspectionTestCase { + + public void test() throws Exception { + final AccessToStaticFieldLockedOnInstanceInspection tool = new AccessToStaticFieldLockedOnInstanceInspection(); + tool.ignoredClasses.add("java.util.List"); + doTest("com/siyeh/igtest/threading/access_to_static_field_locked_on_instance_data", tool); + } +} \ No newline at end of file From 7752d197c946e3aac37f20fdca26fb824165b1e1 Mon Sep 17 00:00:00 2001 From: "Denis.Zhdanov" Date: Wed, 12 Sep 2012 13:25:06 +0400 Subject: [PATCH 5/9] IDEA-91337 doc popup for variable: missing link for ArrayList --- .../intellij/codeInsight/javadoc/JavaDocInfoGenerator.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/java-impl/src/com/intellij/codeInsight/javadoc/JavaDocInfoGenerator.java b/java/java-impl/src/com/intellij/codeInsight/javadoc/JavaDocInfoGenerator.java index 600989e339b2..5a3b8178f542 100644 --- a/java/java-impl/src/com/intellij/codeInsight/javadoc/JavaDocInfoGenerator.java +++ b/java/java-impl/src/com/intellij/codeInsight/javadoc/JavaDocInfoGenerator.java @@ -1808,5 +1808,10 @@ public class JavaDocInfoGenerator { expression.getArgumentList().acceptChildren(this); myBuffer.append(")"); } + + @Override + public void visitLiteralExpression(PsiLiteralExpression expression) { + myBuffer.append(expression.getText()); + } } } From 85cc89a6cb1e863f4abac800ac61bd987397be27 Mon Sep 17 00:00:00 2001 From: Konstantin Bulenkov Date: Wed, 12 Sep 2012 13:33:28 +0400 Subject: [PATCH 6/9] IDEA-91415 Darcula: file colors should change when switch to Darcula and back --- .../com/intellij/ui/tabs/FileColorManagerImpl.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/ui/tabs/FileColorManagerImpl.java b/platform/lang-impl/src/com/intellij/ui/tabs/FileColorManagerImpl.java index e1ba9fb8c622..92e94d596448 100644 --- a/platform/lang-impl/src/com/intellij/ui/tabs/FileColorManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/ui/tabs/FileColorManagerImpl.java @@ -57,19 +57,19 @@ public class FileColorManagerImpl extends FileColorManager implements Persistent static { ourDefaultColors = new LinkedHashMap(); - ourDefaultColors.put("Blue", new Color(220, 240, 255)); + ourDefaultColors.put("Blue", new Color(0xdcf0ff)); ourDefaultColors.put("Green", new Color(231, 250, 219)); ourDefaultColors.put("Orange", new Color(246, 224, 202)); ourDefaultColors.put("Rose", new Color(242, 206, 202)); ourDefaultColors.put("Violet", new Color(222, 213, 241)); ourDefaultColors.put("Yellow", new Color(255, 255, 228)); ourDefaultDarkColors = new LinkedHashMap(); - ourDefaultDarkColors.put("Blue", new Color(255-220, 255-240, 255-255)); - ourDefaultDarkColors.put("Green", new Color(255-231, 255-250, 255-219)); - ourDefaultDarkColors.put("Orange", new Color(255-246, 255-224, 255-202)); - ourDefaultDarkColors.put("Rose", new Color(255-242, 255-206, 255-202)); - ourDefaultDarkColors.put("Violet", new Color(255-222, 255-213, 255-241)); - ourDefaultDarkColors.put("Yellow", new Color(255-255, 255-255, 255-228)); + ourDefaultDarkColors.put("Blue", new Color(0x2B3557)); + ourDefaultDarkColors.put("Green", new Color(0x253B10)); + ourDefaultDarkColors.put("Orange", new Color(0xB85E3A)); + ourDefaultDarkColors.put("Rose", new Color(0x4B193E)); + ourDefaultDarkColors.put("Violet", new Color(0x341657)); + ourDefaultDarkColors.put("Yellow", new Color(0x402D10)); } public FileColorManagerImpl(@NotNull final Project project) { From fbab25d3866cfeaf8a5ffc81dcfa2446f4c61709 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 12 Sep 2012 10:49:30 +0200 Subject: [PATCH 7/9] EA-39129 - NPE: AbstractSuppressByNoInspectionCommentFix.getLineCommentPrefix --- .../actions/AbstractSuppressByNoInspectionCommentFix.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/actions/AbstractSuppressByNoInspectionCommentFix.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/actions/AbstractSuppressByNoInspectionCommentFix.java index 4adfb21e4238..886a668946a2 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/actions/AbstractSuppressByNoInspectionCommentFix.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/actions/AbstractSuppressByNoInspectionCommentFix.java @@ -77,11 +77,9 @@ public abstract class AbstractSuppressByNoInspectionCommentFix extends SuppressI } @Nullable - protected static String getLineCommentPrefix(final PsiElement comment) { + protected static String getLineCommentPrefix(@NotNull final PsiElement comment) { final Commenter commenter = LanguageCommenters.INSTANCE.forLanguage(comment.getLanguage()); - assert commenter != null; - - return commenter.getLineCommentPrefix(); + return commenter == null ? null : commenter.getLineCommentPrefix(); } protected void createSuppression(final Project project, From bb8864c34e1b41ce987b7a5f7c4b3a78136d2eed Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 12 Sep 2012 11:04:29 +0200 Subject: [PATCH 8/9] fix SOE in groovy control flow building (EA-39132,EA-39133) --- .../controlFlow/impl/ControlFlowBuilder.java | 18 +++++++++++------- .../groovy/lang/GroovyHighlightingTest.groovy | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java index e67018a2484f..e91bade429cd 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java @@ -382,12 +382,7 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { addNodeAndCheckPending(throwInstruction); interruptFlow(); - final PsiType type = RecursionManager.doPreventingRecursion(exception, true, new NullableComputable() { - @Override - public PsiType compute() { - return exception.getNominalType(); - } - }); + final PsiType type = getNominalTypeNoRecursion(exception); if (type != null) { ExceptionInfo info = findCatch(type); if (info != null) { @@ -402,6 +397,15 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { } } + private static PsiType getNominalTypeNoRecursion(final GrExpression exception) { + return RecursionManager.doPreventingRecursion(exception, true, new NullableComputable() { + @Override + public PsiType compute() { + return exception.getNominalType(); + } + }); + } + private void interruptFlow() { myHead = null; } @@ -924,7 +928,7 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { final GrExpression condition = statement.getCondition(); if (!(condition instanceof GrReferenceExpression)) return false; - PsiType type = TypesUtil.unboxPrimitiveTypeWrapper(condition.getNominalType()); + PsiType type = TypesUtil.unboxPrimitiveTypeWrapper(getNominalTypeNoRecursion(condition)); if (type == null) return false; if (type instanceof PsiPrimitiveType) { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy index 0b17ef7b0f0e..49e27e17f8d4 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.groovy @@ -1419,4 +1419,19 @@ class X { testHighlighting('''\ def (a, b) = [a, a]''') } + + public void testSwitchInLoopNoSoe() { + testHighlighting(''' +def foo(File f) { + while (true) { + switch (f.name) { + case 'foo': f = new File('bar') + } + if (f) { + return + } + } +}''') + + } } \ No newline at end of file From 7b054046b66a75c1ef2ced3b29713095ff3edd3d Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 12 Sep 2012 11:29:08 +0200 Subject: [PATCH 9/9] diagnostics for SOE in groovy superclass resolve (EA-39131,EA-39134) --- .../plugins/groovy/lang/psi/util/GrClassImplUtil.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java index f0087022a35b..2b2293505cad 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/util/GrClassImplUtil.java @@ -386,8 +386,14 @@ public class GrClassImplUtil { return Arrays.asList(grType.getInnerClasses()); } List result = new ArrayList(); - for (CandidateInfo info : CollectClassMembersUtil.getAllInnerClasses(grType, false).values()) { - ContainerUtil.addIfNotNull(result, (PsiClass)info.getElement()); + try { + for (CandidateInfo info : CollectClassMembersUtil.getAllInnerClasses(grType, false).values()) { + ContainerUtil.addIfNotNull(result, (PsiClass)info.getElement()); + } + } + catch (StackOverflowError e) { + + throw new RuntimeException("lastParent=" + lastParent, e); } return result; }