Add Apex and VisualForce rules (#3085)

* Add apex and VF security rules

* Remove best practice rule

* Clean up

* Add performance and best practice rules

* Rename AbsoluteUrls rule to use Pascal Case

* Update technology to Salesforce

* Update ids and metadata

* Use standard folder structure

* Update ids and metadata for VF rules

* Fix lints

* Move best practice rule

* Fix testsuite

* Update min-version

---------

Co-authored-by: Claudio <claudio@r2c.dev>
Co-authored-by: Claudio <me@p4p3r.dev>

apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.cls

@@ -0,0 +1,49 @@
+/*
+ * global javadoc
+ * javadoc with global in it
+ * javadoc global
+ */
+// ruleid: global-access-modifiers
+global without sharing class GlobalAccessModifiers {
+	// ruleid: global-access-modifiers
+	global String globalInstanceVariable = 'globalInstanceVariable';
+
+	// ok: global-access-modifiers
+	public String publicInstanceVariable = 'publicInstanceVariable';
+
+	// ok: global-access-modifiers
+	private String privateInstanceVariable = 'privateInstanceVariable';
+
+	// ok: global-access-modifiers
+	// This is a test comment that has the word global in it
+	// ruleid: global-access-modifiers
+	global static void myGlobalMethod() { }
+
+	// ok: global-access-modifiers
+	public static void myPublicMethod() { }
+
+	// ok: global-access-modifiers
+	private static void myPrivateMethod() { }
+
+	// ok: global-access-modifiers
+	// This is another test comment with global
+	// ruleid: global-access-modifiers
+	global with sharing class TestGlobalClass { }
+
+	// ok: global-access-modifiers
+	public with sharing class TestPublicClass { }
+
+	// ok: global-access-modifiers
+	private without sharing class SystemMode { }
+
+	// ok: global-access-modifiers
+	// Global test comment - last one
+	// ruleid: global-access-modifiers
+	global static String globalStaticVariable = 'globalStaticVariable';
+
+	// ok: global-access-modifiers
+	public static String publicStaticVariable = 'publicStaticVariable';
+
+	// ok: global-access-modifiers
+	private static String privateStaticVariable = 'privateStaticVariable';
+}

apex/lang/best-practice/ncino/accessModifiers/GlobalAccessModifiers.yaml

@@ -0,0 +1,25 @@
+rules:
+  - id: global-access-modifiers
+    min-version: 1.44.0
+    severity: WARNING
+    languages:
+      - apex
+    metadata:
+      cwe:
+      - 'CWE-284: Improper Access Control'
+      category: best-practice
+      technology:
+      - salesforce
+      references:
+      - https://cwe.mitre.org/data/definitions/284.html
+    message: >-
+      Global classes, methods, and variables should be avoided (especially in managed packages) as they can
+      never be deleted or changed in signature. Always check twice if something needs to be global.
+    patterns:
+      - pattern-regex: global [A-Za-z0-9_]{3,}
+      - pattern-not-regex: //(\s+([a-zA-Z]+\s+)+)[a-zA-Z]+
+      - pattern-not-regex: '[*](\s+([a-zA-Z]+\s+)+)[a-zA-Z]+'
+    paths:
+      exclude:
+        - "*Test*"
+        - "*test*"

apex/lang/best-practice/ncino/tests/UseAssertClass.cls

@@ -0,0 +1,13 @@
+public class UseAssertClass {
+	// ruleid: use-assert-class
+	System.assert(true);
+
+	// ruleid: use-assert-class
+	System.assertEquals(1,1);
+
+	// ruleid: use-assert-class
+	System.assertNotEquals(1,1);
+
+	// ok: use-assert-class
+	Assert.areEqual(1,1);
+}

apex/lang/best-practice/ncino/tests/UseAssertClass.yaml

@@ -0,0 +1,18 @@
+rules:
+  - id: use-assert-class
+    min-version: 1.44.0
+    severity: WARNING
+    languages:
+      - generic
+    metadata:
+      category: best-practice
+      technology:
+      - salesforce
+    message: >-
+      Assert methods in the System class have been replaced with the Assert class:
+      https://developer.salesforce.com/docs/atlas.en-us.apexref.meta/apexref/apex_class_System_Assert.htm
+    pattern-regex: System\.assert
+    paths:
+      include:
+        - "*.cls"
+        - "UseAssertClass.cls"

apex/lang/best-practice/ncino/urls/AbsoluteUrls.cls

@@ -0,0 +1,51 @@
+/*
+ * Test Controller with Absolute URLs
+ */
+public with sharing class AbsoluteUrls {
+
+	public PageReference absoluteSalesforceUrlExample() {
+		// ruleid: absolute-urls
+		String strUrl = 'https://na8.salesforce.com/TestVFPage?AccountId=999';
+		PageReference newUrl = new PageReference(strUrl);
+		newURL.setRedirect(true);
+
+		return newURL;
+	}
+
+	public PageReference absoluteNonSalesforceUrlExample() {
+		// ok: absolute-urls
+		String strUrl = 'https://www.website.com';
+		PageReference newUrl = new PageReference(strUrl);
+		newURL.setRedirect(true);
+
+		return newURL;
+	}
+
+	public PageReference nonAbsoluteSalesforceUrlExample() {
+		// ok: absolute-urls
+		String strUrl = URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999';
+		PageReference newUrl = new PageReference(strUrl);
+		newURL.setRedirect(true);
+
+		return newURL;
+	}
+
+	// Absolute Salesforce URL comment example
+	// ruleid: absolute-urls
+	// https://na8.salesforce.com/TestVFPage?AccountId=999
+
+	// Absolute non-Salesforce URL comment example
+	// ok: absolute-urls
+	// https://www.website.com
+
+	// Non-absolute Salesforce URL comment example
+	// ok: absolute-urls
+	// URL.getSalesforceBaseUrl().toExternalForm() + '/TestVFPage?AccountId=999
+
+	// ruleid: absolute-urls
+	private static final String ABSOLUTE_SF_URL_CONSTANT = 'https://na8.salesforce.com/TestVFPage?AccountId=999';
+	// ok: absolute-urls
+	private static final String ABSOLUTE_NON_SF_URL_CONSTANT = 'https://www.website.com';
+	// ok: absolute-urls
+	private static final String NON_ASBOLUTE_SF_URL_CONSTANT = URL.getOrgDomainUrl().toExternalForm() + '/TestVFPage?AccountId=999';
+}

apex/lang/best-practice/ncino/urls/AbsoluteUrls.yaml

@@ -0,0 +1,21 @@
+rules:
+  - id: absolute-urls
+    min-version: 1.44.0
+    severity: WARNING
+    languages:
+      - apex
+    metadata:
+      category: best-practice
+      technology:
+      - salesforce
+    message: >-
+      Using absolute URLs to Salesforce Pages is bug prone. Different sandboxes and production
+      environments will have different instance names (like "na10", "na15" etc.). Code using
+      absolute URLs will only work when it runs in the corresponding salesforce instances. It
+      will break as soon as it is deployed in another one. Thus only relative URLs, i.e. without
+      the domain and subdomain names, should be used when pointing to a salesforce page.
+    pattern-regex: (http|https)://.*(salesforce|force|visualforce)\.com\.*
+    paths:
+      exclude:
+        - "*Test*"
+        - "*test*"

apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.cls

@@ -0,0 +1,146 @@
+public class AvoidNativeDmlInLoops {
+	public void insertInsideLoop() {
+		for (Integer i = 0; i < 151; i++) {
+			// ruleid: avoid-native-dml-in-loops
+			insert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.insert(a);
+		}
+	}
+
+	public void insertInsideLoop2() {
+		for (Account a : accounts) {
+			// ruleid: avoid-native-dml-in-loops
+			insert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.insert(a);
+		}
+	}
+
+	public void insertInsideLoop3() {
+		while (someCondition) {
+			// ruleid: avoid-native-dml-in-loops
+			insert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.insert(a);
+		}
+	}
+
+	public void insertInsideLoop4() {
+		do {
+			// ruleid: avoid-native-dml-in-loops
+			insert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.insert(a);
+		} while (someCondition);
+	}
+
+	public void updateInsideLoop() {
+		for (Integer i = 0; i < 151; i++) {
+			// ruleid: avoid-native-dml-in-loops
+			update account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.update(a);
+		}
+	}
+
+	public void updateInsideLoop2() {
+		for (Account a : accounts) {
+			// ruleid: avoid-native-dml-in-loops
+			update account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.update(a);
+		}
+	}
+
+	public void updateInsideLoop3() {
+		while (someCondition) {
+			// ruleid: avoid-native-dml-in-loops
+			update account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.update(a);
+		}
+	}
+
+	public void updateInsideLoop4() {
+		do {
+			// ruleid: avoid-native-dml-in-loops
+			update account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.update(a);
+		} while (someCondition);
+	}
+
+	public void upsertInsideLoop() {
+		for (Integer i = 0; i < 151; i++) {
+			// ruleid: avoid-native-dml-in-loops
+			upsert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.upsert(a);
+		}
+	}
+
+	public void upsertInsideLoop2() {
+		for (Account a : accounts) {
+			// ruleid: avoid-native-dml-in-loops
+			upsert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.upsert(a);
+		}
+	}
+
+	public void upsertInsideLoop3() {
+		while (someCondition) {
+			// ruleid: avoid-native-dml-in-loops
+			upsert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.upsert(a);
+		}
+	}
+
+	public void upsertInsideLoop4() {
+		do {
+			// ruleid: avoid-native-dml-in-loops
+			upsert account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.upsert(a);
+		} while (someCondition);
+	}
+
+	public void deleteInsideLoop() {
+		for (Integer i = 0; i < 151; i++) {
+			// ruleid: avoid-native-dml-in-loops
+			delete account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.delete(a);
+		}
+	}
+
+	public void deleteInsideLoop2() {
+		for (Account a : accounts) {
+			// ruleid: avoid-native-dml-in-loops
+			delete account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.delete(a);
+		}
+	}
+
+	public void deleteInsideLoop3() {
+		while (someCondition) {
+			// ruleid: avoid-native-dml-in-loops
+			delete account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.delete(a);
+		}
+	}
+
+	public void deleteInsideLoop4() {
+		do {
+			// ruleid: avoid-native-dml-in-loops
+			delete account;
+			// ruleid: avoid-native-dml-in-loops
+			Database.delete(a);
+		} while (someCondition);
+	}
+
+}

apex/lang/performance/ncino/operationsInLoops/AvoidNativeDmlInLoops.yaml

@@ -0,0 +1,45 @@
+rules:
+  - id: avoid-native-dml-in-loops
+    min-version: 1.44.0
+    severity: ERROR
+    languages:
+      - generic
+    metadata:
+      category: performance
+      technology:
+      - salesforce
+    message: >-
+      Avoid DML statements inside loops to avoid hitting the DML governor limit.
+      Instead, try to batch up the data into a list and invoke your DML once on
+      that list of data outside the loop.
+    patterns:
+      - pattern-either:
+          - pattern-inside: |
+              for (...) {
+                ...
+              }
+          - pattern-inside: |
+              while (...) {
+                ...
+              }
+          - pattern-inside: |
+              do {
+                ...
+              } while (...);
+      - pattern-either:
+          - pattern: |
+              insert $DATA;
+          - pattern: |
+              update $DATA;
+          - pattern: |
+              upsert $DATA;
+          - pattern: |
+              delete $DATA;
+          - pattern: |
+              Database.insert($DATA);
+          - pattern: |
+              Database.update($DATA);
+          - pattern: |
+              Database.upsert($DATA);
+          - pattern: |
+              Database.delete($DATA);