From 0fb697a50b0edc7b1bab37d57220e16b11a19ed0 Mon Sep 17 00:00:00 2001
From: Xavier Le Vourch <xlv@users.sourceforge.net>
Date: Thu, 11 Dec 2008 06:32:25 +0000
Subject: [PATCH] Fixed bug 2404700 - UseSingleton should not act on enums

git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6746 51baf565-9d33-0410-a72c-fc3788e3496d
---
 pmd/etc/changelog.txt                         |  1 +
 .../pmd/rules/design/xml/UseSingleton.xml     | 36 ++++++-
 .../pmd/rules/design/UseSingleton.java        | 99 +++++++++----------
 3 files changed, 81 insertions(+), 55 deletions(-)

diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt
index 51d450333b..bdcdccc9ed 100644
--- a/pmd/etc/changelog.txt
+++ b/pmd/etc/changelog.txt
@@ -6,6 +6,7 @@ Fixed bug 2230809 - False +: ClassWithOnlyPrivateConstructorsShouldBeFinal
 Fixed bug 2338341 - ArrayIndexOutOfBoundsException in CPD (on Ruby)
 Fixed bug 2315599 - False +: UseSingleton with class containing constructor
 Fixed bug 1955852 - false positives for UnusedPrivateMethod & UnusedLocalVariable
+Fixed bug 2404700 - UseSingleton should not act on enums
 
 October 12, 2008 - 4.2.4:
 
diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/UseSingleton.xml b/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/UseSingleton.xml
index 653147ae54..b04c300e1e 100644
--- a/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/UseSingleton.xml
+++ b/pmd/regress/test/net/sourceforge/pmd/rules/design/xml/UseSingleton.xml
@@ -134,6 +134,36 @@ public class UseSingleton extends Exception {
 }
 			   ]]></code>
     </test-code>    
-    
-    
-</test-data>
\ No newline at end of file
+    <test-code>
+        <description><![CDATA[
+inner should be singleton since all static, public constructor
+     ]]></description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+ static class Bar {
+   public Bar() { }
+   public static void doSomething() {}
+ }
+ public void doSomething() {}
+}
+     ]]></code>
+    </test-code>
+    <test-code>
+        <description><![CDATA[
+[ 2404700 ] UseSingleton should not act on enums
+     ]]></description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public enum EnumTest {
+	A, B;
+
+	EnumTest() { }
+
+	public static void main(String[] args) {
+		System.out.println(EnumTest.A);
+	}
+}
+     ]]></code>
+    </test-code>
+</test-data>
diff --git a/pmd/src/net/sourceforge/pmd/rules/design/UseSingleton.java b/pmd/src/net/sourceforge/pmd/rules/design/UseSingleton.java
index c5f7b1b8f9..8f7a078348 100644
--- a/pmd/src/net/sourceforge/pmd/rules/design/UseSingleton.java
+++ b/pmd/src/net/sourceforge/pmd/rules/design/UseSingleton.java
@@ -4,71 +4,66 @@
 package net.sourceforge.pmd.rules.design;
 
 import net.sourceforge.pmd.AbstractRule;
+import net.sourceforge.pmd.ast.ASTClassOrInterfaceBody;
 import net.sourceforge.pmd.ast.ASTClassOrInterfaceDeclaration;
 import net.sourceforge.pmd.ast.ASTClassOrInterfaceType;
-import net.sourceforge.pmd.ast.ASTCompilationUnit;
 import net.sourceforge.pmd.ast.ASTConstructorDeclaration;
 import net.sourceforge.pmd.ast.ASTFieldDeclaration;
 import net.sourceforge.pmd.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.ast.ASTResultType;
+import net.sourceforge.pmd.ast.Node;
 
 public class UseSingleton extends AbstractRule {
 
-    private boolean isOK;
-    private int methodCount;
+    @Override
+    public Object visit(ASTClassOrInterfaceBody decl, Object data) {
+        if (decl.jjtGetParent() instanceof ASTClassOrInterfaceDeclaration) {
+            ASTClassOrInterfaceDeclaration parent = (ASTClassOrInterfaceDeclaration) decl.jjtGetParent();
+            if (parent.isAbstract() || parent.isInterface()) {
+                return super.visit(decl, data);
+            }
+            int i = decl.jjtGetNumChildren();
+            int methodCount = 0;
+            boolean isOK = false;
+            while (i > 0) {
+                Node n = decl.jjtGetChild(--i).jjtGetChild(0);
+                if (n instanceof ASTFieldDeclaration) {
+                    if (!((ASTFieldDeclaration) n).isStatic()) {
+                        isOK = true;
+                        break;
+                    }
+                } else if (n instanceof ASTConstructorDeclaration) {
+                    if (((ASTConstructorDeclaration) n).isPrivate()) {
+                        isOK = true;
+                        break;
+                    }
+                } else if (n instanceof ASTMethodDeclaration) {
+                    ASTMethodDeclaration m = (ASTMethodDeclaration) n;
+                    if (!m.isPrivate()) {
+                        methodCount++;
+                    }
+                    if (!m.isStatic()) {
+                        isOK = true;
+                        break;
+                    }
 
-    public Object visit(ASTCompilationUnit cu, Object data) {
-        methodCount = 0;
-        isOK = false;
-        Object result = cu.childrenAccept(this, data);
-        if (!isOK && methodCount > 0) {
-            addViolation(data, cu);
-        }
+                    // TODO use symbol table
+                    if (m.getMethodName().equals("suite")) {
+                        ASTResultType res = m.getResultType();
+                        ASTClassOrInterfaceType c = res.getFirstChildOfType(ASTClassOrInterfaceType.class);
+                        if (c != null && c.hasImageEqualTo("Test")) {
+                            isOK = true;
+                            break;
+                        }
+                    }
 
-        return result;
-    }
-
-    public Object visit(ASTFieldDeclaration decl, Object data) {
-        if (!decl.isStatic()) {
-            isOK = true;
-        }
-        return data;
-    }
-
-    public Object visit(ASTConstructorDeclaration decl, Object data) {
-        if (decl.isPrivate()) {
-            isOK = true;
-        }
-        return data;
-    }
-
-    public Object visit(ASTClassOrInterfaceDeclaration decl, Object data) {
-        if (decl.isAbstract()) {
-            isOK = true;
+                }
+            }
+            if (!isOK && methodCount > 0) {
+                addViolation(data, decl);
+            }
         }
         return super.visit(decl, data);
     }
 
-    public Object visit(ASTMethodDeclaration decl, Object data) {
-        // Private method does no count
-	if ( ! decl.isPrivate() ) {
-		methodCount++;
-	}
-
-        if (!isOK && !decl.isStatic()  ) {
-            isOK = true;
-        }
-
-        // TODO use symbol table
-        if (decl.getMethodName().equals("suite")) {
-            ASTResultType res = decl.getFirstChildOfType(ASTResultType.class);
-            ASTClassOrInterfaceType c = res.getFirstChildOfType(ASTClassOrInterfaceType.class);
-            if (c != null && c.hasImageEqualTo("Test")) {
-                isOK = true;
-            }
-        }
-
-        return data;
-    }
-
 }