mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-18 09:34:34 +07:00
improve "Unknown @GuardedBy field" inspection
This commit is contained in:
Bas Leijdekkers
+93
-77
@@ -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");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import javax.annotation.concurrent.*;
|
||||
|
||||
class UnknownGuard {
|
||||
@GuardedBy("itself")
|
||||
private final Object one = new Object();
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"nothing\"">"nothing"</warning>)
|
||||
private Object two = new Object();
|
||||
|
||||
@GuardedBy("this")
|
||||
private Object three = new Object();
|
||||
|
||||
@GuardedBy("UnknownGuard.this")
|
||||
private Object four = new Object();
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"Nothing.this\"">"Nothing.this"</warning>)
|
||||
private Object five = new Object();
|
||||
|
||||
@GuardedBy("lock()")
|
||||
private Object six = new Object();
|
||||
private Object lock() {
|
||||
return new Object();
|
||||
}
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"wrong()\"">"wrong()"</warning>)
|
||||
private Object seven = new Object();
|
||||
private void wrong() {}
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"wrong2()\"">"wrong2()"</warning>)
|
||||
private Object eight = new Object();
|
||||
private void wrong2(int i) {}
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"nothing()\"">"nothing()"</warning>)
|
||||
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(<warning descr="Unknown @GuardedBy reference \"wrong\"">"wrong"</warning>)
|
||||
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(<warning descr="Unknown @GuardedBy reference \"Nothing.class\"">"Nothing.class"</warning>)
|
||||
private Object sixteen = new Object();
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"Wrong.this\"">"Wrong.this"</warning>)
|
||||
private Object seventeen = new Object();
|
||||
class Wrong {}
|
||||
|
||||
/**
|
||||
* <warning descr="Unknown @GuardedBy reference \"wrong\"">@GuardedBy(wrong)</warning>
|
||||
*/
|
||||
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(<warning descr="Unknown @GuardedBy reference \"Inner.nothing\"">"Inner.nothing"</warning>)
|
||||
private Object twentyone = new Object();
|
||||
|
||||
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"Nothing.LOCK\"">"Nothing.LOCK"</warning>)
|
||||
private Object twentytwo = new Object();
|
||||
}
|
||||
@@ -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";
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user