IG: do not store psielement in quickfix field

2026-09-27 10:03:11 +07:00 · 2014-09-01 17:14:33 +02:00
parent 75bfa68a16
commit 46daf334b0
5 changed files with 141 additions and 56 deletions
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-2014 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.
@@ -20,7 +20,6 @@ import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.tree.IElementType;
-import com.intellij.util.IncorrectOperationException;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
@@ -74,17 +73,16 @@ public class DoubleCheckedLockingInspection extends BaseInspection {

  private static class DoubleCheckedLockingFix extends InspectionGadgetsFix {

-    private final PsiField field;
+    private final String myFieldName;

    private DoubleCheckedLockingFix(PsiField field) {
-      this.field = field;
+      myFieldName = field.getName();
    }

    @Override
    @NotNull
    public String getName() {
-      return InspectionGadgetsBundle.message(
-        "double.checked.locking.quickfix", field.getName());
+      return InspectionGadgetsBundle.message("double.checked.locking.quickfix", myFieldName);
    }

    @NotNull
@@ -94,8 +92,21 @@ public class DoubleCheckedLockingInspection extends BaseInspection {
    }

    @Override
-    protected void doFix(Project project, ProblemDescriptor descriptor)
-      throws IncorrectOperationException {
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
+      final PsiElement element = descriptor.getPsiElement();
+      final PsiElement parent = element.getParent();
+      if (!(parent instanceof PsiIfStatement)) {
+        return;
+      }
+      final PsiIfStatement ifStatement = (PsiIfStatement)parent;
+      final PsiExpression condition = ifStatement.getCondition();
+      if (condition == null) {
+        return;
+      }
+      final PsiField field = findCheckedField(condition);
+      if (field == null) {
+        return;
+      }
      final PsiModifierList modifierList = field.getModifierList();
      if (modifierList == null) {
        return;
@@ -104,13 +115,55 @@ public class DoubleCheckedLockingInspection extends BaseInspection {
    }
  }

+  @Nullable
+  private static PsiField findCheckedField(PsiExpression expression) {
+    if (expression instanceof PsiReferenceExpression) {
+      final PsiReferenceExpression referenceExpression =
+        (PsiReferenceExpression)expression;
+      final PsiElement target = referenceExpression.resolve();
+      if (!(target instanceof PsiField)) {
+        return null;
+      }
+      return (PsiField)target;
+    }
+    else if (expression instanceof PsiBinaryExpression) {
+      final PsiBinaryExpression binaryExpression =
+        (PsiBinaryExpression)expression;
+      final IElementType tokenType =
+        binaryExpression.getOperationTokenType();
+      if (!JavaTokenType.EQEQ.equals(tokenType)
+          && !JavaTokenType.NE.equals(tokenType)) {
+        return null;
+      }
+      final PsiExpression lhs = binaryExpression.getLOperand();
+      final PsiExpression rhs = binaryExpression.getROperand();
+      final PsiField field = findCheckedField(lhs);
+      if (field != null) {
+        return field;
+      }
+      return findCheckedField(rhs);
+    }
+    else if (expression instanceof PsiPrefixExpression) {
+      final PsiPrefixExpression prefixExpression =
+        (PsiPrefixExpression)expression;
+      final IElementType tokenType =
+        prefixExpression.getOperationTokenType();
+      if (!JavaTokenType.EXCL.equals(tokenType)) {
+        return null;
+      }
+      return findCheckedField(prefixExpression.getOperand());
+    }
+    else {
+      return null;
+    }
+  }
+
  @Override
  public BaseInspectionVisitor buildVisitor() {
    return new DoubleCheckedLockingVisitor();
  }

-  private class DoubleCheckedLockingVisitor
-    extends BaseInspectionVisitor {
+  private class DoubleCheckedLockingVisitor extends BaseInspectionVisitor {

    @Override
    public void visitIfStatement(
@@ -161,48 +214,5 @@ public class DoubleCheckedLockingInspection extends BaseInspection {
      }
      registerStatementError(statement, field);
    }
-
-    @Nullable
-    private PsiField findCheckedField(PsiExpression expression) {
-      if (expression instanceof PsiReferenceExpression) {
-        final PsiReferenceExpression referenceExpression =
-          (PsiReferenceExpression)expression;
-        final PsiElement target = referenceExpression.resolve();
-        if (!(target instanceof PsiField)) {
-          return null;
-        }
-        return (PsiField)target;
-      }
-      else if (expression instanceof PsiBinaryExpression) {
-        final PsiBinaryExpression binaryExpression =
-          (PsiBinaryExpression)expression;
-        final IElementType tokenType =
-          binaryExpression.getOperationTokenType();
-        if (!JavaTokenType.EQEQ.equals(tokenType)
-            && !JavaTokenType.NE.equals(tokenType)) {
-          return null;
-        }
-        final PsiExpression lhs = binaryExpression.getLOperand();
-        final PsiExpression rhs = binaryExpression.getROperand();
-        final PsiField field = findCheckedField(lhs);
-        if (field != null) {
-          return field;
-        }
-        return findCheckedField(rhs);
-      }
-      else if (expression instanceof PsiPrefixExpression) {
-        final PsiPrefixExpression prefixExpression =
-          (PsiPrefixExpression)expression;
-        final IElementType tokenType =
-          prefixExpression.getOperationTokenType();
-        if (!JavaTokenType.EXCL.equals(tokenType)) {
-          return null;
-        }
-        return findCheckedField(prefixExpression.getOperand());
-      }
-      else {
-        return null;
-      }
-    }
  }
 }
@@ -6,9 +6,9 @@ discussion of double-checked locking and why it is unsafe, see
 ">http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html</a>
 <!-- tooltip end -->
 <p>
-Use the checkbox below to ignore double-checked locking on volatile fields. Using
-a volatile field for double-checked locking works correctly on virtual machines which
-implement the new Java Memory Model.
+Use the checkbox below to ignore double-checked locking on <b>volatile</b> fields. Using
+a <b>volatile</b> field for double-checked locking works correctly on virtual machines which
+implement the Java Memory Model.
 <p>

 </body>
@@ -0,0 +1,19 @@
+public class Simple
+{
+  private static volatile Object s_instance;
+
+  public static Object foo()
+  {
+    if(s_instance == null)
+    {
+      synchronized(Simple.class)
+      {
+        if(s_instance == null)
+        {
+          s_instance = new Object();
+        }
+      }
+    }
+    return s_instance;
+  }
+}
@@ -0,0 +1,19 @@
+public class Simple
+{
+  private static Object s_instance;
+
+  public static Object foo()
+  {
+    if<caret>(s_instance == null)
+    {
+      synchronized(Simple.class)
+      {
+        if(s_instance == null)
+        {
+          s_instance = new Object();
+        }
+      }
+    }
+    return s_instance;
+  }
+}
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2000-2014 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.
+ */
+package com.siyeh.ig.fixes.threading;
+
+import com.siyeh.InspectionGadgetsBundle;
+import com.siyeh.ig.IGQuickFixesTestCase;
+import com.siyeh.ig.threading.DoubleCheckedLockingInspection;
+
+/**
+ * @author Bas Leijdekkers
+ */
+public class MakeFieldVolatileFixTest extends IGQuickFixesTestCase {
+
+  public void testSimple() { doTest(InspectionGadgetsBundle.message("double.checked.locking.quickfix", "s_instance")); }
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    final DoubleCheckedLockingInspection inspection = new DoubleCheckedLockingInspection();
+    inspection.ignoreOnVolatileVariables = true;
+    myFixture.enableInspections(inspection);
+    myRelativePath = "threading/make_field_volatile";
+  }
+}