From 595fef1b17e4b291dba78106df2b4664832da187 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 3 Feb 2017 16:15:41 +0100 Subject: [PATCH] allow to omit lock when reading volatile field in "Unguarded field access" inspection --- .../FieldAccessNotGuardedInspection.java | 7 +++++-- .../inspection/guarded/cheapReadWriteLock.java | 13 +++++++++++++ .../FieldAccessedNotGuardedInspectionTest.java | 7 ++++++- 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 java/java-tests/testData/inspection/guarded/cheapReadWriteLock.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/FieldAccessNotGuardedInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/FieldAccessNotGuardedInspection.java index bad497bf0d40..84623131b4df 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/FieldAccessNotGuardedInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/concurrencyAnnotations/FieldAccessNotGuardedInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -54,7 +54,6 @@ public class FieldAccessNotGuardedInspection extends BaseJavaBatchLocalInspectio return new Visitor(holder); } - private static class Visitor extends JavaElementVisitor { private final ProblemsHolder myHolder; @@ -83,6 +82,10 @@ public class FieldAccessNotGuardedInspection extends BaseJavaBatchLocalInspectio } catch (IncorrectOperationException ignore) { return; } + if (guardExpression instanceof PsiThisExpression && !PsiUtil.isAccessedForWriting(expression) && + field.hasModifierProperty(PsiModifier.VOLATILE)) { + return; + } final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class); if (containingMethod != null) { if (JCiPUtil.isGuardedBy(containingMethod, guard) || containingMethod.isConstructor()) { diff --git a/java/java-tests/testData/inspection/guarded/cheapReadWriteLock.java b/java/java-tests/testData/inspection/guarded/cheapReadWriteLock.java new file mode 100644 index 000000000000..4c0c98714b7b --- /dev/null +++ b/java/java-tests/testData/inspection/guarded/cheapReadWriteLock.java @@ -0,0 +1,13 @@ +import javax.annotation.concurrent.GuardedBy; + +class CheapReadWriteLock { + // Employs the cheap read-write lock trick + // All mutative operations MUST be done with the 'this' lock held + @GuardedBy("this") private volatile int value; + + public int getValue() { return value; } + + public synchronized int increment() { + return value++; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/FieldAccessedNotGuardedInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/FieldAccessedNotGuardedInspectionTest.java index 54b8c3f6f977..bda16232e60c 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/FieldAccessedNotGuardedInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/FieldAccessedNotGuardedInspectionTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -42,6 +42,11 @@ public class FieldAccessedNotGuardedInspectionTest extends LightCodeInsightFixtu doTest(); } + public void testCheapReadWriteLock() { + myFixture.addClass("package javax.annotation.concurrent;\n" + getGuardedByAnnotationText()); + doTest(); + } + private void doTest() { myFixture.testHighlighting(true, false, false, getTestName(true) + ".java"); }