IDEA-89271 (RFE: SynchronizedMethod inspection warn about overridden methods)
authorBas Leijdekkers <basleijdekkers@gmail.com>
Fri, 27 Jul 2012 16:09:19 +0000 (18:09 +0200)
committerBas Leijdekkers <basleijdekkers@gmail.com>
Fri, 27 Jul 2012 16:09:19 +0000 (18:09 +0200)
plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties
plugins/InspectionGadgets/src/com/siyeh/ig/threading/SynchronizedMethodInspection.java
plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SynchronizedMethodInspection.java [deleted file]
plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/SynchronizedMethod.java [new file with mode: 0644]
plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/expected.xml [new file with mode: 0644]
plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/SynchronizedMethodInspectionTest.java [new file with mode: 0644]

index e3a6bb07fcfa90322ce153a4b5519058d33e502e..85e09b595ccb57927ff319520d8331203a739799 100644 (file)
@@ -1217,6 +1217,7 @@ object.notify.replace.quickfix=Replace with 'notifyAll()'
 safe.lock.problem.descriptor=''{0}'' should be locked in front of a ''try'' block and unlocked in the corresponding ''finally'' block #loc
 synchronized.method.problem.descriptor=Method ''{0}()'' declared <code>#ref</code> #loc
 synchronized.method.include.option=Include native methods
+synchronized.method.ignore.synchronized.super.option=Ignore overrides synchronized methods
 synchronized.method.move.quickfix=Move synchronization into method
 thread.run.replace.quickfix=Replace with 'start()'
 volatile.field.problem.descriptor=Volatile field <code>#ref</code> of type ''{0}'' #loc
index 34f4e0f938f5e46443358543623987a32625e374..993073b5d6f0188e4990dc79446916278e28d78c 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2011 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.
@@ -16,7 +16,7 @@
 package com.siyeh.ig.threading;
 
 import com.intellij.codeInspection.ProblemDescriptor;
