From 508aef79e73fdf92f3dc67bf0d0685e905888db4 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 30 Jun 2022 15:43:43 -0700 Subject: [PATCH 1/3] SFGE: adding ability to programatically enable/disable rules --- .../java/com/salesforce/cli/CliArgParser.java | 10 +++++++--- .../com/salesforce/rules/AbstractRule.java | 5 +++++ .../salesforce/rules/ApexFlsViolationRule.java | 5 +++++ .../java/com/salesforce/rules/RuleUtil.java | 5 ++++- .../com/salesforce/rules/RuleUtilTest.java | 18 ++++++++++-------- src/lib/sfge/SfgeEngine.ts | 4 ---- 6 files changed, 31 insertions(+), 16 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/cli/CliArgParser.java b/sfge/src/main/java/com/salesforce/cli/CliArgParser.java index e8e7eabd3..9af55b233 100644 --- a/sfge/src/main/java/com/salesforce/cli/CliArgParser.java +++ b/sfge/src/main/java/com/salesforce/cli/CliArgParser.java @@ -126,9 +126,13 @@ private List readFile(String fileName) throws IOException { private void identifyRules(List rulesToRun) { try { - for (String ruleName : rulesToRun) { - AbstractRule rule = RuleUtil.getRule(ruleName); - selectedRules.add(rule); + if (rulesToRun.isEmpty()) { + selectedRules.addAll(RuleUtil.getAllRules()); + } else { + for (String ruleName : rulesToRun) { + AbstractRule rule = RuleUtil.getRule(ruleName); + selectedRules.add(rule); + } } } catch (RuleUtil.RuleNotFoundException ex) { throw new InvocationException(ex.getMessage(), ex); diff --git a/sfge/src/main/java/com/salesforce/rules/AbstractRule.java b/sfge/src/main/java/com/salesforce/rules/AbstractRule.java index 90dd7406d..d7a545914 100644 --- a/sfge/src/main/java/com/salesforce/rules/AbstractRule.java +++ b/sfge/src/main/java/com/salesforce/rules/AbstractRule.java @@ -48,6 +48,11 @@ public Descriptor getDescriptor() { // probably un-abstract this method. protected abstract String getCategory(); + protected boolean isEnabled() { + // By default, every rule is disabled, unless specifically enabled + return false; + } + /** * Unless the rule has a predetermined URL, we'll return a link to information about the engine. */ diff --git a/sfge/src/main/java/com/salesforce/rules/ApexFlsViolationRule.java b/sfge/src/main/java/com/salesforce/rules/ApexFlsViolationRule.java index 87f28b8fb..21ecc1553 100644 --- a/sfge/src/main/java/com/salesforce/rules/ApexFlsViolationRule.java +++ b/sfge/src/main/java/com/salesforce/rules/ApexFlsViolationRule.java @@ -48,6 +48,11 @@ protected String getUrl() { return URL; } + @Override + protected boolean isEnabled() { + return true; + } + @Override protected List _run(GraphTraversalSource g, ApexPath path, BaseSFVertex vertex) { final HashSet flsViolationInfos = new HashSet<>(); diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index 5e088f338..8d5d48745 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -90,7 +90,10 @@ public static List getAllRules() throws RuleNotFoundException { for (Class ruleType : ruleTypes) { // Skip abstract classes. if (!Modifier.isAbstract(ruleType.getModifiers())) { - rules.add(getRuleInner(ruleType.getName())); + final AbstractRule rule = getRuleInner(ruleType.getName()); + if (rule.isEnabled()) { + rules.add(rule); + } } } return rules; diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index e55890da4..056d40144 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -65,14 +65,14 @@ public void getPathEntryPoints_includesAnnotatedMethods(String annotation) { @Test public void getPathEntryPoints_includesGlobalMethods() { String sourceCode = - "public class Foo {\n" - + " global static void globalStaticMethod() {\n" - + " }\n" - + " global void globalInstanceMethod() {\n" - + " }\n" - + " public static void publicStaticMethod() {\n" - + " }\n" - + "}\n"; + "public class Foo {\n" + + " global static void globalStaticMethod() {\n" + + " }\n" + + " global void globalInstanceMethod() {\n" + + " }\n" + + " public static void publicStaticMethod() {\n" + + " }\n" + + "}\n"; TestUtil.buildGraph(g, sourceCode, true); List entryPoints = RuleUtil.getPathEntryPoints(g); @@ -296,6 +296,8 @@ public void getPathEntryPoints_includeMethodAndFileLevelTargets() { public void getAllRules_noExceptionThrown() { try { List allRules = RuleUtil.getAllRules(); + MatcherAssert.assertThat(allRules, hasSize(1)); + assertTrue(allRules.contains(ApexFlsViolationRule.getInstance())); } catch (Exception ex) { fail("Unexpected " + ex.getClass().getSimpleName() + ": " + ex.getMessage()); } diff --git a/src/lib/sfge/SfgeEngine.ts b/src/lib/sfge/SfgeEngine.ts index 09de49885..ba7e1b7a7 100644 --- a/src/lib/sfge/SfgeEngine.ts +++ b/src/lib/sfge/SfgeEngine.ts @@ -83,10 +83,6 @@ export class SfgeEngine extends AbstractRuleEngine { const categoryNames: Set = new Set(); partialRules.forEach(({name, description, category}) => { - // TODO: This should be accomplished by actually disabling the rules within SFGE, instead of this hacky fix. - if (name !== 'ApexFlsViolationRule') { - return; - } completeRules.push({ engine: ENGINE.SFGE, sourcepackage: "sfge", From aa1647c8363521205fd57c03dd0e6a54826fe6ca Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 6 Jul 2022 10:42:41 -0700 Subject: [PATCH 2/3] Applying PR feedback --- sfge/src/main/java/com/salesforce/Main.java | 2 +- .../main/java/com/salesforce/cli/CliArgParser.java | 2 +- sfge/src/main/java/com/salesforce/rules/RuleUtil.java | 11 +++++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/Main.java b/sfge/src/main/java/com/salesforce/Main.java index a4ea7e1f7..aabe40ea9 100644 --- a/sfge/src/main/java/com/salesforce/Main.java +++ b/sfge/src/main/java/com/salesforce/Main.java @@ -89,7 +89,7 @@ private int catalog() { LOGGER.info("Invoked CATALOG flow"); List rules; try { - rules = RuleUtil.getAllRules(); + rules = RuleUtil.getEnabledRules(); } catch (SfgeException | SfgeRuntimeException ex) { System.err.println(ex.getMessage()); return INTERNAL_ERROR; diff --git a/sfge/src/main/java/com/salesforce/cli/CliArgParser.java b/sfge/src/main/java/com/salesforce/cli/CliArgParser.java index 9af55b233..973bea8b3 100644 --- a/sfge/src/main/java/com/salesforce/cli/CliArgParser.java +++ b/sfge/src/main/java/com/salesforce/cli/CliArgParser.java @@ -127,7 +127,7 @@ private List readFile(String fileName) throws IOException { private void identifyRules(List rulesToRun) { try { if (rulesToRun.isEmpty()) { - selectedRules.addAll(RuleUtil.getAllRules()); + selectedRules.addAll(RuleUtil.getEnabledRules()); } else { for (String ruleName : rulesToRun) { AbstractRule rule = RuleUtil.getRule(ruleName); diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index 8d5d48745..0063b7f96 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -79,7 +79,12 @@ public static List getPathEntryPoints( return new ArrayList<>(methods); } - public static List getAllRules() throws RuleNotFoundException { + public static List getEnabledRules() throws RuleNotFoundException { + final List allRules = getAllRules(); + return allRules.stream().filter(rule -> rule.isEnabled()).collect(Collectors.toList()); + } + + static List getAllRules() throws RuleNotFoundException { // Get a set of every class in the Rules package that extends AbstractRule. Reflections reflections = new Reflections(PackageConstants.RULES_PACKAGE); Set> ruleTypes = @@ -91,9 +96,7 @@ public static List getAllRules() throws RuleNotFoundException { // Skip abstract classes. if (!Modifier.isAbstract(ruleType.getModifiers())) { final AbstractRule rule = getRuleInner(ruleType.getName()); - if (rule.isEnabled()) { - rules.add(rule); - } + rules.add(rule); } } return rules; From 7e822f51707d29cdf4e238f83a37cd33a814ebbd Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 6 Jul 2022 10:47:39 -0700 Subject: [PATCH 3/3] Adding missed file --- sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index 056d40144..4d337bee0 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -295,7 +295,7 @@ public void getPathEntryPoints_includeMethodAndFileLevelTargets() { @Test public void getAllRules_noExceptionThrown() { try { - List allRules = RuleUtil.getAllRules(); + List allRules = RuleUtil.getEnabledRules(); MatcherAssert.assertThat(allRules, hasSize(1)); assertTrue(allRules.contains(ApexFlsViolationRule.getInstance())); } catch (Exception ex) {