From 1a51582963469b8af6e63dcd83bc175e33681fb3 Mon Sep 17 00:00:00 2001 From: Akshat Bahety Date: Mon, 23 Apr 2018 02:08:16 +0530 Subject: [PATCH 1/5] Solution for issue #816 Fixed the issue with traversing the tree and checking out if the getInstance method is used twice under the same class. --- .../errorprone/SingleMethodSingletonRule.java | 57 +++++++++++++------ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java index ead66d667a..a47f6890b0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java @@ -4,35 +4,58 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; -import java.util.HashSet; -import java.util.Set; -import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import java.util.List; + + + +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +/** + * Returns Checks if the singleton rule is used properly. + */ public class SingleMethodSingletonRule extends AbstractJavaRule { - private Set methodset = new HashSet(); + /** + * Checks for getInstance method usage in the same class. + * @param node of ASTCLass + * @param data of Object + * @return Object + * + */ - @Override - public Object visit(ASTCompilationUnit node, Object data) { - methodset.clear(); - return super.visit(node, data); - } - @Override - public Object visit(ASTMethodDeclaration node, Object data) { - if (node.getResultType().isVoid()) { - return super.visit(node, data); - } + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if ("getInstance".equals(node.getMethodName())) { - if (!methodset.add(node.getMethodName())) { - addViolation(data, node); + String a = node.getImage(); // Get the name of the Class it's part of + System.out.println(a); + + List methods = node.findDescendantsOfType(ASTMethodDeclaration.class); // Find the name of methods in it + + System.out.println(methods); + + int count = 0; + for (ASTMethodDeclaration method : methods) { + + System.out.println(method.getName()); + if (method.getName().equals("getInstance")) { + count++; } + } + if (count > 1) { + System.out.println("error"); + addViolation(data, node); + } + + /* + Can now check if each class has only one getInstance methods than it's all sorted. + */ + return super.visit(node, data); + } } From bee7adb33a077bbd52fd31fc66c5ad78285f7ee8 Mon Sep 17 00:00:00 2001 From: Akshat Bahety Date: Mon, 23 Apr 2018 09:48:34 +0530 Subject: [PATCH 2/5] Removed Debugging parts Updated the file, removed the unnecessary debugging statements --- .../errorprone/SingleMethodSingletonRule.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java index a47f6890b0..293ecea630 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java @@ -29,31 +29,22 @@ public class SingleMethodSingletonRule extends AbstractJavaRule { public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - String a = node.getImage(); // Get the name of the Class it's part of - System.out.println(a); List methods = node.findDescendantsOfType(ASTMethodDeclaration.class); // Find the name of methods in it - System.out.println(methods); - int count = 0; for (ASTMethodDeclaration method : methods) { - - System.out.println(method.getName()); + if (method.getName().equals("getInstance")) { count++; + if(count > 1){ + addViolation(data, node); + break; + } } } - if (count > 1) { - System.out.println("error"); - addViolation(data, node); - } - - /* - Can now check if each class has only one getInstance methods than it's all sorted. - */ return super.visit(node, data); From c137ebaa60c0121ad6e82f96db86f3498f46366e Mon Sep 17 00:00:00 2001 From: Akshat Bahety Date: Mon, 23 Apr 2018 10:04:32 +0530 Subject: [PATCH 3/5] CheckStyle Updated --- .../java/rule/errorprone/SingleMethodSingletonRule.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java index 293ecea630..8336ba8c0d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/SingleMethodSingletonRule.java @@ -34,11 +34,11 @@ public class SingleMethodSingletonRule extends AbstractJavaRule { int count = 0; for (ASTMethodDeclaration method : methods) { - + if (method.getName().equals("getInstance")) { count++; - if(count > 1){ - addViolation(data, node); + if (count > 1) { + addViolation(data, node); break; } } From 92ecdde2f4448c0ea4541c76baf64e586eb892f0 Mon Sep 17 00:00:00 2001 From: Akshat Bahety Date: Mon, 23 Apr 2018 10:29:43 +0530 Subject: [PATCH 4/5] Singleton Test Case Added a test case to check the singleton rule for nested classes --- .../errorprone/xml/SingleMethodSingleton.xml | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/SingleMethodSingleton.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/SingleMethodSingleton.xml index a04d3e2800..ea2cc5b43d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/SingleMethodSingleton.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/SingleMethodSingleton.xml @@ -48,4 +48,27 @@ private static Singleton instance = null; ]]> - \ No newline at end of file + + + + 0 + + + + + + + + + From dac6d8c1a59262bb2103bf206b03dcda0578efe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 23 Apr 2018 02:15:45 -0300 Subject: [PATCH 5/5] Update changelog, refs #1044 --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index f1c6ab85a9..40f41e4d2e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -71,6 +71,8 @@ we have measured up to 10% improvements during Type Resolution, Symbol Table ana * java-codestyle * [#1003](https://github.com/pmd/pmd/issues/1003): \[java] UnnecessaryConstructor triggered on required empty constructor (Dagger @Inject) * [#1023](https://github.com/pmd/pmd/issues/1023): \[java] False positive for useless parenthesis +* java-errorprone + * [#816](https://github.com/pmd/pmd/issues/816): \[java] SingleMethodSingleton false positives with inner classes * java-performance * [#586](https://github.com/pmd/pmd/issues/586): \[java] AvoidUsingShortType erroneously triggered on overrides of 3rd party methods @@ -90,4 +92,5 @@ we have measured up to 10% improvements during Type Resolution, Symbol Table ana * [#1012](https://github.com/pmd/pmd/pull/1012): \[java] JUnitAssertionsShouldIncludeMessage - False positive with assertEquals and JUnit5 - [BBG](https://github.com/djydewang) * [#1024](https://github.com/pmd/pmd/pull/1024): \[java]Issue 558: Properlogger for enums - [Utku Cuhadaroglu](https://github.com/utkuc) * [#1041](https://github.com/pmd/pmd/pull/1041): \[java] Make BasicProjectMemoizer thread safe. - [bergander](https://github.com/bergander) +* [#1044](https://github.com/pmd/pmd/pull/1044): \[java] Fix for issue #816 - [Akshat Bahety](https://github.com/akshatbahety)