diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/UnknownGuardInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/UnknownGuardInspection.java index 431bb40f770e..8ca391de0727 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/UnknownGuardInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/UnknownGuardInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2016 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. @@ -18,14 +18,14 @@ package com.intellij.codeInspection.concurrencyAnnotations; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool; import com.intellij.codeInspection.ProblemsHolder; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; +import com.intellij.psi.impl.source.DummyHolder; import com.intellij.psi.javadoc.PsiDocTag; -import com.intellij.psi.search.GlobalSearchScope; -import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.InheritanceUtil; +import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * check locks according to http://www.javaconcurrencyinpractice.com/annotations/doc/net/jcip/annotations/GuardedBy.html @@ -71,79 +71,103 @@ public class UnknownGuardInspection extends BaseJavaBatchLocalInspectionTool { return; } final String guardValue = JCiPUtil.getGuardValue(annotation); - if (guardValue == null || "this".equals(guardValue) || "itself".equals(guardValue)) { + if (isValidGuardText(guardValue, annotation)) { return; } - final PsiClass containingClass = PsiTreeUtil.getParentOfType(annotation, PsiClass.class); - if (containingClass == null) { - return; - } - - if (containsFieldOrMethod(containingClass, guardValue)) return; - - //class-name.class - final Project project = containingClass.getProject(); - final JavaPsiFacade facade = JavaPsiFacade.getInstance(project); - if (guardValue.endsWith(".class") && - facade.findClass(StringUtil.getPackageName(guardValue), GlobalSearchScope.allScope(project)) != null) { - return; - } - - //class-name.field-name - final String classFQName = StringUtil.getPackageName(guardValue); - final PsiClass gClass = facade.findClass(classFQName, GlobalSearchScope.allScope(project)); - if (gClass != null) { - final String fieldName = StringUtil.getShortName(guardValue); - if (gClass.findFieldByName(fieldName, true) != null) { - return; - } - //class-name.this - if (fieldName.equals("this")) { - return; - } - } - - //class-name.this.field-name/method-name - final int thisIdx = guardValue.indexOf("this"); - if (thisIdx > -1 && thisIdx + 1 < guardValue.length()) { - final PsiClass lockClass; - if (thisIdx == 0) { - lockClass = containingClass; - } - else { - final String fqn = guardValue.substring(0, thisIdx - 1); - lockClass = facade.findClass(fqn, GlobalSearchScope.allScope(project)); - } - - if (lockClass != null) { - final String fieldName = guardValue.substring(thisIdx + "this".length() + 1); - if (containsFieldOrMethod(lockClass, fieldName)) { - return; - } - } - } - final PsiAnnotationMemberValue member = annotation.findAttributeValue("value"); if (member == null) { return; } - myHolder.registerProblem(member, "Unknown @GuardedBy field #ref #loc"); + myHolder.registerProblem(member, "Unknown @GuardedBy reference #ref #loc"); } - private static boolean containsFieldOrMethod(PsiClass containingClass, String fieldOrMethod) { - //field-name - if (containingClass.findFieldByName(fieldOrMethod, true) != null) { - return true; + private static boolean isValidGuardText(@Nullable String guardText, @NotNull PsiElement context) { + if (guardText == null || "itself".equals(guardText)) { + return false; } + try { + final JavaPsiFacade facade = JavaPsiFacade.getInstance(context.getProject()); + final PsiExpression expression = facade.getElementFactory().createExpressionFromText(guardText, context); + return isValidGuard(expression, context); + } catch (IncorrectOperationException ignore) { + return false; + } + } - //method-name - if (fieldOrMethod.endsWith("()")) { - final PsiMethod[] methods = containingClass.findMethodsByName(StringUtil.trimEnd(fieldOrMethod, "()"), true); - for (PsiMethod method : methods) { - if (method.getParameterList().getParameters().length == 0) { - return true; - } + private static boolean isValidGuard(PsiExpression expression, PsiElement context) { + if (expression instanceof PsiReferenceExpression) { + final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)expression; + final JavaResolveResult result = referenceExpression.advancedResolve(false); + if (!result.isAccessible() || !result.isValidResult()) { + return false; } + final PsiElement target = result.getElement(); + final PsiElement parent = expression.getParent(); + if (!(parent instanceof DummyHolder)) { + // checking qualifier + return target != null; + } + if (!(target instanceof PsiField)) { + return false; + } + final PsiField field = (PsiField)target; + final PsiType type = field.getType(); + if (type instanceof PsiPrimitiveType) { + return false; + } + final PsiExpression qualifier = referenceExpression.getQualifierExpression(); + return qualifier == null || isValidGuard(qualifier, context); + } + else if (expression instanceof PsiMethodCallExpression) { + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; + final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); + if (argumentList.getExpressions().length != 0) { + return false; + } + final JavaResolveResult result = methodCallExpression.resolveMethodGenerics(); + if (!result.isAccessible() || !result.isValidResult()) { + return false; + } + final PsiElement element = result.getElement(); + if (!(element instanceof PsiMethod)) { + return false; + } + final PsiMethod method = (PsiMethod)element; + final PsiType type = method.getReturnType(); + if (type instanceof PsiPrimitiveType) { + return false; + } + final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); + final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); + return qualifierExpression == null || isValidGuard(qualifierExpression, context); + } + else if (expression instanceof PsiThisExpression) { + final PsiThisExpression thisExpression = (PsiThisExpression)expression; + final PsiJavaCodeReferenceElement qualifier = thisExpression.getQualifier(); + if (qualifier == null) { + return true; + } + final JavaResolveResult result = qualifier.advancedResolve(false); + if (!result.isValidResult() || !result.isAccessible()) { + return false; + } + final PsiElement target = result.getElement(); + if (!(target instanceof PsiClass)) { + return false; + } + final PsiClass aClass = (PsiClass)target; + return InheritanceUtil.hasEnclosingInstanceInScope(aClass, context, false, false); + } + else if (expression instanceof PsiClassObjectAccessExpression) { + final PsiClassObjectAccessExpression classObjectAccessExpression = (PsiClassObjectAccessExpression)expression; + final PsiTypeElement operand = classObjectAccessExpression.getOperand(); + final PsiType type = operand.getType(); + if (!(type instanceof PsiClassType)) { + return false; + } + final PsiClassType classType = (PsiClassType)type; + final PsiClass target = classType.resolve(); + return target != null; } return false; } @@ -155,18 +179,10 @@ public class UnknownGuardInspection extends BaseJavaBatchLocalInspectionTool { return; } final String guardValue = JCiPUtil.getGuardValue(psiDocTag); - if ("this".equals(guardValue)) { + if (isValidGuardText(guardValue, psiDocTag)) { return; } - final PsiClass containingClass = PsiTreeUtil.getParentOfType(psiDocTag, PsiClass.class); - if (containingClass == null) { - return; - } - final PsiField guardField = containingClass.findFieldByName(guardValue, true); - if (guardField != null) { - return; - } - myHolder.registerProblem(psiDocTag, "Unknown @GuardedBy field \"" + guardValue + "\" #loc"); + myHolder.registerProblem(psiDocTag, "Unknown @GuardedBy reference \"" + guardValue + "\" #loc"); } } } diff --git a/java/java-tests/testData/inspection/unknownGuard/UnknownGuard.java b/java/java-tests/testData/inspection/unknownGuard/UnknownGuard.java new file mode 100644 index 000000000000..d30dfca2a763 --- /dev/null +++ b/java/java-tests/testData/inspection/unknownGuard/UnknownGuard.java @@ -0,0 +1,83 @@ +import javax.annotation.concurrent.*; + +class UnknownGuard { + @GuardedBy("itself") + private final Object one = new Object(); + + @GuardedBy("nothing") + private Object two = new Object(); + + @GuardedBy("this") + private Object three = new Object(); + + @GuardedBy("UnknownGuard.this") + private Object four = new Object(); + + @GuardedBy("Nothing.this") + private Object five = new Object(); + + @GuardedBy("lock()") + private Object six = new Object(); + private Object lock() { + return new Object(); + } + + @GuardedBy("wrong()") + private Object seven = new Object(); + private void wrong() {} + + @GuardedBy("wrong2()") + private Object eight = new Object(); + private void wrong2(int i) {} + + @GuardedBy("nothing()") + private Object nine = new Object(); + + @GuardedBy("one") + private Object ten = new Object(); + + @GuardedBy("this.one") + private Object eleven = new Object(); + + @GuardedBy("UnknownGuard.this.one") + private Object twelve = new Object(); + + @GuardedBy("wrong") + private Object thirteen = new Object(); + private int wrong = 1; + + @GuardedBy("java.lang.String.class") + private Object fourteen = new Object(); + + @GuardedBy("UnknownGuard.class") + private Object fifteen = new Object(); + + @GuardedBy("Nothing.class") + private Object sixteen = new Object(); + + @GuardedBy("Wrong.this") + private Object seventeen = new Object(); + class Wrong {} + + /** + * @GuardedBy(wrong) + */ + private Object eighteen = new Object(); + + /** + * @GuardedBy(this) + */ + private Object nineteen = new Object(); + + @GuardedBy("Inner.LOCK") + private Object twenty = new Object(); + static class Inner { + private static final Object LOCK = new Object(); + } + + @GuardedBy("Inner.nothing") + private Object twentyone = new Object(); + + @GuardedBy("Nothing.LOCK") + private Object twentytwo = new Object(); +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/UnknownGuardInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/UnknownGuardInspectionTest.java new file mode 100644 index 000000000000..5d6a46115e00 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInspection/UnknownGuardInspectionTest.java @@ -0,0 +1,54 @@ +/* + * Copyright 2000-2016 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. + */ + +/** + * (c) 2016 Silent Forest AB + * created: 18 March 2016 + */ +package com.intellij.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.codeInspection.concurrencyAnnotations.UnknownGuardInspection; +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; + +/** + * @author Bas Leijdekkers + */ +public class UnknownGuardInspectionTest extends LightCodeInsightFixtureTestCase { + + public void testUnknownGuard() { + myFixture.testHighlighting(true, false, false, getTestName(false) + ".java"); + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.enableInspections(new UnknownGuardInspection()); + myFixture.addClass("package javax.annotation.concurrent;\n" + + "@java.lang.annotation.Target({java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD})\n" + + "@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.RUNTIME)\n" + + "public @interface GuardedBy {\n" + + " java.lang.String value();\n" + + "}"); + } + + @NotNull + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection/unknownGuard"; + } +}