IDEA-61662 (Inspection "Class is a singleton" - false positive)

diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/classlayout/SingletonInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/classlayout/SingletonInspection.java
index ef73360935f9f28850ea0a53ef7196d8e67d2242..683f9bbe4092a2f8c4db8621f10d1f61f5f6eac7 100644 (file)
--- a/plugins/InspectionGadgets/src/com/siyeh/ig/classlayout/SingletonInspection.java
+++ b/plugins/InspectionGadgets/src/com/siyeh/ig/classlayout/SingletonInspection.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2007 Dave Griffith, Bas Leijdekkers
+ * Copyright 2003-2010 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.
@@ -24,16 +24,24 @@ import org.jetbrains.annotations.NotNull;
 
 public class SingletonInspection extends BaseInspection {
 
+    @Override
     @NotNull
     public String getDisplayName() {
         return InspectionGadgetsBundle.message("singleton.display.name");
     }
 
+    @Override
     @NotNull
     protected String buildErrorString(Object... infos) {
         return InspectionGadgetsBundle.message("singleton.problem.descriptor");
     }
 
+    @Override
+    public boolean runForWholeFile() {
+        return true;
+    }
+
+    @Override
     public BaseInspectionVisitor buildVisitor() {
         return new SingletonVisitor();
     }

diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/SingletonUtil.java b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/SingletonUtil.java
index 19980f61a46059ae03663d945514745277a49c20..852ea4a333ff7bedd7a7ad3721f132f94f05affb 100644 (file)
--- a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/SingletonUtil.java
+++ b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/SingletonUtil.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2005 Dave Griffith
+ * Copyright 2003-2010 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,61 +16,128 @@
 package com.siyeh.ig.psiutils;
 
 import com.intellij.psi.*;
+import com.intellij.psi.search.searches.MethodReferencesSearch;
+import com.intellij.util.Processor;
+import com.intellij.util.Query;
 import org.jetbrains.annotations.NotNull;
 
 public class SingletonUtil {
-    private SingletonUtil() {
-        super();
-    }
+
+    private SingletonUtil() {}
 
     public static boolean isSingleton(@NotNull PsiClass aClass) {
-        if (aClass.isInterface() || aClass.isEnum() || aClass.isAnnotationType()) {
+        if (aClass.isInterface() || aClass.isEnum() ||
+                aClass.isAnnotationType()) {
             return false;
         }
         if(aClass instanceof PsiTypeParameter ||
                 aClass instanceof PsiAnonymousClass){
             return false;
-        }                         
-        if (!hasConstructor(aClass)) {
+        }
+        final PsiMethod[] constructors = getIfOnlyInvisibleConstructors(aClass);
+        if (constructors.length == 0) {
             return false;
         }
-        if (hasVisibleConstructor(aClass)) {
+        final PsiField selfInstance = getIfOneStaticSelfInstance(aClass);
+        if (selfInstance == null) {
             return false;
         }
-        return containsOneStaticSelfInstance(aClass);
+        return newOnlyAssignsToStaticSelfInstance(constructors[0], selfInstance);
     }
 
-    private static boolean containsOneStaticSelfInstance(PsiClass aClass) {
+    private static PsiField getIfOneStaticSelfInstance(PsiClass aClass) {
         final PsiField[] fields = aClass.getFields();
-        int numSelfInstances = 0;
+        PsiField result = null;
         for(final PsiField field : fields){
             final String className = aClass.getQualifiedName();
-            if(field.hasModifierProperty(PsiModifier.STATIC)){
-                final PsiType type = field.getType();
-                final String fieldTypeName = type.getCanonicalText();
-                if(fieldTypeName.equals(className)){
-                    numSelfInstances++;
-                }
+            if (!field.hasModifierProperty(PsiModifier.STATIC)) {
+                continue;
             }
+            final PsiType type = field.getType();
+            final String fieldTypeName = type.getCanonicalText();
+            if (!fieldTypeName.equals(className)) {
+                continue;
+            }
+            if (result != null) {
+                return null;
+            }
+            result = field;
         }
-        return numSelfInstances == 1;
+        return result;
     }
 
-    private static boolean hasConstructor(PsiClass aClass) {
-        return aClass.getConstructors().length>0;
+    private static PsiMethod[] getIfOnlyInvisibleConstructors(PsiClass aClass) {
+        final PsiMethod[] constructors = aClass.getConstructors();
+        if (constructors.length == 0) {
+            return PsiMethod.EMPTY_ARRAY;
+        }
+        for(final PsiMethod constructor : constructors){
+            if(constructor.hasModifierProperty(PsiModifier.PUBLIC)){
+                return PsiMethod.EMPTY_ARRAY;
+            }
+            if(!constructor.hasModifierProperty(PsiModifier.PRIVATE) &&
+                    !constructor.hasModifierProperty(PsiModifier.PROTECTED)){
+                return PsiMethod.EMPTY_ARRAY;
+            }
+        }
+        return constructors;
     }
 
-    private static boolean hasVisibleConstructor(PsiClass aClass) {
-        final PsiMethod[] methods = aClass.getConstructors();
-        for(final PsiMethod method : methods){
-            if(method.hasModifierProperty(PsiModifier.PUBLIC)){
-                return true;
+    private static boolean newOnlyAssignsToStaticSelfInstance(
+            PsiMethod method, final PsiField field) {
+        final Query<PsiReference> search =
+                MethodReferencesSearch.search(method, field.getUseScope(),
+                        false);
+        final NewOnlyAssignedToFieldProcessor processor =
+                new NewOnlyAssignedToFieldProcessor(field);
+        search.forEach(processor);
+        return processor.isNewOnlyAssignedToField();
+    }
+
+    private static class NewOnlyAssignedToFieldProcessor
+            implements Processor<PsiReference> {
+
+        private boolean newOnlyAssignedToField = true;
+        private final PsiField field;
+
+        public NewOnlyAssignedToFieldProcessor(PsiField field) {
+            this.field = field;
+        }
+
+        public boolean process(PsiReference reference) {
+            final PsiElement element = reference.getElement();
+            final PsiElement parent = element.getParent();
+            if (!(parent instanceof PsiNewExpression)) {
+                newOnlyAssignedToField = false;
+                return false;
             }
-            if(!method.hasModifierProperty(PsiModifier.PRIVATE) &&
-                    !method.hasModifierProperty(PsiModifier.PROTECTED)){
+            final PsiElement grandParent = parent.getParent();
+            if (field.equals(grandParent)) {
                 return true;
             }
+            if (!(grandParent instanceof PsiAssignmentExpression)) {
+                newOnlyAssignedToField = false;
+                return false;
+            }
+            final PsiAssignmentExpression assignmentExpression =
+                    (PsiAssignmentExpression) grandParent;
+            final PsiExpression lhs = assignmentExpression.getLExpression();
+            if (!(lhs instanceof PsiReferenceExpression)) {
+                newOnlyAssignedToField = false;
+                return false;
+            }
+            final PsiReferenceExpression referenceExpression =
+                    (PsiReferenceExpression) lhs;
+            final PsiElement target = referenceExpression.resolve();
+            if (!field.equals(target)) {
+                newOnlyAssignedToField = false;
+                return false;
+            }
+            return true;
+        }
+
+        public boolean isNewOnlyAssignedToField() {
+            return newOnlyAssignedToField;
         }
-        return false;
     }
 }

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/Singleton.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/Singleton.java
deleted file mode 100644 (file)
index 81ffc1e..0000000
--- a/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/Singleton.java
+++ /dev/null
@@ -1,12 +0,0 @@
-package com.siyeh.igtest.classlayout;
-
-public class Singleton{
-    private static Singleton ourInstance = new Singleton();
-
-    public static Singleton getInstance(){
-        return ourInstance;
-    }
-
-    private Singleton(){
-    }
-}

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/Singleton.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/Singleton.java
new file mode 100644 (file)
index 0000000..b691135
--- /dev/null
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/Singleton.java
@@ -0,0 +1,40 @@
+package com.siyeh.igtest.classlayout.singleton;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class Singleton{
+    private static Singleton ourInstance = new Singleton();
+
+    public static Singleton getInstance(){
+        return ourInstance;
+    }
+
+    private Singleton(){
+    }
+}
+class NonSingleton {
+    public static final NonSingleton EMPTY = new NonSingleton(Collections.<String>emptyList());
+
+    private final List<String> values;
+
+    private NonSingleton(List<String> values) {
+        this.values = values;
+    }
+
+    public int size() {
+        return values.size();
+    }
+
+    public String get(int index) {
+        return values.get(index);
+    }
+
+    //this method makes the class a non-singleton
+    public NonSingleton add(String s) {
+        List<String> copy = new ArrayList<String>(values);
+        copy.add(s);
+        return new NonSingleton(copy);
+    }
+}
\ No newline at end of file

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/expected.xml
new file mode 100644 (file)
index 0000000..2946123
--- /dev/null
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/classlayout/singleton/expected.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<problems>
+
+  <problem>
+    <file>Singleton.java</file>
+    <line>7</line>
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Singleton</problem_class>
+    <description>Class &lt;code&gt;Singleton&lt;/code&gt; is a singleton #loc</description>
+  </problem>
+
+</problems>
\ No newline at end of file

diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/SingletonInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/SingletonInspectionTest.java
new file mode 100644 (file)
index 0000000..6025257
--- /dev/null
+++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/classlayout/SingletonInspectionTest.java
@@ -0,0 +1,11 @@
+package com.siyeh.ig.classlayout;
+
+import com.IGInspectionTestCase;
+
+public class SingletonInspectionTest extends IGInspectionTestCase {
+
+    public void test() throws Exception {
+        doTest("com/siyeh/igtest/classlayout/singleton",
+                new SingletonInspection());
+    }
+}
\ No newline at end of file