IDEA-81416 ("Non-private field accessed in synchronized context" triggers for public fields of private member variables)

plugins/InspectionGadgets/src/com/siyeh/ig/threading/PublicFieldAccessedInSynchronizedContextInspection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-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.
@@ -22,13 +22,11 @@ import com.siyeh.ig.BaseInspectionVisitor;
 import com.siyeh.ig.psiutils.SynchronizationUtil;
 import org.jetbrains.annotations.NotNull;
 
-public class PublicFieldAccessedInSynchronizedContextInspection
-  extends BaseInspection {
+public class PublicFieldAccessedInSynchronizedContextInspection extends BaseInspection {
 
   @NotNull
   public String getDisplayName() {
-    return InspectionGadgetsBundle.message(
-      "public.field.accessed.in.synchronized.context.display.name");
+    return InspectionGadgetsBundle.message("public.field.accessed.in.synchronized.context.display.name");
   }
 
   @NotNull
@@ -38,34 +36,34 @@ public class PublicFieldAccessedInSynchronizedContextInspection
 
   @NotNull
   protected String buildErrorString(Object... infos) {
-    return InspectionGadgetsBundle.message(
-      "public.field.accessed.in.synchronized.context.problem.descriptor");
+    return InspectionGadgetsBundle.message("public.field.accessed.in.synchronized.context.problem.descriptor");
   }
 
   public BaseInspectionVisitor buildVisitor() {
     return new PublicFieldAccessedInSynchronizedContextVisitor();
   }
 
-  private static class PublicFieldAccessedInSynchronizedContextVisitor
-    extends BaseInspectionVisitor {
+  private static class PublicFieldAccessedInSynchronizedContextVisitor extends BaseInspectionVisitor {
 
     @Override
-    public void visitReferenceExpression(
-      @NotNull PsiReferenceExpression expression) {
+    public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) {
+      final PsiExpression qualifier = expression.getQualifierExpression();
+      if (qualifier != null && !(qualifier instanceof PsiThisExpression) && !(qualifier instanceof PsiSuperExpression)) {
+        return;
+      }
       final PsiElement element = expression.resolve();
       if (!(element instanceof PsiField)) {
         return;
       }
       final PsiField field = (PsiField)element;
-      if (field.hasModifierProperty(PsiModifier.PRIVATE) ||
-          field.hasModifierProperty(PsiModifier.FINAL)) {
+      if (field.hasModifierProperty(PsiModifier.PRIVATE) || field.hasModifierProperty(PsiModifier.FINAL)) {
         return;
       }
       if (!SynchronizationUtil.isInSynchronizedContext(expression)) {
         return;
       }
       final PsiClass containingClass = field.getContainingClass();
-      if (containingClass.hasModifierProperty(PsiModifier.PRIVATE)) {
+      if (containingClass == null || containingClass.hasModifierProperty(PsiModifier.PRIVATE)) {
         return;
       }
       registerError(expression);

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/public_field_accessed_in_synchronized_context/Foo.java

@@ -0,0 +1,26 @@
+package com.siyeh.igtest.threading.public_field_accessed_in_synchronized_context;
+
+import java.util.List;
+
+class Bar2 {
+  public String field1;
+  private String field2;
+
+  public void setField2(String s) {
+    field2 = s;
+  }
+}
+public class Foo {
+  public Bar2 myBar;
+  private List<Bar2> myBars;
+
+  synchronized public void setSingle() {
+    myBar.field1 = "bar";
+    myBar.setField2("bar");
+  }
+
+  synchronized public void setViaList() {
+    myBars.iterator().next().field1 = "bar";
+    myBars.iterator().next().setField2("bar");
+  }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/threading/public_field_accessed_in_synchronized_context/expected.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<problems>
+  <problem>
+    <file>Foo.java</file>
+    <line>18</line>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Non-private field accessed in synchronized context</problem_class>
+    <description>Non-private field &lt;code&gt;myBar&lt;/code&gt; accessed in synchronized context #loc</description>
+  </problem>
+
+  <problem>
+    <file>Foo.java</file>
+    <line>19</line>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Non-private field accessed in synchronized context</problem_class>
+    <description>Non-private field &lt;code&gt;myBar&lt;/code&gt; accessed in synchronized context #loc</description>
+  </problem>
+</problems>

plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/PublicFieldAccessedInSynchronizedContextInspectionTest.java

@@ -0,0 +1,12 @@
+package com.siyeh.ig.threading;
+
+import com.siyeh.ig.IGInspectionTestCase;
+
+public class PublicFieldAccessedInSynchronizedContextInspectionTest
+  extends IGInspectionTestCase {
+
+  public void test() throws Exception {
+    doTest("com/siyeh/igtest/threading/public_field_accessed_in_synchronized_context",
+           new PublicFieldAccessedInSynchronizedContextInspection());
+  }
+}