From 63a0496d1919df2bec4a61d44c871bf2ee06f1e6 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Tue, 21 Jun 2022 16:04:01 -0500 Subject: [PATCH 1/3] @W-10459675@: Added support for NamespaceAccessible annotation. --- .editorconfig | 4 ++ .../java/com/salesforce/graph/Schema.java | 1 + .../com/salesforce/graph/ops/MethodUtil.java | 47 +++++++++------ .../java/com/salesforce/rules/RuleUtil.java | 2 + .../graph/build/MethodUtilTest.java | 49 --------------- .../salesforce/graph/ops/MethodUtilTest.java | 60 +++++++++++++++++++ 6 files changed, 96 insertions(+), 67 deletions(-) diff --git a/.editorconfig b/.editorconfig index 00543da0b..3afaa32df 100644 --- a/.editorconfig +++ b/.editorconfig @@ -7,6 +7,10 @@ charset = utf-8 trim_trailing_whitespace = true insert_final_newline = true +[*.java] +indent_style = space +indent_size = 4 + [*.md] trim_trailing_whitespace = false diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index 068be7f13..b27726cd4 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -44,6 +44,7 @@ public class Schema { public static final String MODIFIERS = "Modifiers"; public static final String NAME = "Name"; public static final String NAMES = "Names"; + public static final String NAMESPACE_ACCESSIBLE = "NamespaceAccessible"; public static final String OPERATOR = "Operator"; public static final String REFERENCE_TYPE = "ReferenceType"; public static final String RETURN_TYPE = "ReturnType"; diff --git a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java index 5e6720e96..cbb8db917 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -165,24 +165,30 @@ private static void addMessagesForTarget(RuleRunnerTarget target, List getAuraEnabledMethods( GraphTraversalSource g, List targetFiles) { - // Only look at UserClass vertices. Not interested in Enums, Interfaces, or Triggers + return getMethodsWithAnnotation(g, targetFiles, Schema.AURA_ENABLED); + } + + /** + * Returns non-test methods in the target files with a @NamespaceAccessible annotation. An empty + * list implicitly includes all files. + */ + public static List getNamespaceAccessibleMethods( + GraphTraversalSource g, List targetFiles) { + return getMethodsWithAnnotation(g, targetFiles, Schema.NAMESPACE_ACCESSIBLE); + } + + static List getMethodsWithAnnotation( + GraphTraversalSource g, List targetFiles, String annotation) { + // Only look at UserClass vertices. Uninterested in Enums, Interfaces, or Triggers. final String[] labels = new String[] {NodeType.USER_CLASS}; return SFVertexFactory.loadVertices( g, - TraversalUtil.fileRootTraversal(g, labels, targetFiles) - .not(has(Schema.IS_TEST, true)) - .repeat(__.out(Schema.CHILD)) - .until(__.hasLabel(NodeType.METHOD)) - .not(has(Schema.IS_TEST, true)) + rootMethodTraversal(g, targetFiles) .where( out(Schema.CHILD) .hasLabel(NodeType.MODIFIER_NODE) .out(Schema.CHILD) - .where( - H.has( - NodeType.ANNOTATION, - Schema.NAME, - Schema.AURA_ENABLED))) + .where(H.has(NodeType.ANNOTATION, Schema.NAME, annotation))) .order(Scope.global) .by(Schema.DEFINING_TYPE, Order.asc) .by(Schema.NAME, Order.asc)); @@ -194,21 +200,26 @@ public static List getAuraEnabledMethods( */ public static List getPageReferenceMethods( GraphTraversalSource g, List targetFiles) { - // Only look at UserClass vertices. Not interested in Enums, Interfaces, or Triggers - final String[] labels = new String[] {NodeType.USER_CLASS}; return SFVertexFactory.loadVertices( g, - TraversalUtil.fileRootTraversal(g, labels, targetFiles) - .not(has(Schema.IS_TEST, true)) - .repeat(__.out(Schema.CHILD)) - .until(__.hasLabel(NodeType.METHOD)) - .not(has(Schema.IS_TEST, true)) + rootMethodTraversal(g, targetFiles) .where(H.has(NodeType.METHOD, Schema.RETURN_TYPE, PAGE_REFERENCE)) .order(Scope.global) .by(Schema.DEFINING_TYPE, Order.asc) .by(Schema.NAME, Order.asc)); } + private static GraphTraversal rootMethodTraversal( + GraphTraversalSource g, List targetFiles) { + // Only look at UserClass vertices. Not interested in Enums, Interfaces, or Triggers + final String[] labels = new String[] {NodeType.USER_CLASS}; + return TraversalUtil.fileRootTraversal(g, labels, targetFiles) + .not(has(Schema.IS_TEST, true)) + .repeat(__.out(Schema.CHILD)) + .until(__.hasLabel(NodeType.METHOD)) + .not(has(Schema.IS_TEST, true)); + } + /** * Returns all non-test public- and global-scoped methods in controllers referenced by * VisualForce pages, filtered by target file list. An empty list implicitly includes all files. diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index 82a78bdb9..bd0dae90b 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -59,6 +59,8 @@ public static List getPathEntryPoints( if (!fileLevelTargets.isEmpty() || targets.isEmpty()) { // Use the file-level targets to get aura-enabled methods... methods.addAll(MethodUtil.getAuraEnabledMethods(g, fileLevelTargets)); + // ...and NamespaceAccessible methods... + methods.addAll(MethodUtil.getNamespaceAccessibleMethods(g, fileLevelTargets)); // ...and PageReference methods... methods.addAll(MethodUtil.getPageReferenceMethods(g, fileLevelTargets)); // ...and exposed methods on VF controllers. diff --git a/sfge/src/test/java/com/salesforce/graph/build/MethodUtilTest.java b/sfge/src/test/java/com/salesforce/graph/build/MethodUtilTest.java index a5d23d619..c2622c96d 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/MethodUtilTest.java @@ -1189,55 +1189,6 @@ public void testIsTestClass() { MatcherAssert.assertThat(myTestClass.isTest(), equalTo(true)); } - /** - * This only tests the positive UserClass case. The compiler does not accept annotations on the - * other types of vertices that are excluded from the query. Exclude test classes and methods - */ - @Test - public void testGetAuraEnabledMethods() { - String[] sourceCode = { - "public class MyClass {\n" - + " @AuraEnabled\n" - + " public static void foo() {\n" - + " }\n" - + " @AuraEnabled\n" - + " public static testMethod void shouldBeExcludedByModifier() {\n" - + " }\n" - + " @AuraEnabled\n" - + " @isTest\n" - + " public static void shouldBeExcludedByAnnotation() {\n" - + " }\n" - + " public static void bar() {\n" - + " }\n" - + "}\n", - "@isTest\n" - + "public class MyTestClass {\n" - + " @AuraEnabled\n" - + " public static void foo() {\n" - + " }\n" - + "}\n", - }; - - TestUtil.buildGraph(g, sourceCode); - - List methods = MethodUtil.getAuraEnabledMethods(g, new ArrayList<>()); - MatcherAssert.assertThat(methods, hasSize(equalTo(1))); - - MethodVertex method = methods.get(0); - MatcherAssert.assertThat(method.getName(), equalTo("foo")); - MatcherAssert.assertThat(method.isTest(), equalTo(false)); - - for (String excludedName : - new String[] {"shouldBeExcludedByModifier", "shouldBeExcludedByAnnotation"}) { - MethodVertex excludedMethod = - SFVertexFactory.load( - g, - g.V().hasLabel(ASTConstants.NodeType.METHOD) - .has(Schema.NAME, excludedName)); - MatcherAssert.assertThat(excludedName, excludedMethod.isTest(), equalTo(true)); - } - } - /** * Test that UserClass methods that return are PageReference included. UserInterface methods are * excluded. Test methods are exclude. Methods from test classes are excluded. diff --git a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java index 6790847d9..ba6aadcb2 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -8,7 +8,10 @@ import static org.junit.jupiter.api.Assertions.fail; import com.salesforce.TestUtil; +import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.graph.Schema; import com.salesforce.graph.vertex.MethodVertex; +import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.messaging.CliMessager; import com.salesforce.messaging.EventKey; import com.salesforce.rules.AbstractRuleRunner.RuleRunnerTarget; @@ -20,6 +23,8 @@ import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class MethodUtilTest { private GraphTraversalSource g; @@ -329,4 +334,59 @@ public void getTargetMethods_targetMethodInInnerAndOuterClass() { messages, containsString(EventKey.WARNING_MULTIPLE_METHOD_TARGET_MATCHES.getMessageKey())); } + + @ValueSource(strings = {Schema.AURA_ENABLED, Schema.NAMESPACE_ACCESSIBLE}) + @ParameterizedTest(name = "{displayName}: {0}") + public void testGetNamespaceAccessibleMethods(String annotation) { + String[] sourceCode = { + "public class MyClass {\n" + + " @" + + annotation + + "\n" + + " public static void foo() {\n" + + " }\n" + + " @" + + annotation + + "\n" + + " public static testMethod void shouldBeExcludedByModifier() {\n" + + " }\n" + + " @" + + annotation + + "\n" + + " @isTest\n" + + " public static void shouldBeExcludedByAnnotation() {\n" + + " }\n" + + " public static void bar() {\n" + + " }\n" + + "}\n", + "@isTest\n" + + "public class MyTestClass {\n" + + " @" + + annotation + + "\n" + + " public static void foo() {\n" + + " }\n" + + "}\n", + }; + + TestUtil.buildGraph(g, sourceCode); + + List methods = + MethodUtil.getMethodsWithAnnotation(g, new ArrayList<>(), annotation); + MatcherAssert.assertThat(methods, hasSize(equalTo(1))); + + MethodVertex method = methods.get(0); + MatcherAssert.assertThat(method.getName(), equalTo("foo")); + MatcherAssert.assertThat(method.isTest(), equalTo(false)); + + for (String excludedName : + new String[] {"shouldBeExcludedByModifier", "shouldBeExcludedByAnnotation"}) { + MethodVertex excludedMethod = + SFVertexFactory.load( + g, + g.V().hasLabel(ASTConstants.NodeType.METHOD) + .has(Schema.NAME, excludedName)); + MatcherAssert.assertThat(excludedName, excludedMethod.isTest(), equalTo(true)); + } + } } From 2dddfce872de13a08546d06f2f3dc1027c7ea089 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 22 Jun 2022 12:26:17 -0500 Subject: [PATCH 2/3] @W-10459675@: Fixed typo in test name. --- sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java index ba6aadcb2..30f4c41ff 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -337,7 +337,7 @@ public void getTargetMethods_targetMethodInInnerAndOuterClass() { @ValueSource(strings = {Schema.AURA_ENABLED, Schema.NAMESPACE_ACCESSIBLE}) @ParameterizedTest(name = "{displayName}: {0}") - public void testGetNamespaceAccessibleMethods(String annotation) { + public void testGetMethodsWithAnnotation(String annotation) { String[] sourceCode = { "public class MyClass {\n" + " @" From 9164a2e5bb4700dec718069f331c5c51d43b543e Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Fri, 24 Jun 2022 09:53:28 -0500 Subject: [PATCH 3/3] @W-10459675@: Integrating feedback from code review. --- sfge/.editorconfig | 2 +- .../com/salesforce/rules/RuleUtilTest.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/sfge/.editorconfig b/sfge/.editorconfig index 60c0d3274..76d2d297a 100644 --- a/sfge/.editorconfig +++ b/sfge/.editorconfig @@ -9,5 +9,5 @@ trim_trailing_whitespace = true insert_final_newline = true [*.java] -indent_style = tab +indent_style = space indent_size = 4 diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index 0e8618138..a7be11d6a 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -57,6 +57,28 @@ public void getPathEntryPoints_includesAuraEnabledMethods() { assertEquals("auraMethod", firstVertex.getName()); } + @Test + public void getPathEntryPoints_includesNamespaceAccessibleMethods() { + String sourceCode = + "public class Foo {\n" + + " @NamespaceAccessible\n" + + " public boolean nsMethod() {\n" + + " return true;\n" + + " }\n" + + "\n" + + " public boolean nonNsMethod() {\n" + + " return true;\n" + + " }\n" + + "}\n"; + TestUtil.buildGraph(g, sourceCode, true); + + List entryPoints = RuleUtil.getPathEntryPoints(g); + + MatcherAssert.assertThat(entryPoints, hasSize(equalTo(1))); + MethodVertex firstVertex = entryPoints.get(0); + assertEquals("nsMethod", firstVertex.getName()); + } + @Test public void getPathEntryPoints_includesPageReferenceMethods() { String sourceCode =