From ec79ac100490eba5e270543fa1c5b52ea76fd1a2 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@microdoc.com>
Date: Mon, 17 Sep 2018 13:20:04 +0200
Subject: [PATCH] [apex] AvoidGlobalModifierRule gives warning even when its a
 webservice - false positive Fixes #1348

---
 docs/pages/release_notes.md                   |  2 +
 .../AvoidGlobalModifierRule.java              | 37 ++++++++++++++++-
 .../bestpractices/xml/AvoidGlobalModifier.xml | 41 ++++++++++++++++++-
 3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md
index 1028f56569..ea125c2951 100644
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@@ -65,6 +65,8 @@ the breaking API changes will be performed in 7.0.0.
 
 ### Fixed Issues
 
+*   apex-bestpractices
+    *   [#1348](https://github.com/pmd/pmd/issues/1348): \[apex] AvoidGlobalModifierRule gives warning even when its a webservice - false positive
 *   java-codestyle
     *   [#1329](https://github.com/pmd/pmd/issues/1329): \[java] FieldNamingConventions: false positive in serializable class with serialVersionUID
     *   [#1334](https://github.com/pmd/pmd/issues/1334): \[java] LinguisticNaming should support AtomicBooleans
diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java
index 82416f0697..9d583e7f0c 100644
--- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java
+++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/AvoidGlobalModifierRule.java
@@ -4,14 +4,18 @@
 
 package net.sourceforge.pmd.lang.apex.rule.bestpractices;
 
-import static apex.jorje.semantic.symbol.type.ModifierTypeInfos.GLOBAL;
+import java.util.List;
 
+import net.sourceforge.pmd.lang.apex.ast.ASTAnnotation;
+import net.sourceforge.pmd.lang.apex.ast.ASTMethod;
 import net.sourceforge.pmd.lang.apex.ast.ASTModifierNode;
 import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
 import net.sourceforge.pmd.lang.apex.ast.ASTUserInterface;
 import net.sourceforge.pmd.lang.apex.ast.ApexNode;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
 
+import apex.jorje.semantic.symbol.type.ModifierTypeInfos;
+
 public class AvoidGlobalModifierRule extends AbstractApexRule {
 
     public AvoidGlobalModifierRule() {
@@ -33,10 +37,39 @@ public class AvoidGlobalModifierRule extends AbstractApexRule {
     private Object checkForGlobal(ApexNode<?> node, Object data) {
         ASTModifierNode modifierNode = node.getFirstChildOfType(ASTModifierNode.class);
 
-        if (modifierNode != null && modifierNode.getNode().getModifiers().has(GLOBAL)) {
+        if (isGlobal(modifierNode) && !hasRestAnnotation(modifierNode) && !hasWebServices(node)) {
             addViolation(data, node);
         }
 
         return data;
     }
+
+    private boolean hasWebServices(ApexNode<?> node) {
+        List<ASTMethod> methods = node.findChildrenOfType(ASTMethod.class);
+        for (ASTMethod method : methods) {
+            ASTModifierNode methodModifier = method.getFirstChildOfType(ASTModifierNode.class);
+            if (isWebService(methodModifier)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private boolean isWebService(ASTModifierNode modifierNode) {
+        return modifierNode != null && modifierNode.getNode().getModifiers().has(ModifierTypeInfos.WEB_SERVICE);
+    }
+
+    private boolean isGlobal(ASTModifierNode modifierNode) {
+        return modifierNode != null && modifierNode.getNode().getModifiers().has(ModifierTypeInfos.GLOBAL);
+    }
+
+    private boolean hasRestAnnotation(ASTModifierNode modifierNode) {
+        List<ASTAnnotation> annotations = modifierNode.findChildrenOfType(ASTAnnotation.class);
+        for (ASTAnnotation annotation : annotations) {
+            if (annotation.hasImageEqualTo("RestResource")) {
+                return true;
+            }
+        }
+        return false;
+    }
 }
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/AvoidGlobalModifier.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/AvoidGlobalModifier.xml
index ead264bc97..28f5acf78c 100644
--- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/AvoidGlobalModifier.xml
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/AvoidGlobalModifier.xml
@@ -48,5 +48,44 @@ global class Foo {
 }
 		]]></code>
 	</test-code>
-    
+
+    <test-code>
+        <description>#1348 [apex] AvoidGlobalModifierRule gives warning even when its a REST webservice - false positive</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+@RestResource(urlMapping = '/partners/submit/*')
+global class Generic_LoanCreation {
+    @HttpPost
+    global static void createLoanApplication() {
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>#1348 [apex] AvoidGlobalModifierRule gives warning even when its a SOAP webservice - false positive</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+global class Generic_LoanCreation {
+    webservice static void createLoanApplication() {
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Simple public and non-global class</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class myOuterClass {
+   // Additional myOuterClass code here
+   class myInnerClass {
+     // myInnerClass code here
+   }
+
+   void myMethod() {
+   }
+}
+        ]]></code>
+    </test-code>
 </test-data>