-import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel;
+import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.CodeStyleManager;
@@ -37,6 +37,8 @@ public class SynchronizedMethodInspection extends BaseInspection {
    */
   public boolean m_includeNativeMethods = true;
 
+  @SuppressWarnings("PublicField")
+  public boolean ignoreSynchronizedSuperMethods = true;
 
   @Override
   @NotNull
@@ -69,13 +71,16 @@ public class SynchronizedMethodInspection extends BaseInspection {
 
   @Override
   public JComponent createOptionsPanel() {
-    return new SingleCheckboxOptionsPanel(InspectionGadgetsBundle.message(
-      "synchronized.method.include.option"),
-                                          this, "m_includeNativeMethods");
+    final MultipleCheckboxOptionsPanel panel = new MultipleCheckboxOptionsPanel(this);
+    panel.addCheckbox(InspectionGadgetsBundle.message("synchronized.method.include.option"), "m_includeNativeMethods");
+    panel.addCheckbox(InspectionGadgetsBundle.message("synchronized.method.ignore.synchronized.super.option"),
+                      "ignoreSynchronizedSuperMethods");
+    return panel;
   }
 
   private static class SynchronizedMethodFix extends InspectionGadgetsFix {
 
+    @Override
     @NotNull
     public String getName() {
       return InspectionGadgetsBundle.message(
@@ -86,11 +91,10 @@ public class SynchronizedMethodInspection extends BaseInspection {
     public void doFix(Project project, ProblemDescriptor descriptor)
       throws IncorrectOperationException {
       final PsiElement nameElement = descriptor.getPsiElement();
-      final PsiModifierList modiferList =
-        (PsiModifierList)nameElement.getParent();
-      assert modiferList != null;
-      final PsiMethod method = (PsiMethod)modiferList.getParent();
-      modiferList.setModifierProperty(PsiModifier.SYNCHRONIZED, false);
+      final PsiModifierList modifierList = (PsiModifierList)nameElement.getParent();
+      assert modifierList != null;
+      final PsiMethod method = (PsiMethod)modifierList.getParent();
+      modifierList.setModifierProperty(PsiModifier.SYNCHRONIZED, false);
       assert method != null;
       final PsiCodeBlock body = method.getBody();
       if (body == null) {
@@ -102,21 +106,15 @@ public class SynchronizedMethodInspection extends BaseInspection {
         final PsiClass containingClass = method.getContainingClass();
         assert containingClass != null;
         final String className = containingClass.getName();
-        replacementText = "{ synchronized(" + className + ".class){" +
-                          text.substring(1) + '}';
+        replacementText = "{ synchronized(" + className + ".class){" + text.substring(1) + '}';
       }
       else {
-        replacementText = "{ synchronized(this){" + text.substring(1) +
-                          '}';
+        replacementText = "{ synchronized(this){" + text.substring(1) + '}';
       }
-      final PsiElementFactory elementFactory =
-        JavaPsiFacade.getElementFactory(project);
-      final PsiCodeBlock block =
-        elementFactory.createCodeBlockFromText(replacementText,
-                                               null);
+      final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project);
+      final PsiCodeBlock block = elementFactory.createCodeBlockFromText(replacementText, null);
       body.replace(block);
-      final CodeStyleManager codeStyleManager =
-        CodeStyleManager.getInstance(project);
+      final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(project);
       codeStyleManager.reformat(method);
     }
   }
@@ -125,14 +123,20 @@ public class SynchronizedMethodInspection extends BaseInspection {
 
     @Override
     public void visitMethod(@NotNull PsiMethod method) {
-      //no call to super, so we don't drill into anonymous classes
       if (!method.hasModifierProperty(PsiModifier.SYNCHRONIZED)) {
         return;
       }
-      if (!m_includeNativeMethods &&
-          method.hasModifierProperty(PsiModifier.NATIVE)) {
+      if (!m_includeNativeMethods && method.hasModifierProperty(PsiModifier.NATIVE)) {
         return;
       }
+      if (ignoreSynchronizedSuperMethods) {
+        final PsiMethod[] superMethods = method.findSuperMethods();
+        for (final PsiMethod superMethod : superMethods) {
+          if (superMethod.hasModifierProperty(PsiModifier.SYNCHRONIZED)) {
+            return;
+          }
+        }
+      }
       registerModifierError(PsiModifier.SYNCHRONIZED, method, method);
     }
   }
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SynchronizedMethodInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SynchronizedMethodInspection.java
deleted file mode 100644 (file)
index 11deb79..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-package com.siyeh.igtest.threading;
-
-public class SynchronizedMethodInspection {
-    public synchronized void fooBar() {
-        System.out.println("foo");
-    }
-
-    public static synchronized void bar() {
-        System.out.println("foo");
-    }
-
-    public synchronized native void fooBaz();
-}
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/SynchronizedMethod.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/SynchronizedMethod.java
new file mode 100644 (file)
index 0000000..6445a4f
--- /dev/null
@@ -0,0 +1,21 @@
+package com.siyeh.igtest.threading.synchronized_method;
+
+public class SynchronizedMethod {
+    public synchronized void fooBar() {
+        System.out.println("foo");
+    }
+
+    public static synchronized void bar() {
+        System.out.println("foo");
+    }
+
+    public synchronized native void fooBaz();
+
+    static class X extends SynchronizedMethod {
+
+        @Override
+        public synchronized void fooBar() {
+            super.fooBar();
+        }
+    }
+}
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/synchronized_method/expected.xml
new file mode 100644 (file)
index 0000000..51e542c
--- /dev/null
@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<problems>
+  <problem>
+    <file>SynchronizedMethod.java</file>
+    <line>4</line>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'synchronized' method</problem_class>
+    <description>Method 'fooBar()' declared &lt;code&gt;synchronized&lt;/code&gt; #loc</description>
+  </problem>
+
+  <problem>
+    <file>SynchronizedMethod.java</file>
+    <line>8</line>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">'synchronized' method</problem_class>
+    <description>Method 'bar()' declared &lt;code&gt;synchronized&lt;/code&gt; #loc</description>
+  </problem>
+
+</problems>
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/SynchronizedMethodInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/threading/SynchronizedMethodInspectionTest.java
new file mode 100644 (file)
index 0000000..0ff23a2
--- /dev/null
@@ -0,0 +1,13 @@
+package com.siyeh.ig.threading;
+
+import com.siyeh.ig.IGInspectionTestCase;
+
+public class SynchronizedMethodInspectionTest extends IGInspectionTestCase {
+
+  public void test() throws Exception {
+    final SynchronizedMethodInspection tool = new SynchronizedMethodInspection();
+    tool.m_includeNativeMethods = false;
+    tool.ignoreSynchronizedSuperMethods = true;
+    doTest("com/siyeh/igtest/threading/synchronized_method", tool);
+  }
+}
\ No newline at end of file