From 568fe66ba0661a9bcd3bd5dcd882bac411a37c6a Mon Sep 17 00:00:00 2001
From: Sergey <sergey.gorbaty@salesforce.com>
Date: Tue, 31 Jan 2017 15:41:39 -0800
Subject: [PATCH] Excluding count from CRUD/FLS checks

---
 .../rule/security/ApexCRUDViolationRule.java  |  4 +-
 .../rule/security/xml/ApexCRUDViolation.xml   | 60 ++++++++++++++-----
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java
index c0d7a9511b..ea21068660 100644
--- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java
+++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java
@@ -456,6 +456,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
     }
 
     private void checkForAccessibility(final ASTSoqlExpression node, Object data) {
+        final boolean isCount = node.getNode().getCanonicalQuery().startsWith("SELECT COUNT()");
+
         final HashSet<ASTMethodCallExpression> prevCalls = getPreviousMethodCalls(node);
         for (ASTMethodCallExpression prevCall : prevCalls) {
             collectCRUDMethodLevelChecks(prevCall);
@@ -467,7 +469,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule {
         final ASTMethod wrappingMethod = node.getFirstParentOfType(ASTMethod.class);
         final ASTUserClass wrappingClass = node.getFirstParentOfType(ASTUserClass.class);
 
-        if ((wrappingClass != null && Helper.isTestMethodOrClass(wrappingClass))
+        if (isCount || (wrappingClass != null && Helper.isTestMethodOrClass(wrappingClass))
                 || (wrappingMethod != null && Helper.isTestMethodOrClass(wrappingMethod))) {
             return;
         }
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml
index 8ced37f6d1..c0ac3ed716 100644
--- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <test-data>
- 
+
 	<test-code>
 		<description>Proper CRUD,FLS via upsert</description>
 		<expected-problems>0</expected-problems>
@@ -468,8 +468,8 @@ public class Foo {
 }
 		]]></code>
 	</test-code>
-	
-		<test-code>
+
+	<test-code>
 		<description>No issues found in test classes</description>
 		<expected-problems>0</expected-problems>
 		<code><![CDATA[
@@ -480,7 +480,7 @@ public class FooTest {
 }
 		]]></code>
 	</test-code>
-	
+
 	<test-code>
 		<description>Control flow based CRUD,FLS check</description>
 		<expected-problems>0</expected-problems>
@@ -499,7 +499,7 @@ public class Foo {
 	}
 	
 } ]]></code>
-	</test-code> 
+	</test-code>
 	<test-code>
 		<description>Control flow based CRUD,FLS check recursive</description>
 		<expected-problems>0</expected-problems>
@@ -523,8 +523,9 @@ public class Foo {
 	
 } ]]></code>
 	</test-code>
- 	<test-code>
-		<description>Control flow constructor based CRUD,FLS check</description>
+	<test-code>
+		<description>Control flow constructor based CRUD,FLS check
+		</description>
 		<expected-problems>0</expected-problems>
 		<code><![CDATA[ 
 public class Foo { 
@@ -541,8 +542,8 @@ public class Foo {
 	}
 	
 } ]]></code>
-	</test-code> 
-	
+	</test-code>
+
 	<test-code>
 		<description>Control flow accessibility CRUD check</description>
 		<expected-problems>0</expected-problems>
@@ -562,8 +563,8 @@ public class Foo {
 	}
 } 
 ]]></code>
-	</test-code> 
-	
+	</test-code>
+
 	<test-code>
 		<description>Control flow substitute CRUD check</description>
 		<expected-problems>0</expected-problems>
@@ -583,7 +584,7 @@ public class Foo {
 } 
 ]]></code>
 	</test-code>
-	
+
 	<test-code>
 		<description>Forgot to call the CRUD check</description>
 		<expected-problems>1</expected-problems>
@@ -603,7 +604,9 @@ public class Foo {
 ]]></code>
 	</test-code>
 	<test-code>
-		<description>Control flow substitute CRUD check should fail when check follows SOQL</description>
+		<description>Control flow substitute CRUD check should fail when check
+			follows SOQL
+		</description>
 		<expected-problems>1</expected-problems>
 		<code><![CDATA[ 
 public class Foo { 	
@@ -619,7 +622,7 @@ public class Foo {
 	}
 } 
 ]]></code>
-	</test-code> 
+	</test-code>
 
 	<test-code>
 		<description>Control flow with nested statementsL</description>
@@ -643,6 +646,33 @@ public class Foo {
 }
 	
 ]]></code>
-	</test-code>	
+	</test-code>
+
+	<test-code>
+		<description>Count does not expose data and CRUD checks are
+			unnecessary
+		</description>
+		<expected-problems>0</expected-problems>
+		<code><![CDATA[ 
+public class Foo { 	
+	public Integer getBaz() {
+	 	return  [SELECT count() FROM Contact];
+	} 
+} 
+]]></code>
+	</test-code>
+
+	<test-code>
+		<description>Count does not leak data and CRUD checks are unnecessary
+		</description>
+		<expected-problems>0</expected-problems>
+		<code><![CDATA[ 
+public class Foo { 	
+	public void getBaz() {
+		Integer countAdmins = [SELECT count() FROM Profile WHERE Name = 'System Administrator'];
+	} 
+} 
+]]></code>
+	</test-code>
 
 </test-data>