warn about unguarded method calls (IDEA-97040)

extends "Unguarded field access" inspection

GitOrigin-RevId: 16ac8bf425cc910bf3cb962c8c7ac4e603ed380d
This commit is contained in:
Bas Leijdekkers
2020-02-21 19:11:25 +00:00
committed by intellij-monorepo-bot
parent 29f6ad76a4
commit 7f9619fed3
6 changed files with 78 additions and 65 deletions
@@ -148,6 +148,7 @@ scope.package=Package {0}
0.field.is.always.initialized.not.null=@{0} field is always initialized not-null
access.can.be.0=Access can be {0}
access.to.field.code.ref.code.outside.of.declared.guards.loc=Access to field <code>#ref</code> outside of declared guards #loc
call.to.method.code.ref.code.outside.of.declared.guards.loc=Call to method <code>#ref()</code> outside of declared guards #loc
annotate.as.safevarargs=Annotate as @SafeVarargs
annotate.overridden.methods.parameters.family.name=Annotate overridden method parameters
annotate.overridden.methods.parameters=Annotate overridden method parameters as ''@{0}''
@@ -145,7 +145,7 @@
<localInspection groupPath="Java" language="JAVA" shortName="InstanceGuardedByStatic" displayName="Instance member guarded by static field"
groupKey="group.names.concurrency.annotation.issues" groupBundle="messages.InspectionsBundle" enabledByDefault="false" level="WARNING"
implementationClass="com.intellij.codeInspection.concurrencyAnnotations.InstanceGuardedByStaticInspection" />
<localInspection groupPath="Java" language="JAVA" shortName="FieldAccessNotGuarded" displayName="Unguarded field access" groupKey="group.names.concurrency.annotation.issues" groupBundle="messages.InspectionsBundle"
<localInspection groupPath="Java" language="JAVA" shortName="FieldAccessNotGuarded" displayName="Unguarded field access or method call" groupKey="group.names.concurrency.annotation.issues" groupBundle="messages.InspectionsBundle"
enabledByDefault="false" level="WARNING"
implementationClass="com.intellij.codeInspection.concurrencyAnnotations.FieldAccessNotGuardedInspection" />
<localInspection groupPath="Java" language="JAVA" shortName="DuplicateThrows" bundle="messages.JavaAnalysisBundle" key="inspection.duplicate.throws.display.name"
@@ -1,4 +1,4 @@
// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection.concurrencyAnnotations;
import com.intellij.codeInsight.PsiEquivalenceUtil;
@@ -48,22 +48,22 @@ public class FieldAccessNotGuardedInspection extends AbstractBaseJavaLocalInspec
return;
}
final PsiElement referent = expression.resolve();
if (!(referent instanceof PsiField)) {
if (!(referent instanceof PsiField) && !(referent instanceof PsiMethod)) {
return;
}
final PsiField field = (PsiField)referent;
final String guard = JCiPUtil.findGuardForMember(field);
final PsiMember member = (PsiMember)referent;
final String guard = JCiPUtil.findGuardForMember(member);
if (guard == null) {
return;
}
final PsiExpression guardExpression;
try {
guardExpression = JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(guard, field);
guardExpression = JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(guard, member);
} catch (IncorrectOperationException ignore) {
return;
}
if (guardExpression instanceof PsiThisExpression && !PsiUtil.isAccessedForWriting(expression) &&
field.hasModifierProperty(PsiModifier.VOLATILE)) {
member.hasModifierProperty(PsiModifier.VOLATILE)) {
return;
}
final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class);
@@ -74,7 +74,7 @@ public class FieldAccessNotGuardedInspection extends AbstractBaseJavaLocalInspec
if (containingMethod.hasModifierProperty(PsiModifier.SYNCHRONIZED)) {
if (guardExpression instanceof PsiThisExpression) {
final PsiThisExpression thisExpression = (PsiThisExpression)guardExpression;
final PsiClass aClass = getClassFromThisExpression(thisExpression, field);
final PsiClass aClass = getClassFromThisExpression(thisExpression, member);
if (aClass == null || InheritanceUtil.isInheritorOrSelf(containingMethod.getContainingClass(), aClass, true)) {
return;
}
@@ -121,7 +121,7 @@ public class FieldAccessNotGuardedInspection extends AbstractBaseJavaLocalInspec
if (lockExpression instanceof PsiThisExpression) {
final PsiThisExpression thisExpression1 = (PsiThisExpression)guardExpression;
final PsiThisExpression thisExpression2 = (PsiThisExpression)lockExpression;
final PsiClass aClass1 = getClassFromThisExpression(thisExpression1, field);
final PsiClass aClass1 = getClassFromThisExpression(thisExpression1, member);
final PsiClass aClass2 = getClassFromThisExpression(thisExpression2, expression);
if (aClass1 == null || aClass1.equals(aClass2)) {
return;
@@ -182,7 +182,10 @@ public class FieldAccessNotGuardedInspection extends AbstractBaseJavaLocalInspec
}
check = syncStatement;
}
myHolder.registerProblem(expression, JavaAnalysisBundle.message("access.to.field.code.ref.code.outside.of.declared.guards.loc"));
myHolder.registerProblem(expression,
member instanceof PsiField ?
JavaAnalysisBundle.message("access.to.field.code.ref.code.outside.of.declared.guards.loc") :
JavaAnalysisBundle.message("call.to.method.code.ref.code.outside.of.declared.guards.loc"));
}
private static PsiClass getClassFromThisExpression(PsiThisExpression thisExpression, PsiElement context) {
@@ -1,18 +1,4 @@
/*
* 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.
*/
// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
package com.intellij.codeInspection.concurrencyAnnotations;
import com.intellij.codeInsight.AnnotationUtil;
@@ -21,6 +7,7 @@ import com.intellij.psi.*;
import com.intellij.psi.impl.cache.impl.id.IdIndex;
import com.intellij.psi.javadoc.PsiDocComment;
import com.intellij.psi.javadoc.PsiDocTag;
import com.intellij.psi.javadoc.PsiDocTagValue;
import com.intellij.psi.util.CachedValueProvider;
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiTreeUtil;
@@ -30,13 +17,13 @@ import org.jetbrains.annotations.Nullable;
import java.util.List;
public class JCiPUtil {
private JCiPUtil() {}
static boolean isJCiPAnnotation(String ref) {
return "Immutable".equals(ref) || "GuardedBy".equals(ref) || "ThreadSafe".equals(ref) || "NotThreadSafe".equals(ref);
}
private JCiPUtil() {
}
public static boolean isImmutable(@NotNull PsiClass aClass) {
return isImmutable(aClass, true);
}
@@ -60,19 +47,25 @@ public class JCiPUtil {
@Nullable
public static String findGuardForMember(@NotNull PsiMember member) {
final PsiAnnotation annotation = AnnotationUtil.findAnnotation(member, ConcurrencyAnnotationsManager.getInstance(member.getProject()).getGuardedByAnnotations());
List<String> annotations = ConcurrencyAnnotationsManager.getInstance(member.getProject()).getGuardedByAnnotations();
final PsiAnnotation annotation = AnnotationUtil.findAnnotation(member, annotations);
if (annotation != null) {
return getGuardValue(annotation);
}
if (member instanceof PsiCompiledElement) {
member = (PsiMember)member.getNavigationElement();
if (member == null || member instanceof PsiCompiledElement) {
return null; // can't analyze compiled code
if (member instanceof PsiDocCommentOwner) {
PsiDocCommentOwner commentOwner = (PsiDocCommentOwner)member;
PsiDocComment comment = commentOwner.getDocComment();
if (comment != null) {
PsiDocTag[] tags = comment.getTags();
for (int i = tags.length - 1; i >= 0; i--) {
String value = getGuardValue(tags[i]);
if (value != null) {
return value;
}
}
}
}
final GuardedTagVisitor visitor = new GuardedTagVisitor();
member.accept(visitor);
return visitor.getGuardString();
return null;
}
static boolean isGuardedBy(@NotNull PsiMember member, @NotNull String guard) {
@@ -81,18 +74,12 @@ public class JCiPUtil {
return annotation != null && guard.equals(getGuardValue(annotation));
}
public static boolean isGuardedBy(PsiMember member, PsiField field) {
return isGuardedBy(member, field.getName());
}
static boolean isGuardedByAnnotation(@NotNull PsiAnnotation annotation) {
return ConcurrencyAnnotationsManager.getInstance(annotation.getProject()).getGuardedByAnnotations().contains(annotation.getQualifiedName());
}
static boolean isGuardedByTag(PsiDocTag tag) {
final String text = tag.getText();
return text.startsWith("@GuardedBy") && text.contains("(") && text.contains(")");
return tag.getText().startsWith("@GuardedBy");
}
@Nullable
@@ -100,38 +87,38 @@ public class JCiPUtil {
final PsiAnnotationMemberValue psiAnnotationMemberValue = annotation.findAttributeValue("value");
if (psiAnnotationMemberValue instanceof PsiLiteralExpression) {
final Object value = ((PsiLiteralExpression)psiAnnotationMemberValue).getValue();
if ("itself".equals(value)) {
final PsiMember member = PsiTreeUtil.getParentOfType(annotation, PsiMember.class);
if (member != null) return member.getName();
}
if (value instanceof String) {
return (String)value;
return resolveItself((String)value, annotation);
}
}
return null;
}
@NotNull
@Nullable
static String getGuardValue(PsiDocTag tag) {
final String text = tag.getText();
return text.substring(text.indexOf((int)'(') + 1, text.indexOf((int)')')).trim();
if ("GuardedBy".equals(tag.getName())) {
final PsiDocTagValue value = tag.getValueElement();
if (value == null) return "";
return resolveItself(value.getText(), tag);
}
else {
final String text = tag.getText();
if (!text.startsWith("@GuardedBy")) return null;
int start = text.indexOf('(');
int end = text.indexOf(')');
if (start >= end || start < 0) return "";
return resolveItself(text.substring(start + 1, end), tag);
}
}
private static class GuardedTagVisitor extends JavaRecursiveElementWalkingVisitor {
private String guardString;
@Override
public void visitDocTag(PsiDocTag tag) {
super.visitDocTag(tag);
final String text = tag.getText();
if (text.startsWith("@GuardedBy") && text.contains("(") && text.contains(")")) {
guardString = text.substring(text.indexOf((int)'(') + 1, text.indexOf((int)')'));
private static String resolveItself(String value, PsiElement context) {
if ("itself".equals(value)) {
final PsiMember member = PsiTreeUtil.getParentOfType(context, PsiMember.class);
if (!(member instanceof PsiField)) {
return "itself";
}
return member.getName();
}
@Nullable
private String getGuardString() {
return guardString;
}
return value;
}
}
@@ -114,4 +114,18 @@ class Example4 {
Object o = field;
}
}
}
class No {
@GuardedBy("this")
void x() {
notify();
}
void y() {
<warning descr="Call to method 'x()' outside of declared guards">x</warning>(); // warn here
}
synchronized void z() {
x(); // don't warn here
}
}
@@ -80,4 +80,12 @@ class UnknownGuard {
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"Nothing.LOCK\"">"Nothing.LOCK"</warning>)
private Object twentytwo = new Object();
/**
* @GuardedBy itself
*/
private Object twentythree = new Object();
@GuardedBy(<warning descr="Unknown @GuardedBy reference \"itself\"">"itself"</warning>)
private void method() {}
}