Merge branch 'pr-191'

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java

@@ -4,6 +4,9 @@
 
 package net.sourceforge.pmd.lang.apex.rule.security;
 
+import java.util.List;
+
+import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
 import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
 import net.sourceforge.pmd.lang.apex.ast.ApexNode;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
@@ -27,19 +30,51 @@ public class ApexSharingViolationsRule extends AbstractApexRule {
     @Override
     public Object visit(ASTUserClass node, Object data) {
         if (!Helper.isTestMethodOrClass(node)) {
-            checkForSharingDeclaration(node, data);
+            boolean sharingFound = isSharingPresent(node);
+            checkForSharingDeclaration(node, data, sharingFound);
+            checkForDatabaseMethods(node, data, sharingFound);
         }
         return data;
     }
 
+    /**
+     * Check if class contains any Database.query / Database.insert [ Database.*
+     * ] methods
+     * 
+     * @param node
+     * @param data
+     */
+    private void checkForDatabaseMethods(ASTUserClass node, Object data, boolean sharingFound) {
+        List<ASTMethodCallExpression> calls = node.findDescendantsOfType(ASTMethodCallExpression.class);
+        for (ASTMethodCallExpression call : calls) {
+            if (Helper.isMethodName(call, "Database", Helper.ANY_METHOD)) {
+                if (!sharingFound) {
+                    addViolation(data, node);
+                }
+            }
+        }
+    }
+
     /**
      * Check if class has no sharing declared
      * 
      * @param node
      * @param data
      */
-    private void checkForSharingDeclaration(ApexNode<?> node, Object data) {
+    private void checkForSharingDeclaration(ApexNode<?> node, Object data, boolean sharingFound) {
         final boolean foundAnyDMLorSOQL = Helper.foundAnyDML(node) && Helper.foundAnySOQLorSOSL(node);
+        if (!sharingFound && !Helper.isTestMethodOrClass(node) && foundAnyDMLorSOQL) {
+            addViolation(data, node);
+        }
+    }
+
+    /**
+     * Does class have sharing keyword declared?
+     * 
+     * @param node
+     * @return
+     */
+    private boolean isSharingPresent(ApexNode<?> node) {
         boolean sharingFound = false;
 
         for (ModifierOrAnnotationTypeInfo type : node.getNode().getDefiningType().getModifiers().all()) {
@@ -51,10 +86,7 @@ public class ApexSharingViolationsRule extends AbstractApexRule {
             }
 
         }
-
-        if (!sharingFound && !Helper.isTestMethodOrClass(node) && foundAnyDMLorSOQL) {
-            addViolation(data, node);
-        }
+        return sharingFound;
     }
 
 }

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/Helper.java

@@ -39,6 +39,8 @@ import apex.jorje.semantic.ast.statement.VariableDeclaration;
  *
  */
 public final class Helper {
+    protected static final String ANY_METHOD = "*";
+
     private Helper() {
         throw new AssertionError("Can't instantiate helper classes");
     }
@@ -99,13 +101,15 @@ public final class Helper {
             final String methodName) {
         final ASTReferenceExpression reference = methodNode.getFirstChildOfType(ASTReferenceExpression.class);
         if (reference.getNode().getJadtIdentifiers().size() == 1) {
-            if (reference.getNode().getJadtIdentifiers().get(0).value.equalsIgnoreCase(className)
-                    && isMethodName(methodNode, methodName)) {
-                return true;
+            if (reference.getNode().getJadtIdentifiers().get(0).value.equalsIgnoreCase(className)) {
+                if (methodName.equals(ANY_METHOD) || isMethodName(methodNode, methodName)) {
+                    return true;
+                }
             }
         }
 
         return false;
+
     }
 
     static boolean isMethodName(final ASTMethodCallExpression m, final String methodName) {

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSharingViolations.xml

@@ -27,6 +27,32 @@ public class Foo {
 }
 		]]></code>
 	</test-code>
+	
+	<test-code>
+		<description>Apex class without any sharing declared with Database method
+		</description>
+		<expected-problems>1</expected-problems>
+		<code><![CDATA[
+public class Foo {
+	public List<SObject> test1() {
+		return Database.query("Select Id from Account LIMIT 100");
+	}
+}
+		]]></code>
+	</test-code>
+	
+	<test-code>
+		<description>Apex class with sharing and Database method
+		</description>
+		<expected-problems>0</expected-problems>
+		<code><![CDATA[
+public with sharing class Foo {
+	public List<SObject> test1() {
+		return Database.query("Select Id from Account LIMIT 100");
+	}
+}
+		]]></code>
+	</test-code>
 
 	<test-code>
 		<description>Apex class with explicit sharing with DML</description>

src/site/markdown/overview/changelog.md

@@ -274,4 +274,5 @@ to avoid XSS attacks.
 *   [#181](https://github.com/pmd/pmd/pull/181): \[apex] Control flow based CRUD rule checking
 *   [#184](https://github.com/pmd/pmd/pull/184): \[apex] Improving open redirect detection for static fields & assignment operations
 *   [#189](https://github.com/pmd/pmd/pull/189): \[apex] Bug fix of SOQL concatenated vars detection
+*   [#191](https://github.com/pmd/pmd/pull/191): \[apex] Detection of sharing violation when Database. methods are used