From 719aceda7bb299816066f5cb46e48c6a6a9734e6 Mon Sep 17 00:00:00 2001 From: Grace Date: Tue, 7 Jun 2022 12:48:44 -0700 Subject: [PATCH 01/34] @W-11179348@: improving runtime of windows-unit-tests --- .circleci/config.yml | 109 ++++++++++++++++-- package.json | 4 + .../salesforce/graph/ops/MethodUtilTest.java | 39 +++++-- 3 files changed, 134 insertions(+), 18 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1481cbf0f..63e4b3ec9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -80,6 +80,9 @@ commands: - gradle/collect_test_results: reports_path: pmd-cataloger/build/reports/ test_results_path: pmd-cataloger/build/test-results/ + - gradle/collect_test_results: + reports_path: sfge/build/reports/ + test_results_path: sfge/build/test-results/ - store_test_results: path: test-results - store_artifacts: # upload nyc test coverage as artifact. @@ -218,12 +221,21 @@ jobs: # Purpose: Runs the unit tests in a Windows environment. windows-unit-tests: + # `parallelism` indicates how many simultaneous executors should be run, allowing us to split + # long-running tasks across multiple executors. + parallelism: 4 # larger values didn't seem to affect performance greatly executor: name: win/default # executor type - size: "medium" + size: "large" shell: bash.exe parameters: node-version: *node_version_param + # Specify a subset of unit tests to be run, instead of the whole suite. + # This allows us to work around the suboptimal performance of the Windows executor by running + # multiple executors in parallel where different unit tests are ran in each. + test-type: + type: string + default: all working_directory: C:\repo steps: - attach_workspace: @@ -259,12 +271,90 @@ jobs: - run: mkdir test-results # Unit tests - - run: - name: test - # Necessary to explicitly use bash, otherwise gradlew's status code won't be received and the job will hang. - shell: bash.exe - command: yarn test --reporter mocha-junit-reporter --reporter-option mochaFile=test-results/mocha/test-results.xml - when: always + - when: + condition: + equal: [ all, << parameters.test-type >> ] + steps: + - run: + name: test + # Necessary to explicitly use bash, otherwise gradlew's status code won't be received and the job will hang. + shell: bash.exe + command: yarn test --reporter mocha-junit-reporter --reporter-option mochaFile=test-results/mocha/test-results.xml + when: always + + - when: + condition: + equal: [ sfge, << parameters.test-type >> ] + steps: + - run: + name: test-sfge + # Necessary to explicitly use bash, otherwise gradlew's status code won't be received and the job will hang. + shell: bash.exe + # Identify all the test files and allocate them between parallelized executors by timing data. + # Then turn the array of tests into something that gradle can accept, and run the tests. + command: | + TESTGLOB=$(circleci tests glob "sfge/src/test/**/*Test.java" | circleci tests split --split-by=timings) + echo $TESTGLOB + TESTARRAY=($TESTGLOB) + TESTARG="" + for element in "${TESTARRAY[@]}" + do + TESTARG="$TESTARG --tests `basename $element .java`" + done + echo $TESTARG + yarn test-sfge $TESTARG + when: always + + - when: + condition: + equal: [ cli-messaging, << parameters.test-type >> ] + steps: + - run: + name: test-cli-messaging + # Necessary to explicitly use bash, otherwise gradlew's status code won't be received and the job will hang. + shell: bash.exe + # This unit test suite is fast, so we have the first parallel executor run the tests, and all others exit early. + command: | + if [[ $CIRCLE_NODE_INDEX -gt 0 ]] + then + exit 0 + fi + yarn test-cli-messaging + when: always + + - when: + condition: + equal: [ pmd-cataloger, << parameters.test-type >> ] + steps: + - run: + name: test-pmd-cataloger + # Necessary to explicitly use bash, otherwise gradlew's status code won't be received and the job will hang. + shell: bash.exe + # This unit test suite is fast, so we have the first parallel executor run the tests, and all others exit early. + command: | + if [[ $CIRCLE_NODE_INDEX -gt 0 ]] + then + exit 0 + fi + yarn test-pmd-cataloger + when: always + + - when: + condition: + equal: [ ts, << parameters.test-type >> ] + steps: + - run: + name: test-ts + # Explicitly using bash, for simplicity of required shell script. + shell: bash.exe + # This unit test suite is relatively fast, so we have the first parallel executor run the tests, and all others exit early. + command: | + if [[ $CIRCLE_NODE_INDEX -gt 0 ]] + then + exit 0 + fi + yarn test-ts --reporter mocha-junit-reporter --reporter-option mochaFile=test-results/mocha/test-results.xml + when: always # Linting - run: @@ -510,6 +600,11 @@ workflows: <<: *testing_filters requires: - setup + matrix: + parameters: + # The values of the parameters will be appended to the jobs they create. + # So we'll get "windows-unit-tests-pmd-cataloger", "windows-unit-tests-ts", etc. + test-type: [pmd-cataloger, cli-messaging, ts, sfge] - linux-tarball-test: filters: <<: *testing_filters diff --git a/package.json b/package.json index 756d0a15f..c29919e27 100644 --- a/package.json +++ b/package.json @@ -133,6 +133,10 @@ "postpack": "rm -f oclif.manifest.json", "lint": "eslint ./src --ext .ts", "test": "./gradlew test jacocoTestCoverageVerification && nyc mocha --timeout 10000 --retries 5 \"./test/**/*.test.ts\"", + "test-cli-messaging": "./gradlew cli-messaging:test", + "test-pmd-cataloger": "./gradlew pmd-cataloger:test", + "test-sfge": "./gradlew sfge:test", + "test-ts": "nyc mocha --timeout 10000 --retries 5 \"./test/**/*.test.ts\"", "coverage": "nyc report --reporter text", "version": "oclif-dev readme && git add README.md" } 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 a139421a8..6790847d9 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -175,12 +175,21 @@ public void getTargetMethods_targetMultipleMethods() { List methodVertices = MethodUtil.getTargetedMethods(g, targets); MatcherAssert.assertThat(methodVertices, hasSize(equalTo(2))); - MethodVertex firstVertex = methodVertices.get(0); - assertEquals(METHOD_WITHOUT_OVERLOADS_1, firstVertex.getName()); - - MethodVertex secondVertex = methodVertices.get(1); - assertEquals(METHOD_WITHOUT_OVERLOADS_2, secondVertex.getName()); + boolean method1Found = false; + boolean method2Found = false; + for (MethodVertex methodVertex : methodVertices) { + String name = methodVertex.getName(); + if (METHOD_WITHOUT_OVERLOADS_1.equals(name)) { + method1Found = true; + } else if (METHOD_WITHOUT_OVERLOADS_2.equals(name)) { + method2Found = true; + } else { + fail("Unexpected method name " + name); + } + } + assertTrue(method1Found); + assertTrue(method2Found); String messages = CliMessager.getInstance().getAllMessages(); assertEquals("[]", messages); } @@ -227,13 +236,21 @@ public void getTargetMethods_targetNameDupedMethods() { List methodVertices = MethodUtil.getTargetedMethods(g, targets); MatcherAssert.assertThat(methodVertices, hasSize(equalTo(2))); - MethodVertex firstVertex = methodVertices.get(0); - assertEquals(METHOD_WITH_EXTERNAL_NAME_DUPLICATION, firstVertex.getName()); - assertEquals(18, firstVertex.getBeginLine()); - MethodVertex secondVertex = methodVertices.get(1); - assertEquals(METHOD_WITH_EXTERNAL_NAME_DUPLICATION, secondVertex.getName()); - assertEquals(22, secondVertex.getBeginLine()); + boolean line18Found = false; + boolean line22Found = false; + for (MethodVertex methodVertex : methodVertices) { + assertEquals(METHOD_WITH_EXTERNAL_NAME_DUPLICATION, methodVertex.getName()); + if (methodVertex.getBeginLine() == 18) { + line18Found = true; + } else if (methodVertex.getBeginLine() == 22) { + line22Found = true; + } else { + fail("Unexpected line number " + methodVertex.getBeginLine()); + } + } + assertTrue(line18Found); + assertTrue(line22Found); String messages = CliMessager.getInstance().getAllMessages(); MatcherAssert.assertThat( From 63a0496d1919df2bec4a61d44c871bf2ee06f1e6 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Tue, 21 Jun 2022 16:04:01 -0500 Subject: [PATCH 02/34] @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 03/34] @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 04/34] @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 = From c49760e999171e0340cb95f687901444e93268fd Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Fri, 24 Jun 2022 12:35:36 -0500 Subject: [PATCH 05/34] @W-10459675@: Part 2 of several. Added RemoteAction-annotated methods as sources. --- .../java/com/salesforce/graph/Schema.java | 1 + .../com/salesforce/graph/ops/MethodUtil.java | 9 +++ .../java/com/salesforce/rules/RuleUtil.java | 2 + .../salesforce/graph/ops/MethodUtilTest.java | 58 +++++++++---------- .../com/salesforce/rules/RuleUtilTest.java | 40 ++++--------- 5 files changed, 53 insertions(+), 57 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index b27726cd4..b38a1d413 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -47,6 +47,7 @@ public class Schema { public static final String NAMESPACE_ACCESSIBLE = "NamespaceAccessible"; public static final String OPERATOR = "Operator"; public static final String REFERENCE_TYPE = "ReferenceType"; + public static final String REMOTE_ACTION = "RemoteAction"; public static final String RETURN_TYPE = "ReturnType"; public static final String RULE_NAMES = "RulesNames"; public static final String STATIC = "Static"; 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 cbb8db917..4d758cab5 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -177,6 +177,15 @@ public static List getNamespaceAccessibleMethods( return getMethodsWithAnnotation(g, targetFiles, Schema.NAMESPACE_ACCESSIBLE); } + /** + * Returns non-test methods in the target files with a @NamespaceAccessible annotation. An empty + * list implicitly includes all files. + */ + public static List getRemoteActionMethods( + GraphTraversalSource g, List targetFiles) { + return getMethodsWithAnnotation(g, targetFiles, Schema.REMOTE_ACTION); + } + static List getMethodsWithAnnotation( GraphTraversalSource g, List targetFiles, String annotation) { // Only look at UserClass vertices. Uninterested in Enums, Interfaces, or Triggers. diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index bd0dae90b..b0b7c6ec0 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -61,6 +61,8 @@ public static List getPathEntryPoints( methods.addAll(MethodUtil.getAuraEnabledMethods(g, fileLevelTargets)); // ...and NamespaceAccessible methods... methods.addAll(MethodUtil.getNamespaceAccessibleMethods(g, fileLevelTargets)); + // ...and RemoteAction methods... + methods.addAll(MethodUtil.getRemoteActionMethods(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/ops/MethodUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java index 30f4c41ff..051ed13a0 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -181,20 +181,20 @@ public void getTargetMethods_targetMultipleMethods() { MatcherAssert.assertThat(methodVertices, hasSize(equalTo(2))); - boolean method1Found = false; - boolean method2Found = false; - for (MethodVertex methodVertex : methodVertices) { - String name = methodVertex.getName(); - if (METHOD_WITHOUT_OVERLOADS_1.equals(name)) { - method1Found = true; - } else if (METHOD_WITHOUT_OVERLOADS_2.equals(name)) { - method2Found = true; - } else { - fail("Unexpected method name " + name); - } - } - assertTrue(method1Found); - assertTrue(method2Found); + boolean method1Found = false; + boolean method2Found = false; + for (MethodVertex methodVertex : methodVertices) { + String name = methodVertex.getName(); + if (METHOD_WITHOUT_OVERLOADS_1.equals(name)) { + method1Found = true; + } else if (METHOD_WITHOUT_OVERLOADS_2.equals(name)) { + method2Found = true; + } else { + fail("Unexpected method name " + name); + } + } + assertTrue(method1Found); + assertTrue(method2Found); String messages = CliMessager.getInstance().getAllMessages(); assertEquals("[]", messages); } @@ -242,20 +242,20 @@ public void getTargetMethods_targetNameDupedMethods() { MatcherAssert.assertThat(methodVertices, hasSize(equalTo(2))); - boolean line18Found = false; - boolean line22Found = false; - for (MethodVertex methodVertex : methodVertices) { - assertEquals(METHOD_WITH_EXTERNAL_NAME_DUPLICATION, methodVertex.getName()); - if (methodVertex.getBeginLine() == 18) { - line18Found = true; - } else if (methodVertex.getBeginLine() == 22) { - line22Found = true; - } else { - fail("Unexpected line number " + methodVertex.getBeginLine()); - } - } - assertTrue(line18Found); - assertTrue(line22Found); + boolean line18Found = false; + boolean line22Found = false; + for (MethodVertex methodVertex : methodVertices) { + assertEquals(METHOD_WITH_EXTERNAL_NAME_DUPLICATION, methodVertex.getName()); + if (methodVertex.getBeginLine() == 18) { + line18Found = true; + } else if (methodVertex.getBeginLine() == 22) { + line22Found = true; + } else { + fail("Unexpected line number " + methodVertex.getBeginLine()); + } + } + assertTrue(line18Found); + assertTrue(line22Found); String messages = CliMessager.getInstance().getAllMessages(); MatcherAssert.assertThat( @@ -335,7 +335,7 @@ public void getTargetMethods_targetMethodInInnerAndOuterClass() { containsString(EventKey.WARNING_MULTIPLE_METHOD_TARGET_MATCHES.getMessageKey())); } - @ValueSource(strings = {Schema.AURA_ENABLED, Schema.NAMESPACE_ACCESSIBLE}) + @ValueSource(strings = {Schema.AURA_ENABLED, Schema.REMOTE_ACTION, Schema.NAMESPACE_ACCESSIBLE}) @ParameterizedTest(name = "{displayName}: {0}") public void testGetMethodsWithAnnotation(String annotation) { String[] sourceCode = { diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index a7be11d6a..0479775a4 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.fail; import com.salesforce.TestUtil; +import com.salesforce.graph.Schema; import com.salesforce.graph.vertex.MethodVertex; import com.salesforce.metainfo.MetaInfoCollectorTestProvider; import com.salesforce.metainfo.VisualForceHandlerImpl; @@ -25,6 +26,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 RuleUtilTest { private static final Logger LOGGER = LogManager.getLogger(RuleUtilTest.class); @@ -35,16 +38,19 @@ public void setup() { this.g = TestUtil.getGraph(); } - @Test - public void getPathEntryPoints_includesAuraEnabledMethods() { + @ValueSource(strings = {Schema.AURA_ENABLED, Schema.REMOTE_ACTION, Schema.NAMESPACE_ACCESSIBLE}) + @ParameterizedTest(name = "{displayName}: {0}") + public void getPathEntryPoints_includesAnnotatedMethods(String annotation) { String sourceCode = "public class Foo {\n" - + " @AuraEnabled\n" - + " public boolean auraMethod() {\n" + + " @" + + annotation + + "\n" + + " public boolean annotatedMethod() {\n" + " return true;\n" + " }\n" + "\n" - + " public boolean nonAuraMethod() {\n" + + " public boolean nonAnnotatedMethod() {\n" + " return true;\n" + " }\n" + "}\n"; @@ -54,29 +60,7 @@ public void getPathEntryPoints_includesAuraEnabledMethods() { MatcherAssert.assertThat(entryPoints, hasSize(equalTo(1))); MethodVertex firstVertex = entryPoints.get(0); - 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()); + assertEquals("annotatedMethod", firstVertex.getName()); } @Test From 9a5cb3b8c55d582aae7b091a3e7984de34b48936 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 27 Jun 2022 09:52:36 -0500 Subject: [PATCH 06/34] @W-10459675@: Updated typo in documentation. --- sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4d758cab5..37a144e2f 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -178,7 +178,7 @@ public static List getNamespaceAccessibleMethods( } /** - * Returns non-test methods in the target files with a @NamespaceAccessible annotation. An empty + * Returns non-test methods in the target files with a @RemoteAction annotation. An empty * list implicitly includes all files. */ public static List getRemoteActionMethods( From 68bb2c7105a06932f4d80169829768891e49c460 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 27 Jun 2022 14:27:45 -0500 Subject: [PATCH 07/34] @W-10459675@: Part 3 of several. Added global methods as sources. --- .../com/salesforce/graph/ops/MethodUtil.java | 19 +++++- .../java/com/salesforce/rules/RuleUtil.java | 2 + .../salesforce/graph/ops/MethodUtilTest.java | 62 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) 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 37a144e2f..145973e0f 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -178,8 +178,8 @@ public static List getNamespaceAccessibleMethods( } /** - * Returns non-test methods in the target files with a @RemoteAction annotation. An empty - * list implicitly includes all files. + * Returns non-test methods in the target files with a @RemoteAction annotation. An empty list + * implicitly includes all files. */ public static List getRemoteActionMethods( GraphTraversalSource g, List targetFiles) { @@ -229,6 +229,21 @@ private static GraphTraversal rootMethodTraversal( .not(has(Schema.IS_TEST, true)); } + /** + * Returns non-test methods in the target files whose modifier scope is `global`. An empty list implicitly includes + * all files. + */ + public static List getGlobalMethods( + GraphTraversalSource g, List targetFiles) { + // Get all methods in the target files. + List methodVertices = + SFVertexFactory.loadVertices(g, rootMethodTraversal(g, targetFiles)); + // Filter for the global methods. + return methodVertices.stream() + .filter(m -> m.getModifierNode().isGlobal() && !m.isStandardType()) + .collect(Collectors.toList()); + } + /** * 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 b0b7c6ec0..5e088f338 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -65,6 +65,8 @@ public static List getPathEntryPoints( methods.addAll(MethodUtil.getRemoteActionMethods(g, fileLevelTargets)); // ...and PageReference methods... methods.addAll(MethodUtil.getPageReferenceMethods(g, fileLevelTargets)); + // ...and global-exposed methods... + methods.addAll(MethodUtil.getGlobalMethods(g, fileLevelTargets)); // ...and exposed methods on VF controllers. methods.addAll(MethodUtil.getExposedControllerMethods(g, fileLevelTargets)); } 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 051ed13a0..dfc6a9837 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -389,4 +389,66 @@ public void testGetMethodsWithAnnotation(String annotation) { MatcherAssert.assertThat(excludedName, excludedMethod.isTest(), equalTo(true)); } } + + @Test + public void testGetGlobalMethods() { + String[] sourceCode = { + "public class MyClass {\n" + + " global static void foo() {\n" + + " }\n" + + " global static testMethod void shouldBeExcludedByModifier() {\n" + + " }\n" + + " @isTest\n" + + " global static void shouldBeExcludedByAnnotation() {\n" + + " }\n" + + " public static void bar() {\n" + + " }\n" + + "}\n", + "@isTest\n" + + "public class MyTestClass {\n" + + " public static void foo() {\n" + + " }\n" + + "}\n", + }; + + TestUtil.buildGraph(g, sourceCode); + + List methods = MethodUtil.getGlobalMethods(g, new ArrayList<>()); + // The `foo` method should be included because it's declared as global. The `` method + // (i.e. the constructor) and the `clone` method are included because they implicitly exist even though they were + // never actually declared. The inclusion of unreal methods isn't a problem, because they're just empty paths. + MatcherAssert.assertThat(methods, hasSize(equalTo(3))); + + boolean cloneFound = false; + boolean fooFound = false; + boolean initFound = false; + for (MethodVertex methodVertex : methods) { + switch (methodVertex.getName()) { + case "clone": + cloneFound = true; + break; + case "": + initFound = true; + break; + case "foo": + fooFound = true; + break; + default: + fail("Unexpected method name: " + methodVertex.getName()); + } + } + assertTrue(cloneFound); + assertTrue(initFound); + assertTrue(fooFound); + + 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 e4f834c3f3ebff2f87b0cc7635025d7dcb49e7e5 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 27 Jun 2022 17:22:00 -0500 Subject: [PATCH 08/34] @W-10459675@: Fixed issue in which implicitly declared global methods were being returned. --- .../com/salesforce/graph/ops/MethodUtil.java | 29 ++++++++++++++----- .../salesforce/graph/ops/MethodUtilTest.java | 29 ++----------------- 2 files changed, 24 insertions(+), 34 deletions(-) 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 145973e0f..9632bb1d6 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -230,18 +230,31 @@ private static GraphTraversal rootMethodTraversal( } /** - * Returns non-test methods in the target files whose modifier scope is `global`. An empty list implicitly includes - * all files. + * Returns non-test methods in the target files whose modifier scope is `global`. An empty list + * implicitly includes all files. */ public static List getGlobalMethods( GraphTraversalSource g, List targetFiles) { // Get all methods in the target files. - List methodVertices = - SFVertexFactory.loadVertices(g, rootMethodTraversal(g, targetFiles)); - // Filter for the global methods. - return methodVertices.stream() - .filter(m -> m.getModifierNode().isGlobal() && !m.isStandardType()) - .collect(Collectors.toList()); + return SFVertexFactory.loadVertices( + g, + rootMethodTraversal(g, targetFiles) + .filter( + __.and( + // If a method has at least one block statement, then it is + // definitely actually declared, as + // opposed to being an implicit method. + out(Schema.CHILD) + .hasLabel(NodeType.BLOCK_STATEMENT) + .count() + .is(P.gte(1)), + // We only want global methods. + out(Schema.CHILD) + .hasLabel(NodeType.MODIFIER_NODE) + .has(Schema.GLOBAL, true), + // Ignore any standard methods, otherwise will get a ton of + // extra results. + __.not(__.has(Schema.IS_STANDARD, true))))); } /** 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 dfc6a9837..52336ec4f 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -414,32 +414,9 @@ public void testGetGlobalMethods() { TestUtil.buildGraph(g, sourceCode); List methods = MethodUtil.getGlobalMethods(g, new ArrayList<>()); - // The `foo` method should be included because it's declared as global. The `` method - // (i.e. the constructor) and the `clone` method are included because they implicitly exist even though they were - // never actually declared. The inclusion of unreal methods isn't a problem, because they're just empty paths. - MatcherAssert.assertThat(methods, hasSize(equalTo(3))); - - boolean cloneFound = false; - boolean fooFound = false; - boolean initFound = false; - for (MethodVertex methodVertex : methods) { - switch (methodVertex.getName()) { - case "clone": - cloneFound = true; - break; - case "": - initFound = true; - break; - case "foo": - fooFound = true; - break; - default: - fail("Unexpected method name: " + methodVertex.getName()); - } - } - assertTrue(cloneFound); - assertTrue(initFound); - assertTrue(fooFound); + // The `foo` method should be included because it's declared as global. + MatcherAssert.assertThat(methods, hasSize(equalTo(1))); + MatcherAssert.assertThat(methods.get(0).getName(), equalTo("foo")); for (String excludedName : new String[] {"shouldBeExcludedByModifier", "shouldBeExcludedByAnnotation"}) { From 10fc89e96da0bbc754a17ae17d85ef8ec60aaac5 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Tue, 28 Jun 2022 13:36:13 -0500 Subject: [PATCH 09/34] @W-10459675@: Added additional tests as per code review. --- .../com/salesforce/rules/RuleUtilTest.java | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index 0479775a4..e55890da4 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -5,8 +5,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.IsNot.not; import static org.hamcrest.core.IsNull.nullValue; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; import com.salesforce.TestUtil; import com.salesforce.graph.Schema; @@ -63,6 +62,40 @@ public void getPathEntryPoints_includesAnnotatedMethods(String annotation) { assertEquals("annotatedMethod", firstVertex.getName()); } + @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"; + TestUtil.buildGraph(g, sourceCode, true); + + List entryPoints = RuleUtil.getPathEntryPoints(g); + + MatcherAssert.assertThat(entryPoints, hasSize(equalTo(2))); + boolean staticMethodFound = false; + boolean instanceMethodFound = false; + for (MethodVertex entrypoint : entryPoints) { + switch (entrypoint.getName()) { + case "globalStaticMethod": + staticMethodFound = true; + break; + case "globalInstanceMethod": + instanceMethodFound = true; + break; + default: + fail("Unexpected method " + entrypoint.getName()); + } + } + assertTrue(staticMethodFound); + assertTrue(instanceMethodFound); + } + @Test public void getPathEntryPoints_includesPageReferenceMethods() { String sourceCode = From d64825d44cd9d9168d470e7b0d1da1f878707e1e Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Tue, 14 Jun 2022 09:52:34 -0700 Subject: [PATCH 10/34] Add more information while handling error --- sfge/src/main/java/com/salesforce/Main.java | 5 +++++ .../main/java/com/salesforce/graph/build/GremlinUtil.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sfge/src/main/java/com/salesforce/Main.java b/sfge/src/main/java/com/salesforce/Main.java index d7f4e0c31..9493ac13f 100644 --- a/sfge/src/main/java/com/salesforce/Main.java +++ b/sfge/src/main/java/com/salesforce/Main.java @@ -4,6 +4,7 @@ import com.salesforce.cli.OutputFormatter; import com.salesforce.exception.SfgeException; import com.salesforce.exception.SfgeRuntimeException; +import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.ops.GraphUtil; import com.salesforce.messaging.CliMessager; import com.salesforce.metainfo.MetaInfoCollector; @@ -129,6 +130,10 @@ private int execute(String... args) { LOGGER.error("Error while loading graph", ex); System.err.println(formatError(ex)); return INTERNAL_ERROR; + } catch (UnexpectedException ex) { + LOGGER.error("Unexpected exception while loading graph", ex); + System.err.println("Unexpected exception while loading graph. See logs for more information."); + return INTERNAL_ERROR; } // Run all of the rules. diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java index 72c3ac09b..a7a5099a6 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java @@ -32,7 +32,7 @@ public static Optional getOnlyChild( if (children.isEmpty()) { return Optional.empty(); } else if (children.size() > 1) { - throw new UnexpectedException(children); + throw new UnexpectedException("Did not expect more than one child node. Actual count: " + children.size()); } else { return Optional.of(children.get(0)); } From 1dfce6a11e085930beaa19ba93c1925f47d874aa Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 22 Jun 2022 15:29:29 -0700 Subject: [PATCH 11/34] work in progress - static block recognition part --- .../java/com/salesforce/graph/Schema.java | 3 + .../build/AbstractApexVertexBuilder.java | 134 +++++++++++++++--- .../ApexStandardLibraryVertexBuilder.java | 5 +- .../salesforce/graph/build/GremlinUtil.java | 15 +- .../graph/build/MethodPathBuilderVisitor.java | 53 +++++-- .../com/salesforce/graph/ops/MethodUtil.java | 1 + .../graph/symbols/ClassStaticScope.java | 42 +++++- .../salesforce/graph/vertex/MethodVertex.java | 44 +++++- 8 files changed, 260 insertions(+), 37 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index b38a1d413..a6a07f757 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -69,4 +69,7 @@ public static final class JorjeNodeType { public static final String CHILD = "Child"; public static final String PARENT = "Parent"; public static final String NEXT_SIBLING = "NextSibling"; + + /** Indicates if a method is a synthetic static block method */ + public static final String IS_STATIC_BLOCK_METHOD = "IsStaticBlockMethod"; } diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index e9c02ffe1..7a3a93d64 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -6,10 +6,13 @@ import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; +import com.salesforce.graph.ops.MethodUtil; + import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.TreeSet; import org.apache.commons.lang3.StringUtils; @@ -71,23 +74,20 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { vNode.property(Schema.FILE_NAME, fileName); } + final boolean needsStaticBlockSplHandling = isClintMethod(node) && containsStaticBlock(node); Vertex vPreviousSibling = null; - for (JorjeNode child : node.getChildren()) { - Vertex vChild = g.addV(child.getLabel()).next(); - addProperties(g, child, vChild); - // We are currently adding PARENT and CHILD, in theory the same edge could be navigated - // both ways, however - // the code looks messy when that is done. - // TODO: Determine if this causes performance or resource issues. Consider a single edge - g.addE(Schema.PARENT).from(vChild).to(vNode).iterate(); - g.addE(Schema.CHILD).from(vNode).to(vChild).iterate(); - if (vPreviousSibling != null) { - g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(vChild).iterate(); - } - vPreviousSibling = vChild; - // To save memory in the graph, don't pass the source name into recursive calls. - buildVertices(child, vChild, null); - } + final List children = node.getChildren(); + + for (int i = 0; i < children.size(); i++) { + final JorjeNode child = children.get(i); + if (needsStaticBlockSplHandling + && ASTConstants.NodeType.BLOCK_STATEMENT.equals(child.getLabel())) { + // todo: add static blocks as new methods + vPreviousSibling = addStaticBlockAsNewMethod(vNode, vPreviousSibling, child, node, i); + } else { + vPreviousSibling = addChildNodeToGraph(vNode, vPreviousSibling, child); + } + } afterInsert(g, node, vNode); if (rootVNode != null) { // Only call this for the root node @@ -95,13 +95,67 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { } } - /** + private Vertex addChildNodeToGraph(Vertex vNode, Vertex vPreviousSibling, JorjeNode child) { + Vertex vChild = g.addV(child.getLabel()).next(); + addProperties(g, child, vChild); + return addParentChildRelationship(vNode, vChild, child, vPreviousSibling); + } + + private Vertex addParentChildRelationship(Vertex parentVertex, Vertex childVertex, JorjeNode child, Vertex vPreviousSibling) { + addParentChildRelationship(parentVertex, childVertex); + if (vPreviousSibling != null) { + g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(childVertex).iterate(); + } + vPreviousSibling = childVertex; + // To save memory in the graph, don't pass the source name into recursive calls. + buildVertices(child, childVertex, null); + return vPreviousSibling; + } + + private void addParentChildRelationship(Vertex parentVertex, Vertex childVertex) { + // We are currently adding PARENT and CHILD, in theory the same edge could be navigated + // both ways, however + // the code looks messy when that is done. + // TODO: Determine if this causes performance or resource issues. Consider a single edge + g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate(); + g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate(); + } + + private Vertex addStaticBlockAsNewMethod(Vertex clintVertex, Vertex vPreviousSibling, JorjeNode child, JorjeNode clintNode, int staticBlockIndex) { + // Add properties of () to syntheticMethodVertex, + // override properties with a new name and an indicator to + // show this is a synthetic static block method + final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); + + final HashMap overrides = new HashMap<>(); + overrides.put(Schema.NAME, String.format(MethodUtil.SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); + addProperties(g, clintNode, syntheticMethodVertex, overrides); + + // TODO: Add vertex's parent as Synthetic method vertex's parent + // how do i get 's parent vertex? + + final Optional grandParentVertex = GremlinUtil.getParent(g, clintVertex); + if (grandParentVertex.isEmpty()) { + throw new UnexpectedException("Did not expect () to not have a parent vertex. clintVertex=" + clintVertex); + } + + addParentChildRelationship(grandParentVertex.get(), syntheticMethodVertex); + + // Create a vertex for BlockStatement + final Vertex vChild = g.addV(child.getLabel()).next(); + addProperties(g, child, vChild); + + // add BlockStatement as child of SyntheticMethodVertex + return addParentChildRelationship(syntheticMethodVertex, vChild, child, vPreviousSibling); + } + + /** * Adds edges to Method vertices after they are inserted. TODO: Replace this with a listener or * visitor pattern for more general purpose solutions */ private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vNode) { if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) { - MethodPathBuilderVisitor.apply(g, vNode); + MethodPathBuilderVisitor.apply(g, vNode); } } @@ -114,7 +168,11 @@ protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) { // Intentionally left blank } - protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { + protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { + addProperties(g, node, vNode, new HashMap<>()); + } + + protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode, HashMap overrides) { GraphTraversal traversal = g.V(vNode.id()); TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); @@ -122,10 +180,14 @@ protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNod String key = entry.getKey(); Object value = entry.getValue(); value = adjustPropertyValue(node, key, value); + // Override value for key if requested + if (overrides.containsKey(key)) { + value = overrides.get(key); + } addProperty(previouslyInsertedKeys, traversal, key, value); } - for (Map.Entry entry : getAdditionalProperties(node).entrySet()) { + for (Map.Entry entry : getAdditionalProperties(node, overrides).entrySet()) { addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); } @@ -139,11 +201,37 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { } /** Add additional properties to the node that aren't present in the orginal AST */ - protected Map getAdditionalProperties(JorjeNode node) { - return new HashMap<>(); + protected Map getAdditionalProperties(JorjeNode node, HashMap overrides) { + final HashMap returnMap = new HashMap<>(); + + // FIXME: very crude + if (!overrides.isEmpty()) { + returnMap.put(Schema.IS_STATIC_BLOCK_METHOD, Boolean.valueOf(true)); + } + return returnMap; } - /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ + /** + * @return true if given node represents () + */ + private boolean isClintMethod(JorjeNode node) { + return ASTConstants.NodeType.METHOD.equals(node.getLabel()) + && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals(node.getProperties().get(Schema.NAME)); + } + + /** + * @return true if () node contains any static block definitions + */ + private boolean containsStaticBlock(JorjeNode node) { + for (JorjeNode childNode : node.getChildren()) { + if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { + return true; + } + } + return false; + } + + /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ protected void addProperty( TreeSet previouslyInsertedKeys, GraphTraversal traversal, diff --git a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java index 8486cd02f..beb8cda27 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java @@ -5,6 +5,7 @@ import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.logging.log4j.LogManager; @@ -72,8 +73,8 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { } @Override - protected Map getAdditionalProperties(JorjeNode node) { - Map result = super.getAdditionalProperties(node); + protected Map getAdditionalProperties(JorjeNode node, HashMap overrides) { + Map result = super.getAdditionalProperties(node, overrides); // Mark all nodes as being from the Standard library // TODO: Reduce number of nodes where this is set diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java index a7a5099a6..d98f77449 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java @@ -32,7 +32,7 @@ public static Optional getOnlyChild( if (children.isEmpty()) { return Optional.empty(); } else if (children.size() > 1) { - throw new UnexpectedException("Did not expect more than one child node. Actual count: " + children.size()); + throw new UnexpectedException("Did not expect more than one child node of type " + childLabel +". Actual count: " + children.size()); } else { return Optional.of(children.get(0)); } @@ -56,6 +56,10 @@ public static List getChildren(GraphTraversalSource g, Vertex vertex) { .toList(); } + public static List getChildren(GraphTraversalSource g, Vertex vertex, String childLabel) { + return g.V(vertex).out(Schema.CHILD).hasLabel(childLabel).toList(); + } + public static Optional getPreviousSibling(GraphTraversalSource g, Vertex vertex) { Iterator it = g.V(vertex).in(Schema.NEXT_SIBLING); if (it.hasNext()) { @@ -87,5 +91,14 @@ public static Optional getFirstChild(GraphTraversalSource g, Vertex vert } } + public static Optional getParent(GraphTraversalSource g, Vertex vertex) { + Iterator it = g.V(vertex).out(Schema.PARENT); + if (it.hasNext()) { + return Optional.of(it.next()); + } else { + return Optional.empty(); + } + } + private GremlinUtil() {} } diff --git a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java index 5d7ba6c86..805a13443 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java +++ b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java @@ -5,6 +5,8 @@ import com.salesforce.exception.TodoException; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; +import com.salesforce.graph.ops.CloneUtil; +import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.vertex.SFVertexFactory; import java.util.ArrayList; import java.util.Arrays; @@ -22,7 +24,8 @@ /** Visits a method and draws all control flow edges. */ public class MethodPathBuilderVisitor { private static final Logger LOGGER = LogManager.getLogger(MethodPathBuilderVisitor.class); - /** Load the vertices as SFVertices and log more information. Will impact performance. */ + private static final String NAME_PROPERTY = "Name_CaseSafe"; // FIXME: identify where this gets created for real + /** Load the vertices as SFVertices and log more information. Will impact performance. */ private static boolean SF_VERTEX_LOGGING = true; /** @@ -87,16 +90,48 @@ public static void apply(GraphTraversalSource g, Vertex methodVertex) { throw new UnexpectedException(methodVertex); } - Vertex blockStatement = - GremlinUtil.getOnlyChild(g, methodVertex, NodeType.BLOCK_STATEMENT).orElse(null); - // This can be null in cases such as default constructors that don't have an implementation - if (blockStatement != null) { - MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); - visitor._visit(blockStatement, methodVertex, true); - } + // Handle () methods + if (methodVertex.value(NAME_PROPERTY).equals(MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME)) { + handleClintMethod(g, methodVertex); + } else { + + Vertex blockStatement = + GremlinUtil.getOnlyChild(g, methodVertex, NodeType.BLOCK_STATEMENT).orElse(null); + // This can be null in cases such as default constructors that don't have an implementation + if (blockStatement != null) { + MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); + visitor._visit(blockStatement, methodVertex, true); + } + } } - private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { + + /** + * Handles default method generated by jorje for every class. + * Block statements under are static code blocks such as + * class MyClass { + * static { + * System.debug('hello'); + * } + * } + * @param g reference to graph + * @param methodVertex represents vertex of () + */ + private static void handleClintMethod(GraphTraversalSource g, Vertex methodVertex) { + final List blockStatements = GremlinUtil.getChildren(g, methodVertex, NodeType.BLOCK_STATEMENT); + for (Vertex blockStatement : blockStatements) { + // We should handle each block statement one after another, as if they were in the same method + // since they will all be executed one after another. + // TODO: Confirm that static blocks get executed in the same order they are listed in the class + if (blockStatement != null) { +// final Vertex staticBlockVertex = CloneUtil.clone(methodVertex); + final MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); + visitor._visit(blockStatement, methodVertex, true); + } + } + } + + private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { try { String label = vertex.label(); 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 9632bb1d6..8a81a5830 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -83,6 +83,7 @@ public final class MethodUtil { private static final String PAGE_REFERENCE = "PageReference"; public static final String INSTANCE_CONSTRUCTOR_CANONICAL_NAME = ""; public static final String STATIC_CONSTRUCTOR_CANONICAL_NAME = ""; + public static final String SYNTHETIC_STATIC_BLOCK_METHOD_NAME = "SyntheticStaticBlock_%d"; public static List getTargetedMethods( GraphTraversalSource g, List targets) { diff --git a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java index 940f23704..984016cd2 100644 --- a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java +++ b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java @@ -2,6 +2,9 @@ import static com.salesforce.apex.jorje.ASTConstants.NodeType; +import com.salesforce.Collectible; +import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.ApexPath; import com.salesforce.graph.DeepCloneable; @@ -10,7 +13,9 @@ import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.symbols.apex.ApexValue; import com.salesforce.graph.vertex.BaseSFVertex; +import com.salesforce.graph.vertex.BlockStatementVertex; import com.salesforce.graph.vertex.FieldDeclarationVertex; +import com.salesforce.graph.vertex.MethodVertex; import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.graph.vertex.UserClassVertex; import java.util.ArrayList; @@ -105,6 +110,35 @@ private static List getFieldDeclarations( return results; } + private static List getStaticBlocks( + GraphTraversalSource g, UserClassVertex userClass) { + List results = new ArrayList<>(); + + String superClassName = userClass.getSuperClassName().orElse(null); + if (superClassName != null) { + UserClassVertex superClass = ClassUtil.getUserClass(g, superClassName).orElse(null); + if (superClass != null) { + results.addAll(getStaticBlocks(g, superClass)); + } + } + + results.addAll( + SFVertexFactory.loadVertices( + g, + g.V(userClass.getId()) + .out(Schema.CHILD) + .hasLabel(ASTConstants.NodeType.METHOD) + .has(Schema.IS_STATIC_BLOCK_METHOD, true) + .order(Scope.global) + .by(Schema.CHILD_INDEX, Order.asc) + /*.out(Schema.CHILD) + .hasLabel(ASTConstants.NodeType.BLOCK_STATEMENT) + .order(Scope.global) + .by(Schema.CHILD_INDEX, Order.asc)*/)); + + return results; + } + /** * Returns a path that represents the static properties defined by the class. The following * example would contain a path for 'MyClass' that contains the Field and FieldDeclarations for @@ -122,11 +156,17 @@ public static Optional getInitializationPath( GraphTraversalSource g, String classname) { ClassStaticScope classStaticScope = ClassStaticScope.get(g, classname); List vertices = new ArrayList<>(); + List methodVertices = new ArrayList<>(); vertices.addAll(classStaticScope.getFields()); vertices.addAll(getFieldDeclarations(g, classStaticScope.userClass)); - if (vertices.isEmpty()) { + methodVertices.addAll(getStaticBlocks(g, classStaticScope.userClass)); + if (vertices.isEmpty() && methodVertices.isEmpty()) { return Optional.empty(); } else { + ArrayList> apexPaths = new ArrayList<>(); + for (MethodVertex methodVertex : methodVertices) { + apexPaths.add(new ApexPath(methodVertex)); + } ApexPath apexPath = new ApexPath(null); apexPath.addVertices(vertices); return Optional.of(apexPath); diff --git a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java index 3f0b59fd9..9c203c6d7 100644 --- a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java +++ b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java @@ -4,6 +4,7 @@ import com.salesforce.NullCollectible; import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.graph.Schema; +import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.symbols.SymbolProvider; import com.salesforce.graph.symbols.SymbolProviderVertexVisitor; import com.salesforce.graph.visitor.PathVertexVisitor; @@ -137,6 +138,41 @@ public ConstructorVertex getCollectible() { } } + public static final class StaticBlockVertex extends MethodVertex implements Collectible { + public static final NullCollectible NULL_VALUE = + new NullCollectible<>(StaticBlockVertex.class); + + private StaticBlockVertex(Map properties) { + super(properties); + } + + @Override + public boolean visit(PathVertexVisitor visitor, SymbolProvider symbols) { + return visitor.visit(this, symbols); + } + + @Override + public boolean visit(SymbolProviderVertexVisitor visitor) { + return visitor.visit(this); + } + + @Override + public void afterVisit(PathVertexVisitor visitor, SymbolProvider symbols) { + visitor.afterVisit(this, symbols); + } + + @Override + public void afterVisit(SymbolProviderVertexVisitor visitor) { + visitor.afterVisit(this); + } + + @Nullable + @Override + public StaticBlockVertex getCollectible() { + return this; + } + } + public static final class InstanceMethodVertex extends MethodVertex { private InstanceMethodVertex(Map properties) { super(properties); @@ -165,8 +201,14 @@ public void afterVisit(SymbolProviderVertexVisitor visitor) { public static final class Builder { public static MethodVertex create(Map vertex) { + final boolean isStaticBlockMethod = BaseSFVertex.toBoolean(vertex.get(Schema.IS_STATIC_BLOCK_METHOD)); boolean isConstructor = BaseSFVertex.toBoolean(vertex.get(Schema.CONSTRUCTOR)); - return isConstructor ? new ConstructorVertex(vertex) : new InstanceMethodVertex(vertex); + if (isStaticBlockMethod) { + return new StaticBlockVertex(vertex); + } else if (isConstructor) { + return new ConstructorVertex(vertex); + } + return new InstanceMethodVertex(vertex); } } } From d1eb1c51afbb045bcd21402e38213dc48eb3b944 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Fri, 24 Jun 2022 12:40:47 -0700 Subject: [PATCH 12/34] Solution to handle static blocks in code - draft --- .../java/com/salesforce/graph/ApexPath.java | 5 +- .../java/com/salesforce/graph/Schema.java | 6 + .../build/AbstractApexVertexBuilder.java | 181 ++-------- .../ApexStandardLibraryVertexBuilder.java | 4 +- .../graph/build/GremlinVertexUtil.java | 91 +++++ .../graph/build/MethodPathBuilderVisitor.java | 46 +-- .../graph/build/StaticBlockUtil.java | 322 ++++++++++++++++++ .../com/salesforce/graph/ops/MethodUtil.java | 3 +- .../graph/symbols/ClassStaticScope.java | 57 ++-- .../StaticAndAnonymousCodeBlockTest.java | 55 +++ 10 files changed, 555 insertions(+), 215 deletions(-) create mode 100644 sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java create mode 100644 sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java create mode 100644 sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java diff --git a/sfge/src/main/java/com/salesforce/graph/ApexPath.java b/sfge/src/main/java/com/salesforce/graph/ApexPath.java index 340c19c53..e18107183 100644 --- a/sfge/src/main/java/com/salesforce/graph/ApexPath.java +++ b/sfge/src/main/java/com/salesforce/graph/ApexPath.java @@ -269,7 +269,10 @@ public void addVertices(List vertices) { !(vertices.get(0) instanceof BlockStatementVertex) && // Class Instantiation Path - !(vertices.get(0) instanceof FieldVertex)) { + !(vertices.get(0) instanceof FieldVertex) + && + // Static blocks + !(vertices.get(0) instanceof MethodCallExpressionVertex)) { throw new UnexpectedException(vertices); } this.vertices.addAll(vertices); diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index a6a07f757..fb78d6258 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -70,6 +70,12 @@ public static final class JorjeNodeType { public static final String PARENT = "Parent"; public static final String NEXT_SIBLING = "NextSibling"; + /** Mark a vertex as synthetic */ + public static final String IS_SYNTHETIC = "IsSynthetic"; /** Indicates if a method is a synthetic static block method */ public static final String IS_STATIC_BLOCK_METHOD = "IsStaticBlockMethod"; + /** Indicates if a method is a synthetic static block invoker method */ + public static final String IS_STATIC_BLOCK_INVOKER_METHOD = "IsStaticBlockInvokerMethod"; + /** Indicates if a MethodCallExpression is a synthetic invocation of static block from invoker method*/ + public static final String IS_STATIC_BLOCK_INVOCATION = "IsStaticBlockInvocation"; } diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index 7a3a93d64..26d696a76 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -6,13 +6,12 @@ import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; -import com.salesforce.graph.ops.MethodUtil; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.TreeSet; import org.apache.commons.lang3.StringUtils; @@ -74,81 +73,43 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { vNode.property(Schema.FILE_NAME, fileName); } - final boolean needsStaticBlockSplHandling = isClintMethod(node) && containsStaticBlock(node); - Vertex vPreviousSibling = null; + Vertex vPreviousSibling = null; final List children = node.getChildren(); + final Set verticesAddressed = new HashSet<>(); + verticesAddressed.add(vNode); for (int i = 0; i < children.size(); i++) { final JorjeNode child = children.get(i); - if (needsStaticBlockSplHandling - && ASTConstants.NodeType.BLOCK_STATEMENT.equals(child.getLabel())) { - // todo: add static blocks as new methods - vPreviousSibling = addStaticBlockAsNewMethod(vNode, vPreviousSibling, child, node, i); + final Vertex vChild = g.addV(child.getLabel()).next(); + addProperties(g, child, vChild); + + // Handle static block if needed + if (StaticBlockUtil.isStaticBlockStatement(node, child)) { + final Vertex parentVertexForChild = StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, vChild, i); + GremlinVertexUtil.addParentChildRelationship(g, parentVertexForChild, vChild); + verticesAddressed.add(parentVertexForChild); } else { - vPreviousSibling = addChildNodeToGraph(vNode, vPreviousSibling, child); + GremlinVertexUtil.addParentChildRelationship(g, vNode, vChild); } + + if (vPreviousSibling != null) { + g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(vChild).iterate(); + } + vPreviousSibling = vChild; + + // To save memory in the graph, don't pass the source name into recursive calls. + buildVertices(child, vChild, null); + } + // Execute afterInsert() on each vertex we addressed + for (Vertex vertex: verticesAddressed) { + afterInsert(g, node, vertex); } - afterInsert(g, node, vNode); if (rootVNode != null) { // Only call this for the root node afterFileInsert(g, rootVNode); } } - private Vertex addChildNodeToGraph(Vertex vNode, Vertex vPreviousSibling, JorjeNode child) { - Vertex vChild = g.addV(child.getLabel()).next(); - addProperties(g, child, vChild); - return addParentChildRelationship(vNode, vChild, child, vPreviousSibling); - } - - private Vertex addParentChildRelationship(Vertex parentVertex, Vertex childVertex, JorjeNode child, Vertex vPreviousSibling) { - addParentChildRelationship(parentVertex, childVertex); - if (vPreviousSibling != null) { - g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(childVertex).iterate(); - } - vPreviousSibling = childVertex; - // To save memory in the graph, don't pass the source name into recursive calls. - buildVertices(child, childVertex, null); - return vPreviousSibling; - } - - private void addParentChildRelationship(Vertex parentVertex, Vertex childVertex) { - // We are currently adding PARENT and CHILD, in theory the same edge could be navigated - // both ways, however - // the code looks messy when that is done. - // TODO: Determine if this causes performance or resource issues. Consider a single edge - g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate(); - g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate(); - } - - private Vertex addStaticBlockAsNewMethod(Vertex clintVertex, Vertex vPreviousSibling, JorjeNode child, JorjeNode clintNode, int staticBlockIndex) { - // Add properties of () to syntheticMethodVertex, - // override properties with a new name and an indicator to - // show this is a synthetic static block method - final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); - - final HashMap overrides = new HashMap<>(); - overrides.put(Schema.NAME, String.format(MethodUtil.SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); - addProperties(g, clintNode, syntheticMethodVertex, overrides); - - // TODO: Add vertex's parent as Synthetic method vertex's parent - // how do i get 's parent vertex? - - final Optional grandParentVertex = GremlinUtil.getParent(g, clintVertex); - if (grandParentVertex.isEmpty()) { - throw new UnexpectedException("Did not expect () to not have a parent vertex. clintVertex=" + clintVertex); - } - - addParentChildRelationship(grandParentVertex.get(), syntheticMethodVertex); - - // Create a vertex for BlockStatement - final Vertex vChild = g.addV(child.getLabel()).next(); - addProperties(g, child, vChild); - - // add BlockStatement as child of SyntheticMethodVertex - return addParentChildRelationship(syntheticMethodVertex, vChild, child, vPreviousSibling); - } - /** * Adds edges to Method vertices after they are inserted. TODO: Replace this with a listener or * visitor pattern for more general purpose solutions @@ -156,7 +117,9 @@ private Vertex addStaticBlockAsNewMethod(Vertex clintVertex, Vertex vPreviousSib private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vNode) { if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) { MethodPathBuilderVisitor.apply(g, vNode); - } + } else if (ROOT_VERTICES.contains(node.getLabel())) { + StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode); + } } /** @@ -169,10 +132,6 @@ protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) { } protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { - addProperties(g, node, vNode, new HashMap<>()); - } - - protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode, HashMap overrides) { GraphTraversal traversal = g.V(vNode.id()); TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); @@ -180,15 +139,11 @@ protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNod String key = entry.getKey(); Object value = entry.getValue(); value = adjustPropertyValue(node, key, value); - // Override value for key if requested - if (overrides.containsKey(key)) { - value = overrides.get(key); - } - addProperty(previouslyInsertedKeys, traversal, key, value); + GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, key, value); } - for (Map.Entry entry : getAdditionalProperties(node, overrides).entrySet()) { - addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); + for (Map.Entry entry : getAdditionalProperties(node).entrySet()) { + GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); } // Commit the changes. @@ -201,78 +156,8 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { } /** Add additional properties to the node that aren't present in the orginal AST */ - protected Map getAdditionalProperties(JorjeNode node, HashMap overrides) { - final HashMap returnMap = new HashMap<>(); - - // FIXME: very crude - if (!overrides.isEmpty()) { - returnMap.put(Schema.IS_STATIC_BLOCK_METHOD, Boolean.valueOf(true)); - } - return returnMap; - } - - /** - * @return true if given node represents () - */ - private boolean isClintMethod(JorjeNode node) { - return ASTConstants.NodeType.METHOD.equals(node.getLabel()) - && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals(node.getProperties().get(Schema.NAME)); + protected Map getAdditionalProperties(JorjeNode node) { + return new HashMap<>(); } - /** - * @return true if () node contains any static block definitions - */ - private boolean containsStaticBlock(JorjeNode node) { - for (JorjeNode childNode : node.getChildren()) { - if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { - return true; - } - } - return false; - } - - /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ - protected void addProperty( - TreeSet previouslyInsertedKeys, - GraphTraversal traversal, - String keyParam, - Object value) { - final String key = keyParam.intern(); - - if (!previouslyInsertedKeys.add(key)) { - throw new UnexpectedException(key); - } - - if (value instanceof List) { - List list = (List) value; - // Convert ArrayList to an Array. There seems to be a Tinkerpop bug where a singleton - // ArrayList - // isn't properly stored. Remote graphs also have issues with empty lists, so don't add - // an empty list. - if (!list.isEmpty()) { - traversal.property(key, list.toArray()); - } else { - // return so that we don't store a case insensitive version - return; - } - } else if (value instanceof Boolean - || value instanceof Double - || value instanceof Integer - || value instanceof Long - || value instanceof String) { - traversal.property(key, value); - } else { - if (value != null) { - if (!(value instanceof Enum)) { - if (LOGGER.isWarnEnabled()) { - LOGGER.warn( - "Using string for value. type=" + value.getClass().getSimpleName()); - } - } - final String strValue = String.valueOf(value).intern(); - traversal.property(key, strValue); - } - } - CaseSafePropertyUtil.addCaseSafeProperty(traversal, key, value); - } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java index beb8cda27..a821e3804 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java @@ -73,8 +73,8 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { } @Override - protected Map getAdditionalProperties(JorjeNode node, HashMap overrides) { - Map result = super.getAdditionalProperties(node, overrides); + protected Map getAdditionalProperties(JorjeNode node) { + Map result = super.getAdditionalProperties(node); // Mark all nodes as being from the Standard library // TODO: Reduce number of nodes where this is set diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java new file mode 100644 index 000000000..4de95cd58 --- /dev/null +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java @@ -0,0 +1,91 @@ +package com.salesforce.graph.build; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import com.salesforce.exception.UnexpectedException; +import com.salesforce.graph.Schema; + +import java.util.List; +import java.util.Optional; +import java.util.TreeSet; + +/** + * Handles common operations performed while creating vertices on Graph + */ +public final class GremlinVertexUtil { + private static final Logger LOGGER = LogManager.getLogger(GremlinVertexUtil.class); + private GremlinVertexUtil(){} + + /** + * Create parent-child relationship between vertices on graph + */ + static void addParentChildRelationship(GraphTraversalSource g, Vertex parentVertex, Vertex childVertex) { + // We are currently adding PARENT and CHILD, in theory the same edge could be navigated + // both ways, however + // the code looks messy when that is done. + // TODO: Determine if this causes performance or resource issues. Consider a single edge + g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate(); + g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate(); + } + + /** + * Make a synthetic vertex a sibling of an existing vertex on graph + */ + static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex syntheticVertex) { + // Get parent node of vertex + final Optional rootVertex = GremlinUtil.getParent(g, vertex); + if (rootVertex.isEmpty()) { + throw new UnexpectedException("Did not expect vertex to not have a parent vertex. vertex=" + vertex); + } + + addParentChildRelationship(g, rootVertex.get(), syntheticVertex); + } + + /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ + protected static void addProperty( + TreeSet previouslyInsertedKeys, + GraphTraversal traversal, + String keyParam, + Object value) { + final String key = keyParam.intern(); + + if (!previouslyInsertedKeys.add(key)) { + throw new UnexpectedException(key); + } + + if (value instanceof List) { + List list = (List) value; + // Convert ArrayList to an Array. There seems to be a Tinkerpop bug where a singleton + // ArrayList + // isn't properly stored. Remote graphs also have issues with empty lists, so don't add + // an empty list. + if (!list.isEmpty()) { + traversal.property(key, list.toArray()); + } else { + // return so that we don't store a case insensitive version + return; + } + } else if (value instanceof Boolean + || value instanceof Double + || value instanceof Integer + || value instanceof Long + || value instanceof String) { + traversal.property(key, value); + } else { + if (value != null) { + if (!(value instanceof Enum)) { + if (LOGGER.isWarnEnabled()) { + LOGGER.warn( + "Using string for value. type=" + value.getClass().getSimpleName()); + } + } + final String strValue = String.valueOf(value).intern(); + traversal.property(key, strValue); + } + } + CaseSafePropertyUtil.addCaseSafeProperty(traversal, key, value); + } +} diff --git a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java index 805a13443..33c53ae82 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java +++ b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java @@ -24,7 +24,6 @@ /** Visits a method and draws all control flow edges. */ public class MethodPathBuilderVisitor { private static final Logger LOGGER = LogManager.getLogger(MethodPathBuilderVisitor.class); - private static final String NAME_PROPERTY = "Name_CaseSafe"; // FIXME: identify where this gets created for real /** Load the vertices as SFVertices and log more information. Will impact performance. */ private static boolean SF_VERTEX_LOGGING = true; @@ -90,47 +89,16 @@ public static void apply(GraphTraversalSource g, Vertex methodVertex) { throw new UnexpectedException(methodVertex); } - // Handle () methods - if (methodVertex.value(NAME_PROPERTY).equals(MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME)) { - handleClintMethod(g, methodVertex); - } else { - - Vertex blockStatement = - GremlinUtil.getOnlyChild(g, methodVertex, NodeType.BLOCK_STATEMENT).orElse(null); - // This can be null in cases such as default constructors that don't have an implementation - if (blockStatement != null) { - MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); - visitor._visit(blockStatement, methodVertex, true); - } - } + Vertex blockStatement = + GremlinUtil.getOnlyChild(g, methodVertex, NodeType.BLOCK_STATEMENT).orElse(null); + // This can be null in cases such as default constructors that don't have an implementation + if (blockStatement != null) { + MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); + visitor._visit(blockStatement, methodVertex, true); + } } - /** - * Handles default method generated by jorje for every class. - * Block statements under are static code blocks such as - * class MyClass { - * static { - * System.debug('hello'); - * } - * } - * @param g reference to graph - * @param methodVertex represents vertex of () - */ - private static void handleClintMethod(GraphTraversalSource g, Vertex methodVertex) { - final List blockStatements = GremlinUtil.getChildren(g, methodVertex, NodeType.BLOCK_STATEMENT); - for (Vertex blockStatement : blockStatements) { - // We should handle each block statement one after another, as if they were in the same method - // since they will all be executed one after another. - // TODO: Confirm that static blocks get executed in the same order they are listed in the class - if (blockStatement != null) { -// final Vertex staticBlockVertex = CloneUtil.clone(methodVertex); - final MethodPathBuilderVisitor visitor = new MethodPathBuilderVisitor(g); - visitor._visit(blockStatement, methodVertex, true); - } - } - } - private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { try { String label = vertex.label(); diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java new file mode 100644 index 000000000..78ccbef2a --- /dev/null +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -0,0 +1,322 @@ +package com.salesforce.graph.build; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; + +import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.apex.jorje.JorjeNode; +import com.salesforce.collections.CollectionUtil; +import com.salesforce.exception.ProgrammingException; +import com.salesforce.graph.Schema; +import com.salesforce.graph.ops.MethodUtil; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.TreeSet; + +/** + * Handles creation of synthetic methods and vertices to gracefully invoke static code blocks. + * + * + * Consider this example: + * class StaticBlockClass { + * static { + * System.debug("inside static block 1"); + * } + * static { + * System.debug("inside static block 2"); + * } + * } + * In Jorje's compilation structure, static blocks are represented like this: + * class StaticBlockClass { + * private static void () { + * { + * System.debug("inside static block 1"); + * } + * { + * System.debug("inside static block 2"); + * } + * } + * } + * + * Having multiple block statements inside a method breaks SFGE's makes handling code blocks in () + * impossible. As an alternative, we are creating synthetic vertices in the Graph to represent the static + * blocks as individual methods. We also create one top-level synthetic method ("StaticBlockInvoker") that invokes individual static + * block methods. While creating static scope for this class, we should invoke the method call expressions + * inside the top-level synthetic method. + * + * New structure looks like this: + * class StaticBlockClass { + * private static void SyntheticStaticBlock_1() { + * System.debug("inside static block 1"); + * } + * + * private static void SyntheticStaticBlock_2() { + * System.debug("inside static block 2"); + * } + * + * private static void StaticBlockInvoker() { + * SyntheticStaticBlock_1(); + * SyntheticStaticBlock_2(); + * } + * } + * + */ +public final class StaticBlockUtil { + private static final Logger LOGGER = LogManager.getLogger(StaticBlockUtil.class); + + private static final String SYNTHETIC_STATIC_BLOCK_METHOD_NAME = "SyntheticStaticBlock_%d"; + private static final String STATIC_BLOCK_INVOKER_METHOD = "StaticBlockInvoker"; + + private StaticBlockUtil() {} + + /** + * Creates a synthetic method vertex to represent a static code block + * @param g traversal graph + * @param clintVertex () method's vertex + * @param blockStatementVertex static block originally placed by Jorje under () + * @param staticBlockIndex index to use for name uniqueness - + * TODO: this index is currently just the child count index and does not + * necessarily follow sequence + * @return new synthetic method vertex with name SyntheticStaticBlock_%d, which will be the parent + * for blockStatementVertex, and a sibling of () + */ + public static Vertex createSyntheticStaticBlockMethod(GraphTraversalSource g, Vertex clintVertex, Vertex blockStatementVertex, int staticBlockIndex) { + final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); + final String definingType = clintVertex.value(Schema.DEFINING_TYPE); + addSyntheticStaticBlockMethodProperties(g, definingType, syntheticMethodVertex, staticBlockIndex); + + final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); + addStaticModifierProperties(g, definingType, modifierNodeVertex); + GremlinVertexUtil.addParentChildRelationship(g, syntheticMethodVertex, modifierNodeVertex); + + GremlinVertexUtil.makeSiblings(g, clintVertex, syntheticMethodVertex); + + return syntheticMethodVertex; + } + + /** + * If rootNode contains synthetic static block methods (created through {@link #createSyntheticStaticBlockMethod}), + * adds a top-level synthetic method that invokes each static block method. + */ + public static void createSyntheticStaticBlockInvocation(GraphTraversalSource g, Vertex rootNode) { + // Check if root node contains any static block methods + final List staticBlockMethods = getStaticBlockMethods(g, rootNode); + + // Create static block invocation method + if (!staticBlockMethods.isEmpty()) { + final String definingType = rootNode.value(Schema.DEFINING_TYPE); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Creating synthetic invocation method for {} to invoke {} static block(s)", definingType, staticBlockMethods.size()); + } + + final Vertex invokerMethodVertex = createSyntheticInvocationsMethod(g, rootNode, staticBlockMethods, definingType); + MethodPathBuilderVisitor.apply(g, invokerMethodVertex); + } + } + + static boolean isStaticBlockStatement(JorjeNode node, JorjeNode child) { + return isClintMethod(node) + && containsStaticBlock(node) + && isBlockStatement(child); + } + + private static Vertex createSyntheticInvocationsMethod(GraphTraversalSource g, Vertex rootNode, List staticBlockMethods, String definingType) { + // Create new synthetic method StaticBlockInvoker to invoke each synthetic static block method + final Vertex invokerMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); + addStaticBlockInvokerProperties(g, definingType, invokerMethodVertex); + GremlinVertexUtil.addParentChildRelationship(g, rootNode, invokerMethodVertex); + + final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); + addStaticModifierProperties(g, definingType, modifierNodeVertex); + GremlinVertexUtil.addParentChildRelationship(g, invokerMethodVertex, modifierNodeVertex); + + // Create synthetic BlockStatement inside StaticBlockInvoker to hold the method invocations + final Vertex blockStatementInInvoker = g.addV(ASTConstants.NodeType.BLOCK_STATEMENT).next(); + addBlockStatementProperties(g, definingType, blockStatementInInvoker); + GremlinVertexUtil.addParentChildRelationship(g, invokerMethodVertex, blockStatementInInvoker); + + for (int i = 0; i < staticBlockMethods.size(); i++) { + final Vertex staticBlockMethod = staticBlockMethods.get(i); + boolean isLastMethod = i == staticBlockMethods.size() - 1; + // Create a method call invocation to the synthetic static block method + final Vertex exprStaticBlockMethodCall = createMethodCallExpression(g, definingType, staticBlockMethod, isLastMethod); + + // Add the expression statement containing the method call inside StaticBlockInvoker's block statement + GremlinVertexUtil.addParentChildRelationship(g, blockStatementInInvoker, exprStaticBlockMethodCall); + } + + return invokerMethodVertex; + } + + private static Vertex createMethodCallExpression(GraphTraversalSource g, String definingType, Vertex staticBlockMethod, boolean isLastMethod) { + final Vertex expressionStmt = g.addV(ASTConstants.NodeType.EXPRESSION_STATEMENT).next(); + addExpressionStmtProperties(g, definingType, expressionStmt, isLastMethod); + + // Create new MethodCallExpression for the synthetic static block method + final Vertex staticBlockMethodCall = g.addV(ASTConstants.NodeType.METHOD_CALL_EXPRESSION).next(); + addMethodCallExpressionProperties(g, definingType, staticBlockMethod, staticBlockMethodCall); + GremlinVertexUtil.addParentChildRelationship(g, expressionStmt, staticBlockMethodCall); + + // Create EmptyReferenceExpression inside MethodCallExpression + final Vertex emptyMethodReference = g.addV(ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION).next(); + addEmptyMethodReferenceProperties(g, definingType, emptyMethodReference); + GremlinVertexUtil.addParentChildRelationship(g, staticBlockMethodCall, emptyMethodReference); + return expressionStmt; + } + + private static void addExpressionStmtProperties(GraphTraversalSource g, String definingType, Vertex expressionStmt, boolean isLastMethod) { + verifyType(expressionStmt, ASTConstants.NodeType.EXPRESSION_STATEMENT); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, expressionStmt, properties); + } + + private static void addEmptyMethodReferenceProperties(GraphTraversalSource g, String definingType, Vertex emptyMethodReference) { + verifyType(emptyMethodReference, ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, emptyMethodReference, properties); + } + + private static void addMethodCallExpressionProperties(GraphTraversalSource g, String definingType, Vertex staticBlockMethod, Vertex staticBlockMethodCall) { + verifyType(staticBlockMethodCall, ASTConstants.NodeType.METHOD_CALL_EXPRESSION); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.METHOD_NAME, staticBlockMethod.value(Schema.NAME)); + properties.put(Schema.FULL_METHOD_NAME, staticBlockMethod.value(Schema.NAME)); + properties.put(Schema.IS_STATIC_BLOCK_INVOCATION, true); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, staticBlockMethodCall, properties); + } + + private static void addBlockStatementProperties(GraphTraversalSource g, String definingType, Vertex blockStatementInInvoker) { + verifyType(blockStatementInInvoker, ASTConstants.NodeType.BLOCK_STATEMENT); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.CHILD_INDEX, 1); + properties.put(Schema.FIRST_CHILD, false); + properties.put(Schema.LAST_CHILD, true); + + addProperties(g, blockStatementInInvoker, properties); + } + + private static void addStaticModifierProperties(GraphTraversalSource g, String definingType, Vertex modifier) { + verifyType(modifier, ASTConstants.NodeType.MODIFIER_NODE); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.STATIC, true); + properties.put(Schema.ABSTRACT, false); + properties.put(Schema.GLOBAL, false); + properties.put(Schema.MODIFIERS, 8); // Apparently, static methods have modifiers 8 + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, false); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, modifier, properties); + } + + private static void addStaticBlockInvokerProperties(GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex) { + verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); + final Map properties = new HashMap<>(); + properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); + properties.put(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true); + addCommonSynthMethodProperties(g, definingType, staticBlockInvokerVertex, properties); + } + + private static void addSyntheticStaticBlockMethodProperties(GraphTraversalSource g, String definingType, Vertex syntheticMethodVertex, int staticBlockIndex) { + verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); + final Map properties = new HashMap<>(); + properties.put(Schema.NAME, String.format(SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); + properties.put(Schema.IS_STATIC_BLOCK_METHOD, true); + + addCommonSynthMethodProperties(g, definingType, syntheticMethodVertex, properties); + } + + private static void addCommonSynthMethodProperties(GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex, Map properties) { + properties.put(Schema.ARITY, 0); + properties.put(Schema.CONSTRUCTOR, false); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.RETURN_TYPE, ASTConstants.TYPE_VOID); + + + addProperties(g, staticBlockInvokerVertex, properties); + } + + private static void addProperties(GraphTraversalSource g, Vertex vertex, Map properties) { + final TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); + final GraphTraversal traversal = g.V(vertex.id()); + + for (Map.Entry entry : properties.entrySet()) { + GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); + previouslyInsertedKeys.add(entry.getKey()); + } + + // Commit the changes. + traversal.next(); + } + + /** + * @return true if given node represents () + */ + private static boolean isClintMethod(JorjeNode node) { + return ASTConstants.NodeType.METHOD.equals(node.getLabel()) + && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals(node.getProperties().get(Schema.NAME)); + } + + /** + * @return true if () node contains any static block definitions + */ + private static boolean containsStaticBlock(JorjeNode node) { + for (JorjeNode childNode : node.getChildren()) { + if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { + return true; + } + } + return false; + } + + private static List getStaticBlockMethods(GraphTraversalSource g, Vertex root) { + return g.V(root) + .out(Schema.CHILD) + .hasLabel(ASTConstants.NodeType.METHOD) + .has(Schema.IS_STATIC_BLOCK_METHOD, true) + .toList(); + } + + private static boolean isBlockStatement(JorjeNode child) { + return ASTConstants.NodeType.BLOCK_STATEMENT.equals(child.getLabel()); + } + + private static void verifyType(Vertex vertex, String expectedType) { + if (! expectedType.equals(vertex.label())) { + throw new ProgrammingException("Incorrect vertex type: " + vertex.label()); + } + } +} 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 8a81a5830..bcc92ee77 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -83,9 +83,8 @@ public final class MethodUtil { private static final String PAGE_REFERENCE = "PageReference"; public static final String INSTANCE_CONSTRUCTOR_CANONICAL_NAME = ""; public static final String STATIC_CONSTRUCTOR_CANONICAL_NAME = ""; - public static final String SYNTHETIC_STATIC_BLOCK_METHOD_NAME = "SyntheticStaticBlock_%d"; - public static List getTargetedMethods( + public static List getTargetedMethods( GraphTraversalSource g, List targets) { // The targets passed into this method should exclusively be ones that target specific // methods instead of files. diff --git a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java index 984016cd2..036857020 100644 --- a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java +++ b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java @@ -15,15 +15,18 @@ import com.salesforce.graph.vertex.BaseSFVertex; import com.salesforce.graph.vertex.BlockStatementVertex; import com.salesforce.graph.vertex.FieldDeclarationVertex; +import com.salesforce.graph.vertex.MethodCallExpressionVertex; import com.salesforce.graph.vertex.MethodVertex; import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.graph.vertex.UserClassVertex; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Optional; import org.apache.tinkerpop.gremlin.process.traversal.Order; import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.T; /** Used when invoking a static method on a class. */ public final class ClassStaticScope extends AbstractClassScope @@ -110,9 +113,9 @@ private static List getFieldDeclarations( return results; } - private static List getStaticBlocks( + private static List getStaticBlocks( GraphTraversalSource g, UserClassVertex userClass) { - List results = new ArrayList<>(); + List results = new ArrayList<>(); String superClassName = userClass.getSuperClassName().orElse(null); if (superClassName != null) { @@ -122,24 +125,27 @@ private static List getStaticBlocks( } } - results.addAll( - SFVertexFactory.loadVertices( - g, - g.V(userClass.getId()) - .out(Schema.CHILD) - .hasLabel(ASTConstants.NodeType.METHOD) - .has(Schema.IS_STATIC_BLOCK_METHOD, true) - .order(Scope.global) - .by(Schema.CHILD_INDEX, Order.asc) - /*.out(Schema.CHILD) - .hasLabel(ASTConstants.NodeType.BLOCK_STATEMENT) - .order(Scope.global) - .by(Schema.CHILD_INDEX, Order.asc)*/)); + results.addAll(SFVertexFactory.loadVertices( + g, + g.V(userClass.getId()) + .out(Schema.CHILD) + .hasLabel(NodeType.METHOD) + .has(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true) + .order(Scope.global) + .by(Schema.CHILD_INDEX, Order.asc) + .out(Schema.CHILD) + .hasLabel(ASTConstants.NodeType.BLOCK_STATEMENT) + .order(Scope.global) + .by(Schema.CHILD_INDEX, Order.asc) + .out(Schema.CHILD) + .hasLabel(NodeType.EXPRESSION_STATEMENT) + .out(Schema.CHILD) + .hasLabel(NodeType.METHOD_CALL_EXPRESSION))); return results; } - /** + /** * Returns a path that represents the static properties defined by the class. The following * example would contain a path for 'MyClass' that contains the Field and FieldDeclarations for * 's'. {@code @@ -156,20 +162,25 @@ public static Optional getInitializationPath( GraphTraversalSource g, String classname) { ClassStaticScope classStaticScope = ClassStaticScope.get(g, classname); List vertices = new ArrayList<>(); - List methodVertices = new ArrayList<>(); vertices.addAll(classStaticScope.getFields()); vertices.addAll(getFieldDeclarations(g, classStaticScope.userClass)); - methodVertices.addAll(getStaticBlocks(g, classStaticScope.userClass)); - if (vertices.isEmpty() && methodVertices.isEmpty()) { + vertices.addAll(getStaticBlocks(g, classStaticScope.userClass)); + if (vertices.isEmpty()) { return Optional.empty(); } else { - ArrayList> apexPaths = new ArrayList<>(); - for (MethodVertex methodVertex : methodVertices) { - apexPaths.add(new ApexPath(methodVertex)); - } ApexPath apexPath = new ApexPath(null); apexPath.addVertices(vertices); return Optional.of(apexPath); } } + +// private static MethodCallExpressionVertex createSyntheticInvocation(MethodVertex.StaticBlockVertex methodVertex) { +// final HashMap map = new HashMap<>(); +// map.put(T.id, Long.valueOf(-1)); +// map.put(T.label, NodeType.METHOD_CALL_EXPRESSION); +// map.put(Schema.METHOD_NAME, methodVertex.getName()); +// map.put(Schema.DEFINING_TYPE, methodVertex.getDefiningType()); +// map.put(Schema.STATIC, Boolean.valueOf(true)); +// return new MethodCallExpressionVertex(map); +// } } diff --git a/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java b/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java new file mode 100644 index 000000000..e31367cbf --- /dev/null +++ b/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java @@ -0,0 +1,55 @@ +package com.salesforce.graph.build; + +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; +import com.salesforce.TestRunner; +import com.salesforce.TestUtil; +import com.salesforce.graph.symbols.apex.ApexStringValue; +import com.salesforce.graph.symbols.apex.ApexValue; +import com.salesforce.graph.visitor.SystemDebugAccumulator; +import com.salesforce.matchers.TestRunnerMatcher; + +import java.util.List; +import java.util.Optional; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; + +public class StaticAndAnonymousCodeBlockTest extends MethodPathBuilderTest { + @Test + public void testStaticBlock() { + String[] sourceCode = + { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " StaticBlockClass sb = new StaticBlockClass();\n" + + " System.debug(sb.foo());\n" + + " }\n" + + "}" + }; + +// TestUtil.buildGraph(g, sourceCode); + + TestRunner.Result result = TestRunner.walkPath(g, + sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(2)); + + // verify that static block is invoked first + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("static block")); + + } +} From 81ba383b14239f0666453974075a23e01244484a Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 30 Jun 2022 10:50:15 -0700 Subject: [PATCH 13/34] Invoking static block from static class scope --- sfge/src/main/java/com/salesforce/Main.java | 3 +- .../java/com/salesforce/graph/ApexPath.java | 6 +- .../java/com/salesforce/graph/Schema.java | 19 +- .../build/AbstractApexVertexBuilder.java | 82 +-- .../ApexStandardLibraryVertexBuilder.java | 1 - .../salesforce/graph/build/GremlinUtil.java | 29 +- .../graph/build/GremlinVertexUtil.java | 72 +- .../graph/build/MethodPathBuilderVisitor.java | 7 +- .../graph/build/StaticBlockUtil.java | 616 +++++++++--------- .../com/salesforce/graph/ops/MethodUtil.java | 2 +- .../graph/ops/expander/ApexPathExpander.java | 16 +- .../graph/symbols/ClassStaticScope.java | 15 +- .../salesforce/graph/vertex/MethodVertex.java | 73 ++- .../graph/build/GraphBuildTestUtil.java | 61 ++ .../graph/build/MethodPathBuilderTest.java | 160 ++--- .../StaticAndAnonymousCodeBlockTest.java | 55 -- .../graph/build/StaticBlockUtilTest.java | 117 ++++ .../StaticCodeBlockInvocationTest.java | 300 +++++++++ 18 files changed, 1018 insertions(+), 616 deletions(-) create mode 100644 sfge/src/test/java/com/salesforce/graph/build/GraphBuildTestUtil.java delete mode 100644 sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java create mode 100644 sfge/src/test/java/com/salesforce/graph/build/StaticBlockUtilTest.java create mode 100644 sfge/src/test/java/com/salesforce/graph/symbols/StaticCodeBlockInvocationTest.java diff --git a/sfge/src/main/java/com/salesforce/Main.java b/sfge/src/main/java/com/salesforce/Main.java index 9493ac13f..a4ea7e1f7 100644 --- a/sfge/src/main/java/com/salesforce/Main.java +++ b/sfge/src/main/java/com/salesforce/Main.java @@ -132,7 +132,8 @@ private int execute(String... args) { return INTERNAL_ERROR; } catch (UnexpectedException ex) { LOGGER.error("Unexpected exception while loading graph", ex); - System.err.println("Unexpected exception while loading graph. See logs for more information."); + System.err.println( + "Unexpected exception while loading graph. See logs for more information."); return INTERNAL_ERROR; } diff --git a/sfge/src/main/java/com/salesforce/graph/ApexPath.java b/sfge/src/main/java/com/salesforce/graph/ApexPath.java index e18107183..08dc08eb7 100644 --- a/sfge/src/main/java/com/salesforce/graph/ApexPath.java +++ b/sfge/src/main/java/com/salesforce/graph/ApexPath.java @@ -270,9 +270,9 @@ public void addVertices(List vertices) { && // Class Instantiation Path !(vertices.get(0) instanceof FieldVertex) - && - // Static blocks - !(vertices.get(0) instanceof MethodCallExpressionVertex)) { + && + // Static blocks + !(vertices.get(0) instanceof MethodCallExpressionVertex)) { throw new UnexpectedException(vertices); } this.vertices.addAll(vertices); diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index fb78d6258..26cd02b95 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -70,12 +70,15 @@ public static final class JorjeNodeType { public static final String PARENT = "Parent"; public static final String NEXT_SIBLING = "NextSibling"; - /** Mark a vertex as synthetic */ - public static final String IS_SYNTHETIC = "IsSynthetic"; - /** Indicates if a method is a synthetic static block method */ - public static final String IS_STATIC_BLOCK_METHOD = "IsStaticBlockMethod"; - /** Indicates if a method is a synthetic static block invoker method */ - public static final String IS_STATIC_BLOCK_INVOKER_METHOD = "IsStaticBlockInvokerMethod"; - /** Indicates if a MethodCallExpression is a synthetic invocation of static block from invoker method*/ - public static final String IS_STATIC_BLOCK_INVOCATION = "IsStaticBlockInvocation"; + /** Mark a vertex as synthetic */ + public static final String IS_SYNTHETIC = "IsSynthetic"; + /** Indicates if a method is a synthetic static block method */ + public static final String IS_STATIC_BLOCK_METHOD = "IsStaticBlockMethod"; + /** Indicates if a method is a synthetic static block invoker method */ + public static final String IS_STATIC_BLOCK_INVOKER_METHOD = "IsStaticBlockInvokerMethod"; + /** + * Indicates if a MethodCallExpression is a synthetic invocation of static block from invoker + * method + */ + public static final String IS_STATIC_BLOCK_INVOCATION = "IsStaticBlockInvocation"; } diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index 26d696a76..bdcef65c6 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -6,7 +6,6 @@ import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; - import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -73,53 +72,54 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { vNode.property(Schema.FILE_NAME, fileName); } - Vertex vPreviousSibling = null; - final List children = node.getChildren(); - final Set verticesAddressed = new HashSet<>(); - verticesAddressed.add(vNode); - - for (int i = 0; i < children.size(); i++) { - final JorjeNode child = children.get(i); - final Vertex vChild = g.addV(child.getLabel()).next(); - addProperties(g, child, vChild); - - // Handle static block if needed - if (StaticBlockUtil.isStaticBlockStatement(node, child)) { - final Vertex parentVertexForChild = StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, vChild, i); - GremlinVertexUtil.addParentChildRelationship(g, parentVertexForChild, vChild); - verticesAddressed.add(parentVertexForChild); - } else { - GremlinVertexUtil.addParentChildRelationship(g, vNode, vChild); - } - - if (vPreviousSibling != null) { - g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(vChild).iterate(); - } - vPreviousSibling = vChild; - - // To save memory in the graph, don't pass the source name into recursive calls. - buildVertices(child, vChild, null); - } - // Execute afterInsert() on each vertex we addressed - for (Vertex vertex: verticesAddressed) { - afterInsert(g, node, vertex); - } + Vertex vPreviousSibling = null; + final List children = node.getChildren(); + final Set verticesAddressed = new HashSet<>(); + verticesAddressed.add(vNode); + + for (int i = 0; i < children.size(); i++) { + final JorjeNode child = children.get(i); + final Vertex vChild = g.addV(child.getLabel()).next(); + addProperties(g, child, vChild); + + // Handle static block if needed + if (StaticBlockUtil.isStaticBlockStatement(node, child)) { + final Vertex parentVertexForChild = + StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, vChild, i); + GremlinVertexUtil.addParentChildRelationship(g, parentVertexForChild, vChild); + verticesAddressed.add(parentVertexForChild); + } else { + GremlinVertexUtil.addParentChildRelationship(g, vNode, vChild); + } + + if (vPreviousSibling != null) { + g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(vChild).iterate(); + } + vPreviousSibling = vChild; + + // To save memory in the graph, don't pass the source name into recursive calls. + buildVertices(child, vChild, null); + } + // Execute afterInsert() on each vertex we addressed + for (Vertex vertex : verticesAddressed) { + afterInsert(g, node, vertex); + } if (rootVNode != null) { // Only call this for the root node afterFileInsert(g, rootVNode); } } - /** + /** * Adds edges to Method vertices after they are inserted. TODO: Replace this with a listener or * visitor pattern for more general purpose solutions */ private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vNode) { if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) { - MethodPathBuilderVisitor.apply(g, vNode); + MethodPathBuilderVisitor.apply(g, vNode); } else if (ROOT_VERTICES.contains(node.getLabel())) { - StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode); - } + StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode); + } } /** @@ -131,7 +131,7 @@ protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) { // Intentionally left blank } - protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { + protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { GraphTraversal traversal = g.V(vNode.id()); TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); @@ -143,7 +143,8 @@ protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNod } for (Map.Entry entry : getAdditionalProperties(node).entrySet()) { - GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); + GremlinVertexUtil.addProperty( + previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); } // Commit the changes. @@ -157,7 +158,6 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { /** Add additional properties to the node that aren't present in the orginal AST */ protected Map getAdditionalProperties(JorjeNode node) { - return new HashMap<>(); - } - + return new HashMap<>(); + } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java index a821e3804..8486cd02f 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java @@ -5,7 +5,6 @@ import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.logging.log4j.LogManager; diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java index d98f77449..a34fc84eb 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java @@ -32,7 +32,11 @@ public static Optional getOnlyChild( if (children.isEmpty()) { return Optional.empty(); } else if (children.size() > 1) { - throw new UnexpectedException("Did not expect more than one child node of type " + childLabel +". Actual count: " + children.size()); + throw new UnexpectedException( + "Did not expect more than one child node of type " + + childLabel + + ". Actual count: " + + children.size()); } else { return Optional.of(children.get(0)); } @@ -56,9 +60,10 @@ public static List getChildren(GraphTraversalSource g, Vertex vertex) { .toList(); } - public static List getChildren(GraphTraversalSource g, Vertex vertex, String childLabel) { - return g.V(vertex).out(Schema.CHILD).hasLabel(childLabel).toList(); - } + public static List getChildren( + GraphTraversalSource g, Vertex vertex, String childLabel) { + return g.V(vertex).out(Schema.CHILD).hasLabel(childLabel).toList(); + } public static Optional getPreviousSibling(GraphTraversalSource g, Vertex vertex) { Iterator it = g.V(vertex).in(Schema.NEXT_SIBLING); @@ -91,14 +96,14 @@ public static Optional getFirstChild(GraphTraversalSource g, Vertex vert } } - public static Optional getParent(GraphTraversalSource g, Vertex vertex) { - Iterator it = g.V(vertex).out(Schema.PARENT); - if (it.hasNext()) { - return Optional.of(it.next()); - } else { - return Optional.empty(); - } - } + public static Optional getParent(GraphTraversalSource g, Vertex vertex) { + Iterator it = g.V(vertex).out(Schema.PARENT); + if (it.hasNext()) { + return Optional.of(it.next()); + } else { + return Optional.empty(); + } + } private GremlinUtil() {} } diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java index 4de95cd58..56282b904 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java @@ -1,55 +1,51 @@ package com.salesforce.graph.build; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; -import org.apache.tinkerpop.gremlin.structure.Vertex; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; - import java.util.List; import java.util.Optional; import java.util.TreeSet; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; -/** - * Handles common operations performed while creating vertices on Graph - */ +/** Handles common operations performed while creating vertices on Graph */ public final class GremlinVertexUtil { - private static final Logger LOGGER = LogManager.getLogger(GremlinVertexUtil.class); - private GremlinVertexUtil(){} + private static final Logger LOGGER = LogManager.getLogger(GremlinVertexUtil.class); - /** - * Create parent-child relationship between vertices on graph - */ - static void addParentChildRelationship(GraphTraversalSource g, Vertex parentVertex, Vertex childVertex) { - // We are currently adding PARENT and CHILD, in theory the same edge could be navigated - // both ways, however - // the code looks messy when that is done. - // TODO: Determine if this causes performance or resource issues. Consider a single edge - g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate(); - g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate(); - } + private GremlinVertexUtil() {} - /** - * Make a synthetic vertex a sibling of an existing vertex on graph - */ - static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex syntheticVertex) { - // Get parent node of vertex - final Optional rootVertex = GremlinUtil.getParent(g, vertex); - if (rootVertex.isEmpty()) { - throw new UnexpectedException("Did not expect vertex to not have a parent vertex. vertex=" + vertex); - } + /** Create parent-child relationship between vertices on graph */ + static void addParentChildRelationship( + GraphTraversalSource g, Vertex parentVertex, Vertex childVertex) { + // We are currently adding PARENT and CHILD, in theory the same edge could be navigated + // both ways, however + // the code looks messy when that is done. + // TODO: Determine if this causes performance or resource issues. Consider a single edge + g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate(); + g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate(); + } - addParentChildRelationship(g, rootVertex.get(), syntheticVertex); - } + /** Make a synthetic vertex a sibling of an existing vertex on graph */ + static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex syntheticVertex) { + // Get parent node of vertex + final Optional rootVertex = GremlinUtil.getParent(g, vertex); + if (rootVertex.isEmpty()) { + throw new UnexpectedException( + "Did not expect vertex to not have a parent vertex. vertex=" + vertex); + } + + addParentChildRelationship(g, rootVertex.get(), syntheticVertex); + } - /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ + /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ protected static void addProperty( - TreeSet previouslyInsertedKeys, - GraphTraversal traversal, - String keyParam, - Object value) { + TreeSet previouslyInsertedKeys, + GraphTraversal traversal, + String keyParam, + Object value) { final String key = keyParam.intern(); if (!previouslyInsertedKeys.add(key)) { diff --git a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java index 33c53ae82..5d7ba6c86 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java +++ b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java @@ -5,8 +5,6 @@ import com.salesforce.exception.TodoException; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; -import com.salesforce.graph.ops.CloneUtil; -import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.vertex.SFVertexFactory; import java.util.ArrayList; import java.util.Arrays; @@ -24,7 +22,7 @@ /** Visits a method and draws all control flow edges. */ public class MethodPathBuilderVisitor { private static final Logger LOGGER = LogManager.getLogger(MethodPathBuilderVisitor.class); - /** Load the vertices as SFVertices and log more information. Will impact performance. */ + /** Load the vertices as SFVertices and log more information. Will impact performance. */ private static boolean SF_VERTEX_LOGGING = true; /** @@ -98,8 +96,7 @@ public static void apply(GraphTraversalSource g, Vertex methodVertex) { } } - - private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { + private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { try { String label = vertex.label(); diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java index 78ccbef2a..d21e97246 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -1,322 +1,340 @@ package com.salesforce.graph.build; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; -import org.apache.tinkerpop.gremlin.structure.Vertex; - import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.apex.jorje.JorjeNode; import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.ProgrammingException; import com.salesforce.graph.Schema; import com.salesforce.graph.ops.MethodUtil; - import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.TreeSet; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; /** * Handles creation of synthetic methods and vertices to gracefully invoke static code blocks. * + *

Consider this example: class StaticBlockClass { static { System.debug("inside static block + * 1"); } static { System.debug("inside static block 2"); } } In Jorje's compilation structure, + * static blocks are represented like this: class StaticBlockClass { private static void () { + * { System.debug("inside static block 1"); } { System.debug("inside static block 2"); } } } * - * Consider this example: - * class StaticBlockClass { - * static { - * System.debug("inside static block 1"); - * } - * static { - * System.debug("inside static block 2"); - * } - * } - * In Jorje's compilation structure, static blocks are represented like this: - * class StaticBlockClass { - * private static void () { - * { - * System.debug("inside static block 1"); - * } - * { - * System.debug("inside static block 2"); - * } - * } - * } - * - * Having multiple block statements inside a method breaks SFGE's makes handling code blocks in () - * impossible. As an alternative, we are creating synthetic vertices in the Graph to represent the static - * blocks as individual methods. We also create one top-level synthetic method ("StaticBlockInvoker") that invokes individual static - * block methods. While creating static scope for this class, we should invoke the method call expressions - * inside the top-level synthetic method. - * - * New structure looks like this: - * class StaticBlockClass { - * private static void SyntheticStaticBlock_1() { - * System.debug("inside static block 1"); - * } + *

Having multiple block statements inside a method breaks SFGE's makes handling code blocks in + * () impossible. As an alternative, we are creating synthetic vertices in the Graph to + * represent the static blocks as individual methods. We also create one top-level synthetic method + * ("StaticBlockInvoker") that invokes individual static block methods. While creating static scope + * for this class, we should invoke the method call expressions inside the top-level synthetic + * method. * - * private static void SyntheticStaticBlock_2() { - * System.debug("inside static block 2"); - * } + *

New structure looks like this: class StaticBlockClass { private static void + * SyntheticStaticBlock_1() { System.debug("inside static block 1"); } * - * private static void StaticBlockInvoker() { - * SyntheticStaticBlock_1(); - * SyntheticStaticBlock_2(); - * } - * } + *

private static void SyntheticStaticBlock_2() { System.debug("inside static block 2"); } * + *

private static void StaticBlockInvoker() { SyntheticStaticBlock_1(); SyntheticStaticBlock_2(); + * } } */ public final class StaticBlockUtil { - private static final Logger LOGGER = LogManager.getLogger(StaticBlockUtil.class); - - private static final String SYNTHETIC_STATIC_BLOCK_METHOD_NAME = "SyntheticStaticBlock_%d"; - private static final String STATIC_BLOCK_INVOKER_METHOD = "StaticBlockInvoker"; - - private StaticBlockUtil() {} - - /** - * Creates a synthetic method vertex to represent a static code block - * @param g traversal graph - * @param clintVertex () method's vertex - * @param blockStatementVertex static block originally placed by Jorje under () - * @param staticBlockIndex index to use for name uniqueness - - * TODO: this index is currently just the child count index and does not - * necessarily follow sequence - * @return new synthetic method vertex with name SyntheticStaticBlock_%d, which will be the parent - * for blockStatementVertex, and a sibling of () - */ - public static Vertex createSyntheticStaticBlockMethod(GraphTraversalSource g, Vertex clintVertex, Vertex blockStatementVertex, int staticBlockIndex) { - final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); - final String definingType = clintVertex.value(Schema.DEFINING_TYPE); - addSyntheticStaticBlockMethodProperties(g, definingType, syntheticMethodVertex, staticBlockIndex); - - final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); - addStaticModifierProperties(g, definingType, modifierNodeVertex); - GremlinVertexUtil.addParentChildRelationship(g, syntheticMethodVertex, modifierNodeVertex); - - GremlinVertexUtil.makeSiblings(g, clintVertex, syntheticMethodVertex); - - return syntheticMethodVertex; - } - - /** - * If rootNode contains synthetic static block methods (created through {@link #createSyntheticStaticBlockMethod}), - * adds a top-level synthetic method that invokes each static block method. - */ - public static void createSyntheticStaticBlockInvocation(GraphTraversalSource g, Vertex rootNode) { - // Check if root node contains any static block methods - final List staticBlockMethods = getStaticBlockMethods(g, rootNode); - - // Create static block invocation method - if (!staticBlockMethods.isEmpty()) { - final String definingType = rootNode.value(Schema.DEFINING_TYPE); - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Creating synthetic invocation method for {} to invoke {} static block(s)", definingType, staticBlockMethods.size()); - } - - final Vertex invokerMethodVertex = createSyntheticInvocationsMethod(g, rootNode, staticBlockMethods, definingType); - MethodPathBuilderVisitor.apply(g, invokerMethodVertex); - } - } - - static boolean isStaticBlockStatement(JorjeNode node, JorjeNode child) { - return isClintMethod(node) - && containsStaticBlock(node) - && isBlockStatement(child); - } - - private static Vertex createSyntheticInvocationsMethod(GraphTraversalSource g, Vertex rootNode, List staticBlockMethods, String definingType) { - // Create new synthetic method StaticBlockInvoker to invoke each synthetic static block method - final Vertex invokerMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); - addStaticBlockInvokerProperties(g, definingType, invokerMethodVertex); - GremlinVertexUtil.addParentChildRelationship(g, rootNode, invokerMethodVertex); - - final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); - addStaticModifierProperties(g, definingType, modifierNodeVertex); - GremlinVertexUtil.addParentChildRelationship(g, invokerMethodVertex, modifierNodeVertex); - - // Create synthetic BlockStatement inside StaticBlockInvoker to hold the method invocations - final Vertex blockStatementInInvoker = g.addV(ASTConstants.NodeType.BLOCK_STATEMENT).next(); - addBlockStatementProperties(g, definingType, blockStatementInInvoker); - GremlinVertexUtil.addParentChildRelationship(g, invokerMethodVertex, blockStatementInInvoker); - - for (int i = 0; i < staticBlockMethods.size(); i++) { - final Vertex staticBlockMethod = staticBlockMethods.get(i); - boolean isLastMethod = i == staticBlockMethods.size() - 1; - // Create a method call invocation to the synthetic static block method - final Vertex exprStaticBlockMethodCall = createMethodCallExpression(g, definingType, staticBlockMethod, isLastMethod); - - // Add the expression statement containing the method call inside StaticBlockInvoker's block statement - GremlinVertexUtil.addParentChildRelationship(g, blockStatementInInvoker, exprStaticBlockMethodCall); - } - - return invokerMethodVertex; - } - - private static Vertex createMethodCallExpression(GraphTraversalSource g, String definingType, Vertex staticBlockMethod, boolean isLastMethod) { - final Vertex expressionStmt = g.addV(ASTConstants.NodeType.EXPRESSION_STATEMENT).next(); - addExpressionStmtProperties(g, definingType, expressionStmt, isLastMethod); - - // Create new MethodCallExpression for the synthetic static block method - final Vertex staticBlockMethodCall = g.addV(ASTConstants.NodeType.METHOD_CALL_EXPRESSION).next(); - addMethodCallExpressionProperties(g, definingType, staticBlockMethod, staticBlockMethodCall); - GremlinVertexUtil.addParentChildRelationship(g, expressionStmt, staticBlockMethodCall); - - // Create EmptyReferenceExpression inside MethodCallExpression - final Vertex emptyMethodReference = g.addV(ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION).next(); - addEmptyMethodReferenceProperties(g, definingType, emptyMethodReference); - GremlinVertexUtil.addParentChildRelationship(g, staticBlockMethodCall, emptyMethodReference); - return expressionStmt; - } - - private static void addExpressionStmtProperties(GraphTraversalSource g, String definingType, Vertex expressionStmt, boolean isLastMethod) { - verifyType(expressionStmt, ASTConstants.NodeType.EXPRESSION_STATEMENT); - - final Map properties = new HashMap<>(); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.FIRST_CHILD, true); - properties.put(Schema.LAST_CHILD, true); - properties.put(Schema.CHILD_INDEX, 0); - - addProperties(g, expressionStmt, properties); - } - - private static void addEmptyMethodReferenceProperties(GraphTraversalSource g, String definingType, Vertex emptyMethodReference) { - verifyType(emptyMethodReference, ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION); - - final Map properties = new HashMap<>(); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.FIRST_CHILD, true); - properties.put(Schema.LAST_CHILD, true); - properties.put(Schema.CHILD_INDEX, 0); - - addProperties(g, emptyMethodReference, properties); - } - - private static void addMethodCallExpressionProperties(GraphTraversalSource g, String definingType, Vertex staticBlockMethod, Vertex staticBlockMethodCall) { - verifyType(staticBlockMethodCall, ASTConstants.NodeType.METHOD_CALL_EXPRESSION); - - final Map properties = new HashMap<>(); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.METHOD_NAME, staticBlockMethod.value(Schema.NAME)); - properties.put(Schema.FULL_METHOD_NAME, staticBlockMethod.value(Schema.NAME)); - properties.put(Schema.IS_STATIC_BLOCK_INVOCATION, true); - properties.put(Schema.FIRST_CHILD, true); - properties.put(Schema.LAST_CHILD, true); - properties.put(Schema.CHILD_INDEX, 0); - - addProperties(g, staticBlockMethodCall, properties); - } - - private static void addBlockStatementProperties(GraphTraversalSource g, String definingType, Vertex blockStatementInInvoker) { - verifyType(blockStatementInInvoker, ASTConstants.NodeType.BLOCK_STATEMENT); - - final Map properties = new HashMap<>(); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.CHILD_INDEX, 1); - properties.put(Schema.FIRST_CHILD, false); - properties.put(Schema.LAST_CHILD, true); - - addProperties(g, blockStatementInInvoker, properties); - } - - private static void addStaticModifierProperties(GraphTraversalSource g, String definingType, Vertex modifier) { - verifyType(modifier, ASTConstants.NodeType.MODIFIER_NODE); - - final Map properties = new HashMap<>(); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.STATIC, true); - properties.put(Schema.ABSTRACT, false); - properties.put(Schema.GLOBAL, false); - properties.put(Schema.MODIFIERS, 8); // Apparently, static methods have modifiers 8 - properties.put(Schema.FIRST_CHILD, true); - properties.put(Schema.LAST_CHILD, false); - properties.put(Schema.CHILD_INDEX, 0); - - addProperties(g, modifier, properties); - } - - private static void addStaticBlockInvokerProperties(GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex) { - verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); - final Map properties = new HashMap<>(); - properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); - properties.put(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true); - addCommonSynthMethodProperties(g, definingType, staticBlockInvokerVertex, properties); - } - - private static void addSyntheticStaticBlockMethodProperties(GraphTraversalSource g, String definingType, Vertex syntheticMethodVertex, int staticBlockIndex) { - verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); - final Map properties = new HashMap<>(); - properties.put(Schema.NAME, String.format(SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); - properties.put(Schema.IS_STATIC_BLOCK_METHOD, true); - - addCommonSynthMethodProperties(g, definingType, syntheticMethodVertex, properties); - } - - private static void addCommonSynthMethodProperties(GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex, Map properties) { - properties.put(Schema.ARITY, 0); - properties.put(Schema.CONSTRUCTOR, false); - properties.put(Schema.DEFINING_TYPE, definingType); - properties.put(Schema.IS_SYNTHETIC, true); - properties.put(Schema.RETURN_TYPE, ASTConstants.TYPE_VOID); - - - addProperties(g, staticBlockInvokerVertex, properties); - } - - private static void addProperties(GraphTraversalSource g, Vertex vertex, Map properties) { - final TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); - final GraphTraversal traversal = g.V(vertex.id()); - - for (Map.Entry entry : properties.entrySet()) { - GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); - previouslyInsertedKeys.add(entry.getKey()); - } - - // Commit the changes. - traversal.next(); - } - - /** - * @return true if given node represents () - */ - private static boolean isClintMethod(JorjeNode node) { - return ASTConstants.NodeType.METHOD.equals(node.getLabel()) - && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals(node.getProperties().get(Schema.NAME)); - } - - /** - * @return true if () node contains any static block definitions - */ - private static boolean containsStaticBlock(JorjeNode node) { - for (JorjeNode childNode : node.getChildren()) { - if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { - return true; - } - } - return false; - } - - private static List getStaticBlockMethods(GraphTraversalSource g, Vertex root) { - return g.V(root) - .out(Schema.CHILD) - .hasLabel(ASTConstants.NodeType.METHOD) - .has(Schema.IS_STATIC_BLOCK_METHOD, true) - .toList(); - } - - private static boolean isBlockStatement(JorjeNode child) { - return ASTConstants.NodeType.BLOCK_STATEMENT.equals(child.getLabel()); - } - - private static void verifyType(Vertex vertex, String expectedType) { - if (! expectedType.equals(vertex.label())) { - throw new ProgrammingException("Incorrect vertex type: " + vertex.label()); - } - } + private static final Logger LOGGER = LogManager.getLogger(StaticBlockUtil.class); + + static final String SYNTHETIC_STATIC_BLOCK_METHOD_NAME = "SyntheticStaticBlock_%d"; + static final String STATIC_BLOCK_INVOKER_METHOD = "StaticBlockInvoker"; + + private StaticBlockUtil() {} + + /** + * Creates a synthetic method vertex to represent a static code block + * + * @param g traversal graph + * @param clintVertex () method's vertex + * @param blockStatementVertex static block originally placed by Jorje under () + * @param staticBlockIndex index to use for name uniqueness - TODO: this index is currently just + * the child count index and does not necessarily follow sequence + * @return new synthetic method vertex with name SyntheticStaticBlock_%d, which will be the + * parent for blockStatementVertex, and a sibling of () + */ + public static Vertex createSyntheticStaticBlockMethod( + GraphTraversalSource g, + Vertex clintVertex, + Vertex blockStatementVertex, + int staticBlockIndex) { + final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); + final String definingType = clintVertex.value(Schema.DEFINING_TYPE); + addSyntheticStaticBlockMethodProperties( + g, definingType, syntheticMethodVertex, staticBlockIndex); + + final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); + addStaticModifierProperties(g, definingType, modifierNodeVertex); + GremlinVertexUtil.addParentChildRelationship(g, syntheticMethodVertex, modifierNodeVertex); + + GremlinVertexUtil.makeSiblings(g, clintVertex, syntheticMethodVertex); + + return syntheticMethodVertex; + } + + /** + * If rootNode contains synthetic static block methods (created through {@link + * #createSyntheticStaticBlockMethod}), adds a top-level synthetic method that invokes each + * static block method. + */ + public static void createSyntheticStaticBlockInvocation( + GraphTraversalSource g, Vertex rootNode) { + // Check if root node contains any static block methods + final List staticBlockMethods = getStaticBlockMethods(g, rootNode); + + // Create static block invocation method + if (!staticBlockMethods.isEmpty()) { + final String definingType = rootNode.value(Schema.DEFINING_TYPE); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "Creating synthetic invocation method for {} to invoke {} static block(s)", + definingType, + staticBlockMethods.size()); + } + + final Vertex invokerMethodVertex = + createSyntheticInvocationsMethod(g, rootNode, staticBlockMethods, definingType); + MethodPathBuilderVisitor.apply(g, invokerMethodVertex); + } + } + + static boolean isStaticBlockStatement(JorjeNode node, JorjeNode child) { + return isClintMethod(node) && containsStaticBlock(node) && isBlockStatement(child); + } + + private static Vertex createSyntheticInvocationsMethod( + GraphTraversalSource g, + Vertex rootNode, + List staticBlockMethods, + String definingType) { + // Create new synthetic method StaticBlockInvoker to invoke each synthetic static block + // method + final Vertex invokerMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); + addStaticBlockInvokerProperties(g, definingType, invokerMethodVertex); + GremlinVertexUtil.addParentChildRelationship(g, rootNode, invokerMethodVertex); + + final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); + addStaticModifierProperties(g, definingType, modifierNodeVertex); + GremlinVertexUtil.addParentChildRelationship(g, invokerMethodVertex, modifierNodeVertex); + + // Create synthetic BlockStatement inside StaticBlockInvoker to hold the method invocations + final Vertex blockStatementInInvoker = g.addV(ASTConstants.NodeType.BLOCK_STATEMENT).next(); + addBlockStatementProperties(g, definingType, blockStatementInInvoker); + GremlinVertexUtil.addParentChildRelationship( + g, invokerMethodVertex, blockStatementInInvoker); + + for (int i = 0; i < staticBlockMethods.size(); i++) { + final Vertex staticBlockMethod = staticBlockMethods.get(i); + boolean isLastMethod = i == staticBlockMethods.size() - 1; + // Create a method call invocation to the synthetic static block method + final Vertex exprStaticBlockMethodCall = + createMethodCallExpression(g, definingType, staticBlockMethod, isLastMethod); + + // Add the expression statement containing the method call inside StaticBlockInvoker's + // block statement + GremlinVertexUtil.addParentChildRelationship( + g, blockStatementInInvoker, exprStaticBlockMethodCall); + } + + return invokerMethodVertex; + } + + private static Vertex createMethodCallExpression( + GraphTraversalSource g, + String definingType, + Vertex staticBlockMethod, + boolean isLastMethod) { + final Vertex expressionStmt = g.addV(ASTConstants.NodeType.EXPRESSION_STATEMENT).next(); + addExpressionStmtProperties(g, definingType, expressionStmt, isLastMethod); + + // Create new MethodCallExpression for the synthetic static block method + final Vertex staticBlockMethodCall = + g.addV(ASTConstants.NodeType.METHOD_CALL_EXPRESSION).next(); + addMethodCallExpressionProperties( + g, definingType, staticBlockMethod, staticBlockMethodCall); + GremlinVertexUtil.addParentChildRelationship(g, expressionStmt, staticBlockMethodCall); + + // Create EmptyReferenceExpression inside MethodCallExpression + final Vertex emptyMethodReference = + g.addV(ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION).next(); + addEmptyMethodReferenceProperties(g, definingType, emptyMethodReference); + GremlinVertexUtil.addParentChildRelationship( + g, staticBlockMethodCall, emptyMethodReference); + return expressionStmt; + } + + private static void addExpressionStmtProperties( + GraphTraversalSource g, + String definingType, + Vertex expressionStmt, + boolean isLastMethod) { + verifyType(expressionStmt, ASTConstants.NodeType.EXPRESSION_STATEMENT); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, expressionStmt, properties); + } + + private static void addEmptyMethodReferenceProperties( + GraphTraversalSource g, String definingType, Vertex emptyMethodReference) { + verifyType(emptyMethodReference, ASTConstants.NodeType.EMPTY_REFERENCE_EXPRESSION); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, emptyMethodReference, properties); + } + + private static void addMethodCallExpressionProperties( + GraphTraversalSource g, + String definingType, + Vertex staticBlockMethod, + Vertex staticBlockMethodCall) { + verifyType(staticBlockMethodCall, ASTConstants.NodeType.METHOD_CALL_EXPRESSION); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.METHOD_NAME, staticBlockMethod.value(Schema.NAME)); + properties.put(Schema.FULL_METHOD_NAME, staticBlockMethod.value(Schema.NAME)); + properties.put(Schema.IS_STATIC_BLOCK_INVOCATION, true); + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, true); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, staticBlockMethodCall, properties); + } + + private static void addBlockStatementProperties( + GraphTraversalSource g, String definingType, Vertex blockStatementInInvoker) { + verifyType(blockStatementInInvoker, ASTConstants.NodeType.BLOCK_STATEMENT); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.CHILD_INDEX, 1); + properties.put(Schema.FIRST_CHILD, false); + properties.put(Schema.LAST_CHILD, true); + + addProperties(g, blockStatementInInvoker, properties); + } + + private static void addStaticModifierProperties( + GraphTraversalSource g, String definingType, Vertex modifier) { + verifyType(modifier, ASTConstants.NodeType.MODIFIER_NODE); + + final Map properties = new HashMap<>(); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.STATIC, true); + properties.put(Schema.ABSTRACT, false); + properties.put(Schema.GLOBAL, false); + properties.put(Schema.MODIFIERS, 8); // Apparently, static methods have modifiers 8 + properties.put(Schema.FIRST_CHILD, true); + properties.put(Schema.LAST_CHILD, false); + properties.put(Schema.CHILD_INDEX, 0); + + addProperties(g, modifier, properties); + } + + private static void addStaticBlockInvokerProperties( + GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex) { + verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); + final Map properties = new HashMap<>(); + properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); + properties.put(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true); + addCommonSynthMethodProperties(g, definingType, staticBlockInvokerVertex, properties); + } + + private static void addSyntheticStaticBlockMethodProperties( + GraphTraversalSource g, + String definingType, + Vertex syntheticMethodVertex, + int staticBlockIndex) { + verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); + final Map properties = new HashMap<>(); + properties.put( + Schema.NAME, String.format(SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); + properties.put(Schema.IS_STATIC_BLOCK_METHOD, true); + + addCommonSynthMethodProperties(g, definingType, syntheticMethodVertex, properties); + } + + private static void addCommonSynthMethodProperties( + GraphTraversalSource g, + String definingType, + Vertex staticBlockInvokerVertex, + Map properties) { + properties.put(Schema.ARITY, 0); + properties.put(Schema.CONSTRUCTOR, false); + properties.put(Schema.DEFINING_TYPE, definingType); + properties.put(Schema.IS_SYNTHETIC, true); + properties.put(Schema.RETURN_TYPE, ASTConstants.TYPE_VOID); + + addProperties(g, staticBlockInvokerVertex, properties); + } + + private static void addProperties( + GraphTraversalSource g, Vertex vertex, Map properties) { + final TreeSet previouslyInsertedKeys = CollectionUtil.newTreeSet(); + final GraphTraversal traversal = g.V(vertex.id()); + + for (Map.Entry entry : properties.entrySet()) { + GremlinVertexUtil.addProperty( + previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue()); + previouslyInsertedKeys.add(entry.getKey()); + } + + // Commit the changes. + traversal.next(); + } + + /** @return true if given node represents () */ + private static boolean isClintMethod(JorjeNode node) { + return ASTConstants.NodeType.METHOD.equals(node.getLabel()) + && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals( + node.getProperties().get(Schema.NAME)); + } + + /** @return true if () node contains any static block definitions */ + private static boolean containsStaticBlock(JorjeNode node) { + for (JorjeNode childNode : node.getChildren()) { + if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { + return true; + } + } + return false; + } + + private static List getStaticBlockMethods(GraphTraversalSource g, Vertex root) { + return g.V(root) + .out(Schema.CHILD) + .hasLabel(ASTConstants.NodeType.METHOD) + .has(Schema.IS_STATIC_BLOCK_METHOD, true) + .toList(); + } + + private static boolean isBlockStatement(JorjeNode child) { + return ASTConstants.NodeType.BLOCK_STATEMENT.equals(child.getLabel()); + } + + private static void verifyType(Vertex vertex, String expectedType) { + if (!expectedType.equals(vertex.label())) { + throw new ProgrammingException("Incorrect vertex type: " + vertex.label()); + } + } } 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 bcc92ee77..9632bb1d6 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -84,7 +84,7 @@ public final class MethodUtil { public static final String INSTANCE_CONSTRUCTOR_CANONICAL_NAME = ""; public static final String STATIC_CONSTRUCTOR_CANONICAL_NAME = ""; - public static List getTargetedMethods( + public static List getTargetedMethods( GraphTraversalSource g, List targets) { // The targets passed into this method should exclusively be ones that target specific // methods instead of files. diff --git a/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpander.java b/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpander.java index af23a3009..f1dbb9665 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpander.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpander.java @@ -150,6 +150,12 @@ final class ApexPathExpander implements ClassStaticScopeProvider, EngineDirectiv */ private final TreeSet currentlyInitializingStaticClasses; + /** + * Track classes whose static scopes have been initialized. This would help double visiting some + * nodes. + */ + private final TreeSet alreadyInitializedStaticClasses; + /** The symbol provider that corresponds to the {@link #topMostPath} */ private /*finalTODO Add clear method to SymbolProviderVertexVisitor*/ SymbolProviderVertexVisitor symbolProviderVisitor; @@ -195,6 +201,7 @@ final class ApexPathExpander implements ClassStaticScopeProvider, EngineDirectiv this.classStaticScopes = CollectionUtil.newTreeMap(); this.engineDirectiveContext = new EngineDirectiveContext(); this.currentlyInitializingStaticClasses = CollectionUtil.newTreeSet(); + this.alreadyInitializedStaticClasses = CollectionUtil.newTreeSet(); } /** @@ -272,6 +279,8 @@ final class ApexPathExpander implements ClassStaticScopeProvider, EngineDirectiv this.startScope = (SymbolProvider) CloneUtil.clone((DeepCloneable) other.startScope); this.currentlyInitializingStaticClasses = CloneUtil.cloneTreeSet(other.currentlyInitializingStaticClasses); + this.alreadyInitializedStaticClasses = + CloneUtil.cloneTreeSet(other.alreadyInitializedStaticClasses); } /** @@ -370,15 +379,18 @@ void initializeClassStaticScope(String className) apexPath = getTopMostPath().getStaticInitializationPath(className).get(); } - if (!classStaticScope.getState().equals(AbstractClassScope.State.INITIALIZED)) { + if (!classStaticScope.getState().equals(AbstractClassScope.State.INITIALIZED) + && !alreadyInitializedStaticClasses.contains(fullClassName)) { if (apexPath != null && apexPath.getCollectible() != null) { visit(apexPath.getCollectible()); } symbolProviderVisitor.popScope(classStaticScope); classStaticScope.setState(AbstractClassScope.State.INITIALIZED); + alreadyInitializedStaticClasses.add(fullClassName); } } finally { - if (!currentlyInitializingStaticClasses.remove(fullClassName)) { + if (!currentlyInitializingStaticClasses.remove(fullClassName) + && alreadyInitializedStaticClasses.contains(fullClassName)) { throw new ProgrammingException( "Set did not contain class. values=" + currentlyInitializingStaticClasses); diff --git a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java index 036857020..df046f8bf 100644 --- a/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java +++ b/sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java @@ -5,6 +5,7 @@ import com.salesforce.Collectible; import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.collections.CollectionUtil; +import com.salesforce.exception.ProgrammingException; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.ApexPath; import com.salesforce.graph.DeepCloneable; @@ -131,8 +132,6 @@ private static List getStaticBlocks( .out(Schema.CHILD) .hasLabel(NodeType.METHOD) .has(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true) - .order(Scope.global) - .by(Schema.CHILD_INDEX, Order.asc) .out(Schema.CHILD) .hasLabel(ASTConstants.NodeType.BLOCK_STATEMENT) .order(Scope.global) @@ -161,6 +160,9 @@ private static List getStaticBlocks( public static Optional getInitializationPath( GraphTraversalSource g, String classname) { ClassStaticScope classStaticScope = ClassStaticScope.get(g, classname); + if (classStaticScope.getState().equals(State.INITIALIZED)) { + throw new ProgrammingException("Initialization path does not need to be invoked on a class that's already initialized: " + classStaticScope.getClassName()); + } List vertices = new ArrayList<>(); vertices.addAll(classStaticScope.getFields()); vertices.addAll(getFieldDeclarations(g, classStaticScope.userClass)); @@ -174,13 +176,4 @@ public static Optional getInitializationPath( } } -// private static MethodCallExpressionVertex createSyntheticInvocation(MethodVertex.StaticBlockVertex methodVertex) { -// final HashMap map = new HashMap<>(); -// map.put(T.id, Long.valueOf(-1)); -// map.put(T.label, NodeType.METHOD_CALL_EXPRESSION); -// map.put(Schema.METHOD_NAME, methodVertex.getName()); -// map.put(Schema.DEFINING_TYPE, methodVertex.getDefiningType()); -// map.put(Schema.STATIC, Boolean.valueOf(true)); -// return new MethodCallExpressionVertex(map); -// } } diff --git a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java index 9c203c6d7..958d25b00 100644 --- a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java +++ b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java @@ -4,7 +4,6 @@ import com.salesforce.NullCollectible; import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.graph.Schema; -import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.symbols.SymbolProvider; import com.salesforce.graph.symbols.SymbolProviderVertexVisitor; import com.salesforce.graph.visitor.PathVertexVisitor; @@ -138,40 +137,41 @@ public ConstructorVertex getCollectible() { } } - public static final class StaticBlockVertex extends MethodVertex implements Collectible { - public static final NullCollectible NULL_VALUE = - new NullCollectible<>(StaticBlockVertex.class); + public static final class StaticBlockVertex extends MethodVertex + implements Collectible { + public static final NullCollectible NULL_VALUE = + new NullCollectible<>(StaticBlockVertex.class); - private StaticBlockVertex(Map properties) { - super(properties); - } + private StaticBlockVertex(Map properties) { + super(properties); + } - @Override - public boolean visit(PathVertexVisitor visitor, SymbolProvider symbols) { - return visitor.visit(this, symbols); - } + @Override + public boolean visit(PathVertexVisitor visitor, SymbolProvider symbols) { + return visitor.visit(this, symbols); + } - @Override - public boolean visit(SymbolProviderVertexVisitor visitor) { - return visitor.visit(this); - } + @Override + public boolean visit(SymbolProviderVertexVisitor visitor) { + return visitor.visit(this); + } - @Override - public void afterVisit(PathVertexVisitor visitor, SymbolProvider symbols) { - visitor.afterVisit(this, symbols); - } + @Override + public void afterVisit(PathVertexVisitor visitor, SymbolProvider symbols) { + visitor.afterVisit(this, symbols); + } - @Override - public void afterVisit(SymbolProviderVertexVisitor visitor) { - visitor.afterVisit(this); - } + @Override + public void afterVisit(SymbolProviderVertexVisitor visitor) { + visitor.afterVisit(this); + } - @Nullable - @Override - public StaticBlockVertex getCollectible() { - return this; - } - } + @Nullable + @Override + public StaticBlockVertex getCollectible() { + return this; + } + } public static final class InstanceMethodVertex extends MethodVertex { private InstanceMethodVertex(Map properties) { @@ -201,14 +201,15 @@ public void afterVisit(SymbolProviderVertexVisitor visitor) { public static final class Builder { public static MethodVertex create(Map vertex) { - final boolean isStaticBlockMethod = BaseSFVertex.toBoolean(vertex.get(Schema.IS_STATIC_BLOCK_METHOD)); + final boolean isStaticBlockMethod = + BaseSFVertex.toBoolean(vertex.get(Schema.IS_STATIC_BLOCK_METHOD)); boolean isConstructor = BaseSFVertex.toBoolean(vertex.get(Schema.CONSTRUCTOR)); - if (isStaticBlockMethod) { - return new StaticBlockVertex(vertex); - } else if (isConstructor) { - return new ConstructorVertex(vertex); - } - return new InstanceMethodVertex(vertex); + if (isStaticBlockMethod) { + return new StaticBlockVertex(vertex); + } else if (isConstructor) { + return new ConstructorVertex(vertex); + } + return new InstanceMethodVertex(vertex); } } } diff --git a/sfge/src/test/java/com/salesforce/graph/build/GraphBuildTestUtil.java b/sfge/src/test/java/com/salesforce/graph/build/GraphBuildTestUtil.java new file mode 100644 index 000000000..07cc97edf --- /dev/null +++ b/sfge/src/test/java/com/salesforce/graph/build/GraphBuildTestUtil.java @@ -0,0 +1,61 @@ +package com.salesforce.graph.build; + +import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has; + +import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.apex.jorje.JorjeUtil; +import com.salesforce.graph.ApexPath; +import com.salesforce.graph.Schema; +import com.salesforce.graph.cache.VertexCacheProvider; +import com.salesforce.graph.ops.ApexPathUtil; +import com.salesforce.graph.symbols.DefaultSymbolProviderVertexVisitor; +import com.salesforce.graph.vertex.MethodVertex; +import com.salesforce.graph.vertex.SFVertexFactory; +import com.salesforce.graph.visitor.ApexPathWalker; +import com.salesforce.graph.visitor.DefaultNoOpPathVertexVisitor; +import com.salesforce.graph.visitor.PathVertexVisitor; +import java.util.ArrayList; +import java.util.List; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; + +public class GraphBuildTestUtil { + static void buildGraph(GraphTraversalSource g, String sourceCode) { + buildGraph(g, new String[] {sourceCode}); + } + + static void buildGraph(GraphTraversalSource g, String[] sourceCodes) { + List compilations = new ArrayList(); + for (int i = 0; i < sourceCodes.length; i++) { + compilations.add( + new Util.CompilationDescriptor( + "TestCode" + i, JorjeUtil.compileApexFromString(sourceCodes[i]))); + } + + VertexCacheProvider.get().initialize(g); + CustomerApexVertexBuilder customerApexVertexBuilder = + new CustomerApexVertexBuilder(g, compilations); + + for (GraphBuilder graphBuilder : new GraphBuilder[] {customerApexVertexBuilder}) { + graphBuilder.build(); + } + } + + /** Sanity method to walk all paths. Helps to ensure all of the push/pops are correct */ + static List walkAllPaths(GraphTraversalSource g, String methodName) { + MethodVertex methodVertex = + SFVertexFactory.load( + g, + g.V().hasLabel(ASTConstants.NodeType.METHOD) + .has(Schema.NAME, methodName) + .not(has(Schema.IS_STANDARD, true))); + List paths = ApexPathUtil.getForwardPaths(g, methodVertex); + + for (ApexPath path : paths) { + DefaultSymbolProviderVertexVisitor symbols = new DefaultSymbolProviderVertexVisitor(g); + PathVertexVisitor visitor = new DefaultNoOpPathVertexVisitor(); + ApexPathWalker.walkPath(g, path, visitor, symbols); + } + + return paths; + } +} diff --git a/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java index 49698f5da..ddef8934d 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java @@ -13,12 +13,9 @@ import com.salesforce.TestUtil; import com.salesforce.apex.jorje.ASTConstants.NodeType; -import com.salesforce.apex.jorje.JorjeUtil; import com.salesforce.graph.ApexPath; import com.salesforce.graph.Schema; -import com.salesforce.graph.cache.VertexCacheProvider; import com.salesforce.graph.ops.ApexPathUtil; -import com.salesforce.graph.symbols.DefaultSymbolProviderVertexVisitor; import com.salesforce.graph.vertex.BaseSFVertex; import com.salesforce.graph.vertex.BlockStatementVertex; import com.salesforce.graph.vertex.CatchBlockStatementVertex; @@ -39,9 +36,6 @@ import com.salesforce.graph.vertex.ValueWhenBlockVertex; import com.salesforce.graph.vertex.VariableDeclarationStatementsVertex; import com.salesforce.graph.vertex.VariableExpressionVertex; -import com.salesforce.graph.visitor.ApexPathWalker; -import com.salesforce.graph.visitor.DefaultNoOpPathVertexVisitor; -import com.salesforce.graph.visitor.PathVertexVisitor; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -57,7 +51,7 @@ import org.junit.jupiter.api.Test; public class MethodPathBuilderTest { - private GraphTraversalSource g; + protected GraphTraversalSource g; private static final String[] BLOCK = new String[] {NodeType.BLOCK_STATEMENT}; @@ -129,7 +123,7 @@ public void testMethodWithSingleExpression() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -159,7 +153,7 @@ public void testMethodWithSingleExpression() { MatcherAssert.assertThat(getVerticesWithEndScope(), hasSize(equalTo(1))); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 3); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -175,7 +169,7 @@ public void testMethodWithNestedIfs() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // // // @@ -437,7 +431,7 @@ public void testMethodWithIfStatement() { // Implicit else assertEndScopes(BLOCK_IF_BLOCK, BlockStatementVertex.class, 3, 3); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -453,7 +447,7 @@ public void testMethodWithSingleIfElseStatement() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -542,7 +536,7 @@ public void testMethodWithSingleIfElseStatement() { // System.debug('GoodBye'); assertEndScopes(BLOCK_IF_BLOCK, ExpressionStatementVertex.class, 6); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -560,7 +554,7 @@ public void testMethodWithSingleIf_ElseIf_ElseStatement() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -692,7 +686,7 @@ public void testMethodWithSingleIf_ElseIf_ElseStatement() { // System.debug('GoodBye'); assertEndScopes(BLOCK_IF_BLOCK, ExpressionStatementVertex.class, 8); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -712,7 +706,7 @@ public void testMethodWithNestedIfElses() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -819,7 +813,7 @@ public void testMethodWithNestedIfElses() { // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK, ExpressionStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -835,7 +829,7 @@ public void testMethodWithExpressionBeforeAndAfterIf() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -946,7 +940,7 @@ public void testMethodWithExpressionBeforeAndAfterIf() { // System.debug('After'); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 7); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -965,7 +959,7 @@ public void testMethodWithInnerIfExpressionBeforeAndAfterIf() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -1114,7 +1108,7 @@ public void testMethodWithInnerIfExpressionBeforeAndAfterIf() { // System.debug('After'); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -1133,7 +1127,7 @@ public void testMethodWithForEach() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (9 edges) BlockStatement->VariableDeclarationStatements->ForEachStatement // @@ -1185,7 +1179,7 @@ public void testMethodWithForEach() { // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK_FOREACH_BLOCK, ExpressionStatementVertex.class, 8); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -1205,7 +1199,7 @@ public void testMethodWithForLoop() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (10 edges) BlockStatement->VariableDeclarationStatements->ForLoopStatement // @@ -1258,7 +1252,7 @@ public void testMethodWithForLoop() { // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK_FORLOOP_BLOCK, ExpressionStatementVertex.class, 9); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } /** @@ -1283,7 +1277,7 @@ public void testMethodWithForLoopNoInitializerOrStandardConditionOrIncrementer() + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (8 edges) BlockStatement->VariableDeclarationStatements->ForLoopStatement // ->BlockStatement @@ -1335,7 +1329,7 @@ public void testMethodWithForLoopNoInitializerOrStandardConditionOrIncrementer() // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK_FORLOOP_BLOCK, ExpressionStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } /** Tests the case where the for loop doesn't contain an initializer. */ @@ -1357,7 +1351,7 @@ public void testMethodWithForLoopNoInitializer() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (10 edges) // BlockStatement->VariableDeclarationStatements->ForLoopStatement->StandardCondition->PostfixExpression @@ -1410,7 +1404,7 @@ public void testMethodWithForLoopNoInitializer() { // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK_FORLOOP_BLOCK, ExpressionStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } /** @@ -1435,7 +1429,7 @@ public void testMethodWithForLoopNoInitializerOrStandardCondition() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (9 edges) // BlockStatement->VariableDeclarationStatements->ForLoopStatement->PostfixExpression @@ -1488,7 +1482,7 @@ public void testMethodWithForLoopNoInitializerOrStandardCondition() { // System.debug('Not Logged'); assertEndScopes(BLOCK_IF_BLOCK_FORLOOP_BLOCK, ExpressionStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -1509,7 +1503,7 @@ public void testMethodWithForLoopExpressionBeforeAndAfter() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (10 edges) BlockStatement->VariableDeclarationStatements->ForLoopStatement // @@ -1591,7 +1585,7 @@ public void testMethodWithForLoopExpressionBeforeAndAfter() { // System.debug('After'); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 12); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -1613,7 +1607,7 @@ public void testMethodWithForLoopExpressionBeforeAndAfterEndsForScope() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); MatcherAssert.assertThat(getVerticesWithEndScope(), hasSize(equalTo(4))); @@ -1629,7 +1623,7 @@ public void testMethodWithForLoopExpressionBeforeAndAfterEndsForScope() { // System.debug('After'); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 13); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -1644,7 +1638,7 @@ public void testMethodWithEarlyReturn() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // // @@ -2046,7 +2040,7 @@ public void testMultipleEarlyReturns() { // insert assertEndScopes(BLOCK, DmlInsertStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -2066,7 +2060,7 @@ public void testInsertGuardedByExceptionInOtherMethodWithReturn() { + "}\n" }; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); List paths; MethodVertex methodVertex = TestUtil.getVertexOnLine(g, MethodVertex.class, 2); @@ -2092,7 +2086,7 @@ public void testInsertGuardedByExceptionInOtherMethodWithReturn() { // insert assertEndScopes(BLOCK, ThrowStatementVertex.class, 10); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -2108,7 +2102,7 @@ public void testMethodWithSingleTryCatch() { + " }\n" + "}"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // // @@ -2322,7 +2316,7 @@ public void testMethodWithSingleTryCatchAndExpressionBeforeAndAfter() { // System.debug('After'); assertEndScopes(BLOCK, ExpressionStatementVertex.class, 9); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -2339,7 +2333,7 @@ public void testIfWithMethodInStandardCondition() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // doSomething // @@ -2397,7 +2391,7 @@ public void testIfWithMethodInStandardCondition() { List edges = g.V().outE(Schema.CFG_PATH).toList(); MatcherAssert.assertThat(edges, hasSize(7)); - walkAllPaths("doSomething"); + GraphBuildTestUtil.walkAllPaths(g, "doSomething"); } @Test @@ -2413,7 +2407,7 @@ public void testWhileStatement() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); // (6 edges) // BlockStatement->VariableDeclarationStatements->WhileLoopStatement->StandardCondition->BlockStatement->ExpressionStatement->ExpressionStatement @@ -2424,7 +2418,7 @@ public void testWhileStatement() { ExpressionStatementVertex.class, 6); - List paths = walkAllPaths("doSomething"); + List paths = GraphBuildTestUtil.walkAllPaths(g, "doSomething"); MatcherAssert.assertThat(paths, hasSize(equalTo(1))); } @@ -2448,9 +2442,9 @@ public void testWhileStatementInOtherMethod() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); - List paths = walkAllPaths("doSomething"); + List paths = GraphBuildTestUtil.walkAllPaths(g, "doSomething"); MatcherAssert.assertThat(paths, hasSize(equalTo(1))); } @@ -2535,7 +2529,7 @@ public void testEnumSwitchStatement() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); BlockStatementVertex blockStatementLine2 = TestUtil.getVertexOnLine(g, BlockStatementVertex.class, 2); @@ -2611,7 +2605,7 @@ public void testEnumSwitchStatement() { Pair.of(elseWhenBlock, elseWhenBlockBlockStatement), Pair.of(elseWhenBlockBlockStatement, elseWhenBlockExpressionStatement)); - List paths = walkAllPaths("doSomething"); + List paths = GraphBuildTestUtil.walkAllPaths(g, "doSomething"); MatcherAssert.assertThat(paths, hasSize(equalTo(3))); } @@ -2635,7 +2629,7 @@ public void testIntegerSwitchStatement() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); + GraphBuildTestUtil.buildGraph(g, sourceCode); BlockStatementVertex blockStatementLine2 = TestUtil.getVertexOnLine(g, BlockStatementVertex.class, 2); @@ -2709,7 +2703,7 @@ public void testIntegerSwitchStatement() { Pair.of(elseWhenBlock, elseWhenBlockBlockStatement), Pair.of(elseWhenBlockBlockStatement, elseWhenBlockExpressionStatement)); - List paths = walkAllPaths("doSomething"); + List paths = GraphBuildTestUtil.walkAllPaths(g, "doSomething"); MatcherAssert.assertThat(paths, hasSize(equalTo(3))); } @@ -2736,8 +2730,8 @@ public void testMethodSwitchStatement() { + " }\n" + "}\n"; - buildGraph(g, sourceCode); - List paths = walkAllPaths("doSomething"); + GraphBuildTestUtil.buildGraph(g, sourceCode); + List paths = GraphBuildTestUtil.walkAllPaths(g, "doSomething"); // There are 3 paths since #walkPaths does not use any excluders MatcherAssert.assertThat(paths, hasSize(equalTo(3))); } @@ -2809,44 +2803,4 @@ private void assertEndScopes( hasSize(equalTo(endScopes.length))); MatcherAssert.assertThat(vertex.getEndScopes(), contains(endScopes)); } - - /** Sanity method to walk all paths. Helps to ensure all of the push/pops are correct */ - private List walkAllPaths(String methodName) { - MethodVertex methodVertex = - SFVertexFactory.load( - g, - g.V().hasLabel(NodeType.METHOD) - .has(Schema.NAME, methodName) - .not(has(Schema.IS_STANDARD, true))); - List paths = ApexPathUtil.getForwardPaths(g, methodVertex); - - for (ApexPath path : paths) { - DefaultSymbolProviderVertexVisitor symbols = new DefaultSymbolProviderVertexVisitor(g); - PathVertexVisitor visitor = new DefaultNoOpPathVertexVisitor(); - ApexPathWalker.walkPath(g, path, visitor, symbols); - } - - return paths; - } - - private static void buildGraph(GraphTraversalSource g, String sourceCode) { - buildGraph(g, new String[] {sourceCode}); - } - - private static void buildGraph(GraphTraversalSource g, String[] sourceCodes) { - List compilations = new ArrayList<>(); - for (int i = 0; i < sourceCodes.length; i++) { - compilations.add( - new Util.CompilationDescriptor( - "TestCode" + i, JorjeUtil.compileApexFromString(sourceCodes[i]))); - } - - VertexCacheProvider.get().initialize(g); - CustomerApexVertexBuilder customerApexVertexBuilder = - new CustomerApexVertexBuilder(g, compilations); - - for (GraphBuilder graphBuilder : new GraphBuilder[] {customerApexVertexBuilder}) { - graphBuilder.build(); - } - } } diff --git a/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java b/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java deleted file mode 100644 index e31367cbf..000000000 --- a/sfge/src/test/java/com/salesforce/graph/build/StaticAndAnonymousCodeBlockTest.java +++ /dev/null @@ -1,55 +0,0 @@ -package com.salesforce.graph.build; - -import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; -import org.junit.jupiter.api.Test; -import com.salesforce.TestRunner; -import com.salesforce.TestUtil; -import com.salesforce.graph.symbols.apex.ApexStringValue; -import com.salesforce.graph.symbols.apex.ApexValue; -import com.salesforce.graph.visitor.SystemDebugAccumulator; -import com.salesforce.matchers.TestRunnerMatcher; - -import java.util.List; -import java.util.Optional; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasSize; - -public class StaticAndAnonymousCodeBlockTest extends MethodPathBuilderTest { - @Test - public void testStaticBlock() { - String[] sourceCode = - { - "public class StaticBlockClass {\n" + - " static {\n" - + " System.debug('static block');\n" - + " }\n" - + " public static String foo() {\n" + - " return 'hello';\n" + - "}\n" - + "}", - "public class MyClass {\n" + - " public void doSomething() {\n" + - " StaticBlockClass sb = new StaticBlockClass();\n" + - " System.debug(sb.foo());\n" + - " }\n" + - "}" - }; - -// TestUtil.buildGraph(g, sourceCode); - - TestRunner.Result result = TestRunner.walkPath(g, - sourceCode); - SystemDebugAccumulator visitor = result.getVisitor(); - final List>> allResults = visitor.getAllResults(); - - assertThat(allResults, hasSize(2)); - - // verify that static block is invoked first - ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); - assertThat(stringValue.getValue().get(), equalTo("static block")); - - } -} diff --git a/sfge/src/test/java/com/salesforce/graph/build/StaticBlockUtilTest.java b/sfge/src/test/java/com/salesforce/graph/build/StaticBlockUtilTest.java new file mode 100644 index 000000000..075bb92b5 --- /dev/null +++ b/sfge/src/test/java/com/salesforce/graph/build/StaticBlockUtilTest.java @@ -0,0 +1,117 @@ +package com.salesforce.graph.build; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; + +import com.salesforce.TestUtil; +import com.salesforce.graph.ApexPath; +import com.salesforce.graph.vertex.BaseSFVertex; +import com.salesforce.graph.vertex.BlockStatementVertex; +import com.salesforce.graph.vertex.ExpressionStatementVertex; +import com.salesforce.graph.vertex.LiteralExpressionVertex; +import com.salesforce.graph.vertex.MethodCallExpressionVertex; +import java.util.List; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class StaticBlockUtilTest { + protected GraphTraversalSource g; + private static final String SYNTHETIC_STATIC_BLOCK_METHOD_1 = + String.format(StaticBlockUtil.SYNTHETIC_STATIC_BLOCK_METHOD_NAME, 1); + + @BeforeEach + public void setup() { + this.g = TestUtil.getGraph(); + } + + // does each static block have a corresponding synthetic method block? + @Test + public void testMethodForStaticBlock() { + String[] sourceCode = { + "public class MyClass {\n" + + " static {\n" + + " System.debug('inside static block 1');\n" + + "}\n" + + " static {\n" + + " System.debug('inside static block 2');\n" + + " }\n" + + "void doSomething() {\n" + + " System.debug('inside doSomething');\n" + + "}\n" + + "}" + }; + + GraphBuildTestUtil.buildGraph(g, sourceCode); + + verifyStaticBlockMethod( + String.format(StaticBlockUtil.SYNTHETIC_STATIC_BLOCK_METHOD_NAME, 1), + "inside static block 1"); + verifyStaticBlockMethod( + String.format(StaticBlockUtil.SYNTHETIC_STATIC_BLOCK_METHOD_NAME, 2), + "inside static block 2"); + } + + private void verifyStaticBlockMethod(String methodName, String printedString) { + final List staticBlockPaths = GraphBuildTestUtil.walkAllPaths(g, methodName); + assertThat(staticBlockPaths, hasSize(1)); + final ApexPath path = staticBlockPaths.get(0); + + // Make sure synthetic method is linked to the static block and its contents + final BlockStatementVertex blockStatementVertex = (BlockStatementVertex) path.firstVertex(); + final List blockStmtChildren = blockStatementVertex.getChildren(); + assertThat(blockStmtChildren, hasSize(1)); + + final ExpressionStatementVertex expressionStatementVertex = + (ExpressionStatementVertex) blockStmtChildren.get(0); + final List exprStmtChildren = expressionStatementVertex.getChildren(); + assertThat(exprStmtChildren, hasSize(1)); + + final MethodCallExpressionVertex methodCallExpressionVertex = + (MethodCallExpressionVertex) exprStmtChildren.get(0); + assertThat(methodCallExpressionVertex.getFullMethodName(), equalTo("System.debug")); + final LiteralExpressionVertex literalExpressionVertex = + (LiteralExpressionVertex) methodCallExpressionVertex.getParameters().get(0); + + assertThat(literalExpressionVertex.getLiteralAsString(), equalTo(printedString)); + } + + // does synthetic invoker method exist and contain the necessary parts and relationship? + @Test + public void testMethodInvokerForStaticBlock() { + String[] sourceCode = { + "public class MyClass {\n" + + " static {\n" + + " System.debug('inside static block');\n" + + "}\n" + + "void doSomething() {\n" + + " System.debug('inside doSomething');\n" + + "}\n" + + "}" + }; + + GraphBuildTestUtil.buildGraph(g, sourceCode); + + final List staticBlockPaths = + GraphBuildTestUtil.walkAllPaths(g, StaticBlockUtil.STATIC_BLOCK_INVOKER_METHOD); + assertThat(staticBlockPaths, hasSize(1)); + final ApexPath path = staticBlockPaths.get(0); + + // Make sure synthetic method is linked to the static block and its contents + final BlockStatementVertex blockStatementVertex = (BlockStatementVertex) path.firstVertex(); + final List blockStmtChildren = blockStatementVertex.getChildren(); + assertThat(blockStmtChildren, hasSize(1)); + + final ExpressionStatementVertex expressionStatementVertex = + (ExpressionStatementVertex) blockStmtChildren.get(0); + final List exprStmtChildren = expressionStatementVertex.getChildren(); + assertThat(exprStmtChildren, hasSize(1)); + + final MethodCallExpressionVertex methodCallExpressionVertex = + (MethodCallExpressionVertex) exprStmtChildren.get(0); + assertThat( + methodCallExpressionVertex.getFullMethodName(), + equalTo(SYNTHETIC_STATIC_BLOCK_METHOD_1)); + } +} diff --git a/sfge/src/test/java/com/salesforce/graph/symbols/StaticCodeBlockInvocationTest.java b/sfge/src/test/java/com/salesforce/graph/symbols/StaticCodeBlockInvocationTest.java new file mode 100644 index 000000000..914be6b5b --- /dev/null +++ b/sfge/src/test/java/com/salesforce/graph/symbols/StaticCodeBlockInvocationTest.java @@ -0,0 +1,300 @@ +package com.salesforce.graph.symbols; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.salesforce.TestRunner; +import com.salesforce.TestUtil; +import com.salesforce.graph.symbols.apex.ApexStringValue; +import com.salesforce.graph.symbols.apex.ApexValue; +import com.salesforce.graph.visitor.SystemDebugAccumulator; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +public class StaticCodeBlockInvocationTest { + protected GraphTraversalSource g; + + @BeforeEach + public void setup() { + this.g = TestUtil.getGraph(); + } + + @Test + public void testSingleStaticBlockFromStaticMethod() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(2)); + + // verify that static block is invoked first + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("static block")); + } + + @Test + public void testStaticBlockFromConstructor() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " StaticBlockClass sb = new StaticBlockClass();\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(1)); + + // verify that static block is invoked first + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("static block")); + } + + @Test + public void testNoStaticBlock() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + " public static String foo = 'hello';\n" + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo);\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(1)); + + // verify non-static-block case is handled correctly + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("hello")); + } + + @Test + @Disabled // TODO: static field defined along with static block should not be double-invoked + public void testSingleStaticBlockAndField() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " static String myStr = 'hello';\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(2)); + + // verify that static block is invoked first + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("static block")); + } + + @Test + @Disabled // TODO: static field defined along with static block should not be double-invoked + public void testSingleStaticBlockAndFieldInvokingAMethod() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " static String myStr = foo();\n" + + " public static String bar() {\n" + + " return 'bar';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.bar());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(2)); + + // verify that static block is invoked first + ApexStringValue stringValue = (ApexStringValue) allResults.get(0).get(); + assertThat(stringValue.getValue().get(), equalTo("static block")); + } + + @Test + public void testMultipleStaticBlocks() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block 1');\n" + + " }\n" + + " static {\n" + + " System.debug('static block 2');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + final List resultStrings = getResultStrings(allResults); + assertTrue(resultStrings.contains("static block 1")); + assertTrue(resultStrings.contains("static block 2")); + assertTrue(resultStrings.contains("hello")); + } + + @Test + @Disabled // TODO: fix issue where static block 2 is invoked twice instead of once + public void testEachStaticBlockIsInvokedOnlyOnce() { + String[] sourceCode = { + "public class StaticBlockClass {\n" + + " static {\n" + + " System.debug('static block 1');\n" + + " }\n" + + " static {\n" + + " System.debug('static block 2');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat( + allResults, + hasSize(3)); // TODO: this is currently returning 4 where static block 2 gets + // invoked twice + } + + @Test + public void testSuperClassStaticBlocks() { + String[] sourceCode = { + "public class SuperStaticBlockClass {\n" + + " static {\n" + + " System.debug('super static block');\n" + + " }\n" + + "}", + "public class StaticBlockClass extends SuperStaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + final List resultStrings = getResultStrings(allResults); + assertTrue(resultStrings.contains("super static block")); + assertTrue(resultStrings.contains("static block")); + assertTrue(resultStrings.contains("hello")); + } + + @Test + @Disabled // TODO: super class's static block should be invoked only once + public void testSuperClassStaticBlocksInvokedOnceEach() { + String[] sourceCode = { + "public class SuperStaticBlockClass {\n" + + " static {\n" + + " System.debug('super static block');\n" + + " }\n" + + "}", + "public class StaticBlockClass extends SuperStaticBlockClass {\n" + + " static {\n" + + " System.debug('static block');\n" + + " }\n" + + " public static String foo() {\n" + + " return 'hello';\n" + + "}\n" + + "}", + "public class MyClass {\n" + + " public void doSomething() {\n" + + " System.debug(StaticBlockClass.foo());\n" + + " }\n" + + "}" + }; + + TestRunner.Result result = TestRunner.walkPath(g, sourceCode); + SystemDebugAccumulator visitor = result.getVisitor(); + final List>> allResults = visitor.getAllResults(); + + assertThat(allResults, hasSize(3)); + } + + private List getResultStrings(List>> allResults) { + return allResults.stream() + .map(r -> ((ApexStringValue) r.get()).getValue().get()) + .collect(Collectors.toList()); + } +} From ecc2bdd026e25d75e20e17dfac3f88d63471868e Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 30 Jun 2022 12:01:36 -0700 Subject: [PATCH 14/34] Removing intermediary code now unused --- .../salesforce/graph/vertex/MethodVertex.java | 45 +------------------ 1 file changed, 1 insertion(+), 44 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java index 958d25b00..3f0b59fd9 100644 --- a/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java +++ b/sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java @@ -137,42 +137,6 @@ public ConstructorVertex getCollectible() { } } - public static final class StaticBlockVertex extends MethodVertex - implements Collectible { - public static final NullCollectible NULL_VALUE = - new NullCollectible<>(StaticBlockVertex.class); - - private StaticBlockVertex(Map properties) { - super(properties); - } - - @Override - public boolean visit(PathVertexVisitor visitor, SymbolProvider symbols) { - return visitor.visit(this, symbols); - } - - @Override - public boolean visit(SymbolProviderVertexVisitor visitor) { - return visitor.visit(this); - } - - @Override - public void afterVisit(PathVertexVisitor visitor, SymbolProvider symbols) { - visitor.afterVisit(this, symbols); - } - - @Override - public void afterVisit(SymbolProviderVertexVisitor visitor) { - visitor.afterVisit(this); - } - - @Nullable - @Override - public StaticBlockVertex getCollectible() { - return this; - } - } - public static final class InstanceMethodVertex extends MethodVertex { private InstanceMethodVertex(Map properties) { super(properties); @@ -201,15 +165,8 @@ public void afterVisit(SymbolProviderVertexVisitor visitor) { public static final class Builder { public static MethodVertex create(Map vertex) { - final boolean isStaticBlockMethod = - BaseSFVertex.toBoolean(vertex.get(Schema.IS_STATIC_BLOCK_METHOD)); boolean isConstructor = BaseSFVertex.toBoolean(vertex.get(Schema.CONSTRUCTOR)); - if (isStaticBlockMethod) { - return new StaticBlockVertex(vertex); - } else if (isConstructor) { - return new ConstructorVertex(vertex); - } - return new InstanceMethodVertex(vertex); + return isConstructor ? new ConstructorVertex(vertex) : new InstanceMethodVertex(vertex); } } } From a9017ba46f5318f1fe133b55b065417cbdbe5cbf Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 30 Jun 2022 15:04:02 -0700 Subject: [PATCH 15/34] Adding child index to synthetic methods --- .../build/AbstractApexVertexBuilder.java | 2 +- .../graph/build/GremlinVertexUtil.java | 9 ++++-- .../graph/build/StaticBlockUtil.java | 29 +++++++++++-------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index bdcef65c6..4b4003e10 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -85,7 +85,7 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { // Handle static block if needed if (StaticBlockUtil.isStaticBlockStatement(node, child)) { final Vertex parentVertexForChild = - StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, vChild, i); + StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, i); GremlinVertexUtil.addParentChildRelationship(g, parentVertexForChild, vChild); verticesAddressed.add(parentVertexForChild); } else { diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java index 56282b904..677595747 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java @@ -30,14 +30,19 @@ static void addParentChildRelationship( /** Make a synthetic vertex a sibling of an existing vertex on graph */ static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex syntheticVertex) { + final Vertex rootVertex = getParentVertex(g, vertex); + + addParentChildRelationship(g, rootVertex, syntheticVertex); + } + + static Vertex getParentVertex(GraphTraversalSource g, Vertex vertex) { // Get parent node of vertex final Optional rootVertex = GremlinUtil.getParent(g, vertex); if (rootVertex.isEmpty()) { throw new UnexpectedException( "Did not expect vertex to not have a parent vertex. vertex=" + vertex); } - - addParentChildRelationship(g, rootVertex.get(), syntheticVertex); + return rootVertex.get(); } /** Add a property to the traversal, throwing an exception if any keys are duplicated. */ diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java index d21e97246..d207146a6 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -52,21 +52,22 @@ private StaticBlockUtil() {} * * @param g traversal graph * @param clintVertex () method's vertex - * @param blockStatementVertex static block originally placed by Jorje under () * @param staticBlockIndex index to use for name uniqueness - TODO: this index is currently just * the child count index and does not necessarily follow sequence * @return new synthetic method vertex with name SyntheticStaticBlock_%d, which will be the * parent for blockStatementVertex, and a sibling of () */ public static Vertex createSyntheticStaticBlockMethod( - GraphTraversalSource g, - Vertex clintVertex, - Vertex blockStatementVertex, - int staticBlockIndex) { + GraphTraversalSource g, + Vertex clintVertex, + int staticBlockIndex) { final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); final String definingType = clintVertex.value(Schema.DEFINING_TYPE); + final List siblings = GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clintVertex)); + final int nextSiblingIndex = siblings.size(); + addSyntheticStaticBlockMethodProperties( - g, definingType, syntheticMethodVertex, staticBlockIndex); + g, definingType, syntheticMethodVertex, staticBlockIndex, nextSiblingIndex); final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); addStaticModifierProperties(g, definingType, modifierNodeVertex); @@ -114,8 +115,9 @@ private static Vertex createSyntheticInvocationsMethod( String definingType) { // Create new synthetic method StaticBlockInvoker to invoke each synthetic static block // method + final List siblings = GremlinUtil.getChildren(g, rootNode); final Vertex invokerMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); - addStaticBlockInvokerProperties(g, definingType, invokerMethodVertex); + addStaticBlockInvokerProperties(g, definingType, invokerMethodVertex, siblings.size()); GremlinVertexUtil.addParentChildRelationship(g, rootNode, invokerMethodVertex); final Vertex modifierNodeVertex = g.addV(ASTConstants.NodeType.MODIFIER_NODE).next(); @@ -252,24 +254,27 @@ private static void addStaticModifierProperties( } private static void addStaticBlockInvokerProperties( - GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex) { + GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex, int childIndex) { verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); properties.put(Schema.IS_STATIC_BLOCK_INVOKER_METHOD, true); + properties.put(Schema.CHILD_INDEX, childIndex); addCommonSynthMethodProperties(g, definingType, staticBlockInvokerVertex, properties); } private static void addSyntheticStaticBlockMethodProperties( - GraphTraversalSource g, - String definingType, - Vertex syntheticMethodVertex, - int staticBlockIndex) { + GraphTraversalSource g, + String definingType, + Vertex syntheticMethodVertex, + int staticBlockIndex, + int childIndex) { verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put( Schema.NAME, String.format(SYNTHETIC_STATIC_BLOCK_METHOD_NAME, staticBlockIndex)); properties.put(Schema.IS_STATIC_BLOCK_METHOD, true); + properties.put(Schema.CHILD_INDEX, childIndex); addCommonSynthMethodProperties(g, definingType, syntheticMethodVertex, properties); } From 89f7cd6f7507d50570c368c80beb4561b0ddf20c Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Tue, 5 Jul 2022 14:39:47 -0700 Subject: [PATCH 16/34] Attempting to remove compilation error in CircleCI --- .../main/java/com/salesforce/graph/build/GremlinVertexUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java index 677595747..3efb7f3c1 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java @@ -38,7 +38,7 @@ static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex synthetic static Vertex getParentVertex(GraphTraversalSource g, Vertex vertex) { // Get parent node of vertex final Optional rootVertex = GremlinUtil.getParent(g, vertex); - if (rootVertex.isEmpty()) { + if (!rootVertex.isPresent()) { throw new UnexpectedException( "Did not expect vertex to not have a parent vertex. vertex=" + vertex); } From d7fd3604b3c26c3f04113ee36db14481b793bb94 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Tue, 5 Jul 2022 16:19:13 -0700 Subject: [PATCH 17/34] Addressing PR feedback --- .../build/AbstractApexVertexBuilder.java | 12 ++- .../build/CustomerApexVertexBuilder.java | 1 + .../graph/build/StaticBlockUtil.java | 82 +++++++++++++------ 3 files changed, 66 insertions(+), 29 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index 4b4003e10..2cd405346 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -82,7 +82,9 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { final Vertex vChild = g.addV(child.getLabel()).next(); addProperties(g, child, vChild); - // Handle static block if needed + /** Handle static block if we are looking at a method that has a block statement. + * See {@linkplain StaticBlockUtil} on why this is needed + * and how we handle it. */ if (StaticBlockUtil.isStaticBlockStatement(node, child)) { final Vertex parentVertexForChild = StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, i); @@ -116,9 +118,9 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { */ private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vNode) { if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) { + // If we just added a method, create forward and + // backward code flow for the contents of the method MethodPathBuilderVisitor.apply(g, vNode); - } else if (ROOT_VERTICES.contains(node.getLabel())) { - StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode); } } @@ -128,7 +130,9 @@ private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vN * @param vNode root node that corresponds to the file */ protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) { - // Intentionally left blank + // If the root (class/trigger/etc) contained any static blocks, + // create an invoker method to invoke the static blocks + StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode); } protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) { diff --git a/sfge/src/main/java/com/salesforce/graph/build/CustomerApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/CustomerApexVertexBuilder.java index b7419fbcf..073695448 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/CustomerApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/CustomerApexVertexBuilder.java @@ -24,6 +24,7 @@ public void build() { @Override protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) { + super.afterFileInsert(g, vNode); ApexPropertyAnnotator.apply(g, vNode); } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java index d207146a6..3a2a8756f 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -19,25 +19,57 @@ /** * Handles creation of synthetic methods and vertices to gracefully invoke static code blocks. * - *

Consider this example: class StaticBlockClass { static { System.debug("inside static block - * 1"); } static { System.debug("inside static block 2"); } } In Jorje's compilation structure, - * static blocks are represented like this: class StaticBlockClass { private static void () { - * { System.debug("inside static block 1"); } { System.debug("inside static block 2"); } } } + *

Consider this example: * - *

Having multiple block statements inside a method breaks SFGE's makes handling code blocks in - * () impossible. As an alternative, we are creating synthetic vertices in the Graph to - * represent the static blocks as individual methods. We also create one top-level synthetic method - * ("StaticBlockInvoker") that invokes individual static block methods. While creating static scope - * for this class, we should invoke the method call expressions inside the top-level synthetic - * method. + * + * class StaticBlockClass { + * static { + * System.debug("inside static block 1"); + * } + * static { + * System.debug("inside static block 2"); + * } + * } + * * - *

New structure looks like this: class StaticBlockClass { private static void - * SyntheticStaticBlock_1() { System.debug("inside static block 1"); } + * In Jorje's compilation structure, static blocks are represented like this: + * + * class StaticBlockClass { + * private static void () { + * { + * System.debug("inside static block 1"); + * } + * { + * System.debug("inside static block 2"); + * } + * } + * } + * * - *

private static void SyntheticStaticBlock_2() { System.debug("inside static block 2"); } + *

Having multiple block statements inside a method breaks SFGE's normal code flow logic. + * This makes handling code blocks in () impossible. + *

As an alternative, we are creating synthetic vertices in the Graph to + * represent the static blocks as individual methods. + *

We also create one top-level synthetic method ("StaticBlockInvoker") that invokes + * individual static block methods. While creating static scope + * for this class, we should invoke the method call expressions inside the top-level synthetic + * method. * - *

private static void StaticBlockInvoker() { SyntheticStaticBlock_1(); SyntheticStaticBlock_2(); - * } } + *

New structure looks like this: + * + * class StaticBlockClass { + * private static void SyntheticStaticBlock_1() { + * System.debug("inside static block 1"); + * } + * private static void SyntheticStaticBlock_2() { + * System.debug("inside static block 2"); + * } + * private static void StaticBlockInvoker() { + * SyntheticStaticBlock_1(); + * SyntheticStaticBlock_2(); + * } + * } + * */ public final class StaticBlockUtil { private static final Logger LOGGER = LogManager.getLogger(StaticBlockUtil.class); @@ -51,19 +83,19 @@ private StaticBlockUtil() {} * Creates a synthetic method vertex to represent a static code block * * @param g traversal graph - * @param clintVertex () method's vertex + * @param clinitVertex () method's vertex * @param staticBlockIndex index to use for name uniqueness - TODO: this index is currently just * the child count index and does not necessarily follow sequence * @return new synthetic method vertex with name SyntheticStaticBlock_%d, which will be the - * parent for blockStatementVertex, and a sibling of () + * parent for blockStatementVertex, and a sibling of () */ public static Vertex createSyntheticStaticBlockMethod( GraphTraversalSource g, - Vertex clintVertex, + Vertex clinitVertex, int staticBlockIndex) { final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); - final String definingType = clintVertex.value(Schema.DEFINING_TYPE); - final List siblings = GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clintVertex)); + final String definingType = clinitVertex.value(Schema.DEFINING_TYPE); + final List siblings = GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clinitVertex)); final int nextSiblingIndex = siblings.size(); addSyntheticStaticBlockMethodProperties( @@ -73,7 +105,7 @@ public static Vertex createSyntheticStaticBlockMethod( addStaticModifierProperties(g, definingType, modifierNodeVertex); GremlinVertexUtil.addParentChildRelationship(g, syntheticMethodVertex, modifierNodeVertex); - GremlinVertexUtil.makeSiblings(g, clintVertex, syntheticMethodVertex); + GremlinVertexUtil.makeSiblings(g, clinitVertex, syntheticMethodVertex); return syntheticMethodVertex; } @@ -105,7 +137,7 @@ public static void createSyntheticStaticBlockInvocation( } static boolean isStaticBlockStatement(JorjeNode node, JorjeNode child) { - return isClintMethod(node) && containsStaticBlock(node) && isBlockStatement(child); + return isClinitMethod(node) && containsStaticBlock(node) && isBlockStatement(child); } private static Vertex createSyntheticInvocationsMethod( @@ -308,14 +340,14 @@ private static void addProperties( traversal.next(); } - /** @return true if given node represents () */ - private static boolean isClintMethod(JorjeNode node) { + /** @return true if given node represents () */ + private static boolean isClinitMethod(JorjeNode node) { return ASTConstants.NodeType.METHOD.equals(node.getLabel()) && MethodUtil.STATIC_CONSTRUCTOR_CANONICAL_NAME.equals( node.getProperties().get(Schema.NAME)); } - /** @return true if () node contains any static block definitions */ + /** @return true if () node contains any static block definitions */ private static boolean containsStaticBlock(JorjeNode node) { for (JorjeNode childNode : node.getChildren()) { if (ASTConstants.NodeType.BLOCK_STATEMENT.equals(childNode.getLabel())) { From e8832fbef3779f134042448a60cd3b5b47f4c647 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Tue, 5 Jul 2022 12:49:00 -0500 Subject: [PATCH 18/34] @W-10459675@: Part 4 of several. Added Messaging.InboundEmailHandler implementations as sources. --- .../java/com/salesforce/graph/Schema.java | 1 + .../graph/build/CaseSafePropertyUtil.java | 17 +- .../graph/build/InheritanceEdgeBuilder.java | 88 +++-- .../com/salesforce/graph/ops/MethodUtil.java | 18 + .../salesforce/graph/ops/TraversalUtil.java | 61 ++++ .../java/com/salesforce/rules/RuleUtil.java | 2 + .../build/InheritanceEdgeBuilderTest.java | 323 +++++++++++++++++- .../salesforce/graph/ops/MethodUtilTest.java | 30 ++ .../graph/ops/TraversalUtilTest.java | 95 ++++++ .../com/salesforce/rules/RuleUtilTest.java | 20 ++ 10 files changed, 617 insertions(+), 38 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index 26cd02b95..9cdaa9a65 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -28,6 +28,7 @@ public class Schema { public static final String IDENTIFIER = "Identifier"; public static final String IMPLEMENTATION_OF = "ImplementationOf"; public static final String IMPLEMENTED_BY = "ImplementedBy"; + public static final String INTERFACE_DEFINING_TYPES = "InterfaceDefiningTypes"; public static final String INTERFACE_NAMES = "InterfaceNames"; /** True if this vertex is part of the Apex Standard Library */ public static final String IS_STANDARD = "IsStandard"; diff --git a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java index e0439ee71..fc1e59e0c 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java @@ -39,12 +39,13 @@ public class CaseSafePropertyUtil { // The following properties are (with rare exception) assumed to be case-sensitive. private static final ImmutableSet CASE_SENSITIVE_PROPERTIES = ImmutableSet.of( - Schema.NAME, Schema.DEFINING_TYPE, Schema.FULL_METHOD_NAME, + Schema.INTERFACE_DEFINING_TYPES, Schema.INTERFACE_NAMES, Schema.METHOD_NAME, Schema.NAME, + Schema.RETURN_TYPE, Schema.SUPER_CLASS_NAME, Schema.SUPER_INTERFACE_NAME); @@ -191,5 +192,19 @@ public static GraphTraversal hasEndingWith( return __.has(nodeType, property, TextP.endingWith(value)); } } + + public static GraphTraversal hasArrayContaining( + String nodeType, String property, String value) { + if (property.equalsIgnoreCase(Schema.DEFINING_TYPE)) { + throw new UnexpectedException(property); + } + String caseSafeVariant = toCaseSafeProperty(property).orElse(null); + if (caseSafeVariant != null) { + return __.hasLabel(nodeType) + .has(caseSafeVariant, __.unfold().is(toCaseSafeValue(value))); + } else { + return __.hasLabel(nodeType).has(property, __.unfold().is(value)); + } + } } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java index 7847dac40..f366b90e7 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java @@ -10,7 +10,6 @@ import com.salesforce.graph.vertex.InheritableSFVertex; import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.graph.vertex.UserClassVertex; -import com.salesforce.graph.vertex.UserInterfaceVertex; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -18,10 +17,10 @@ import java.util.Map; import java.util.Optional; import java.util.TreeMap; -import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -73,7 +72,8 @@ private void buildInheritanceMaps() { private void processVertexInheritance(InheritableSFVertex vertex) { InheritableSFVertex extendedVertex = findExtendedVertex(vertex).orElse(null); - List implementedVertices = findImplementedVertices(vertex); + Map implementedVerticesByName = + findImplementedVerticesByName(vertex); // It's possible for the extendedVertex to be null, if the extended class is declared in a // file that wasn't scanned. @@ -85,17 +85,30 @@ private void processVertexInheritance(InheritableSFVertex vertex) { Schema.EXTENSION_OF); } - if (implementedVertices != null) { - for (InheritableSFVertex implementedVertex : implementedVertices) { - // It's possible for the implementedVertex to be null if the interface being - // implemented is declared in a file - // that wasn't scanned. + if (!implementedVerticesByName.isEmpty()) { + List implementedVertexDefiningTypes = new ArrayList<>(); + Long myId = vertex.getId(); + for (String implementedVertexName : implementedVerticesByName.keySet()) { + InheritableSFVertex implementedVertex = + implementedVerticesByName.get(implementedVertexName); + // If we actually have a vertex for this interface, we need to create edges + // connecting it to the implementor, + // and we can use its defining type for our list. if (implementedVertex != null) { Long implId = implementedVertex.getId(); - Long myId = vertex.getId(); addEdges(implId, myId, Schema.IMPLEMENTED_BY, Schema.IMPLEMENTATION_OF); + implementedVertexDefiningTypes.add(implementedVertex.getDefiningType()); + } else { + // If there's no vertex for this interface, we can assume it's defined in some + // other codebase, and therefore + // whatever name was used by the implementor to reference it can be assumed to + // be the defining type. + implementedVertexDefiningTypes.add(implementedVertexName); } } + // Give the implementor new properties indicating the full names of the implemented + // interfaces. + addProperties(myId, implementedVertexDefiningTypes); } } @@ -108,18 +121,20 @@ private Optional findExtendedVertex(InheritableSFVertex ver } } - private List findImplementedVertices(InheritableSFVertex vertex) { - if (vertex instanceof UserInterfaceVertex) { - return new ArrayList<>(); - } else { + private Map findImplementedVerticesByName( + InheritableSFVertex vertex) { + Map results = new HashMap<>(); + // Only classes can implement interfaces. + if (vertex instanceof UserClassVertex) { String inheritorType = vertex.getDefiningType(); - return ((UserClassVertex) vertex) - .getInterfaceNames().stream() - .map(i -> findInheritedVertex(i, inheritorType)) - .filter(v -> v.isPresent()) - .map(v -> v.get()) - .collect(Collectors.toList()); + List interfaceNames = ((UserClassVertex) vertex).getInterfaceNames(); + for (String interfaceName : interfaceNames) { + InheritableSFVertex inheritedVertex = + findInheritedVertex(interfaceName, inheritorType).orElse(null); + results.put(interfaceName, inheritedVertex); + } } + return results; } private Optional findInheritedVertex( @@ -127,17 +142,19 @@ private Optional findInheritedVertex( if (inheritableType == null) { throw new UnexpectedException("inheritableType can't be null"); } - // If the inheritor type contains a period, it's an inner type. If the inheritable type does - // not contain a period, - // it could be either an outer type, or an inner type declared in the same outer type. The - // latter overrides the - // former, so we'll derive the full name of such an inner type so we can check if it exists. - if (inheritorType.contains(".") && !inheritableType.contains(".")) { - String[] parts = inheritorType.split("\\."); - - String possibleInnerType = parts[0] + "." + inheritableType; - if (verticesByDefiningType.containsKey(possibleInnerType)) { - return Optional.ofNullable(verticesByDefiningType.get(possibleInnerType)); + // If the inheritable type does NOT contain a period, then it's either an outer type, or an + // inner type declared + // in the same outer type as the inheritor type. + // The latter takes priority over the former, so we should check if it's the case. + if (!inheritableType.contains(".")) { + // If the inheritor type contains a period, then it's an inner type, and we should use + // its outer type. + // Otherwise, we should use the inheritor type as a potential outer type. + String potentialOuterType = + inheritorType.contains(".") ? inheritorType.split("\\.")[0] : inheritorType; + String potentialInnerType = potentialOuterType + "." + inheritableType; + if (verticesByDefiningType.containsKey(potentialInnerType)) { + return Optional.ofNullable(verticesByDefiningType.get(potentialInnerType)); } } @@ -163,4 +180,15 @@ private void addEdges(Long inheritableId, Long inheritorId, String fromEdge, Str g.addE(fromEdge).from(inheritable).to(inheritor).iterate(); g.addE(toEdge).from(inheritor).to(inheritable).iterate(); } + + private void addProperties(Long inheritorId, List implementedVertexDefiningTypes) { + // Add the base property and its case-safe version. + GraphTraversal traversal = + g.V(inheritorId) + .property(Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); + CaseSafePropertyUtil.addCaseSafeProperty( + traversal, Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); + // Commit the traversal. + traversal.iterate(); + } } 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 9632bb1d6..acc1e7fc9 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -81,6 +81,9 @@ public final class MethodUtil { private static final Logger LOGGER = LogManager.getLogger(MethodUtil.class); private static final String PAGE_REFERENCE = "PageReference"; + private static final String INBOUND_EMAIL_HANDLER = "Messaging.InboundEmailHandler"; + private static final String HANDLE_INBOUND_EMAIL = "handleInboundEmail"; + private static final String INBOUND_EMAIL_RESULT = "Messaging.InboundEmailResult"; public static final String INSTANCE_CONSTRUCTOR_CANONICAL_NAME = ""; public static final String STATIC_CONSTRUCTOR_CANONICAL_NAME = ""; @@ -257,6 +260,21 @@ public static List getGlobalMethods( __.not(__.has(Schema.IS_STANDARD, true))))); } + public static List getInboundEmailHandlerMethods( + GraphTraversalSource g, List targetFiles) { + return SFVertexFactory.loadVertices( + g, + // Get any target class that implements the email handler interface. + TraversalUtil.traverseImplementationsOf(g, targetFiles, INBOUND_EMAIL_HANDLER) + // Get every implementation of the handle email method. + .out(Schema.CHILD) + .where(H.has(NodeType.METHOD, Schema.NAME, HANDLE_INBOUND_EMAIL)) + // Filter the results by return type and arity to limit the possibility of + // getting unnecessary results. + .where(H.has(NodeType.METHOD, Schema.RETURN_TYPE, INBOUND_EMAIL_RESULT)) + .has(Schema.ARITY, 2)); + } + /** * 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/graph/ops/TraversalUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java index bdc0e831b..127b5195e 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java @@ -1,7 +1,9 @@ package com.salesforce.graph.ops; import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.apex.jorje.ASTConstants.NodeType; import com.salesforce.graph.Schema; +import com.salesforce.graph.build.CaseSafePropertyUtil.H; import com.salesforce.rules.AbstractRuleRunner.RuleRunnerTarget; import java.util.List; import java.util.stream.Collectors; @@ -68,5 +70,64 @@ public static GraphTraversal ruleTargetTraversal( .toArray(GraphTraversal[]::new)); } + public static GraphTraversal traverseImplementationsOf( + GraphTraversalSource g, List targetFiles, String interfaceName) { + // For our traversal, we want to start with every class that implements either the target + // interface or one of its + // subtypes. Do that with a union of these two subtraversals. + // Also, start with the hasLabel() call, because doing an initial filter along an indexed + // field saves us a ton of time. + GraphTraversal traversal = + g.V().hasLabel(NodeType.USER_CLASS, NodeType.USER_INTERFACE) + .union( + // Subtraversal 1: Get every class that implements the target + // interface, via a helper method. + __.where( + H.hasArrayContaining( + NodeType.USER_CLASS, + Schema.INTERFACE_DEFINING_TYPES, + interfaceName)), + // Subtraversal 2: Get every class that implements a subtype of the + // target interface, by starting with all + // the direct subtypes of the interface... + __.where( + H.has( + NodeType.USER_INTERFACE, + Schema.SUPER_INTERFACE_NAME, + interfaceName)) + .union( + __.identity(), + // ...recursively adding subtypes of those + // interfaces... + __.repeat(__.out(Schema.EXTENDED_BY)).emit()) + // ... and getting every class that implements one of those + // interfaces. + .out(Schema.IMPLEMENTED_BY) + // Now, we add every subclass of any of these classes, recursively. + ) + .union(__.identity(), __.repeat(__.out(Schema.EXTENDED_BY)).emit()); + + // If there are no target files, we're clear to return this traversal since it includes all + // relevant classes in + // the graph. + if (targetFiles.isEmpty()) { + return traversal; + } else { + // Otherwise, we need to filter so we're only returning the classes in a target file. To + // do that, do a traversal + // to get all the IDs of classes defined in the target files. + Object[] targetIds = + fileRootTraversal(g, targetFiles) + .union(__.identity(), __.repeat(__.out(Schema.CHILD)).emit()) + .hasLabel(NodeType.USER_CLASS) + .id() + .toList() + .toArray(); + // Note, there's no .hasId(Object...) overload, so we're using the .hasId(Object, + // ...Object) overload instead. + return traversal.hasId(targetIds); + } + } + private TraversalUtil() {} } diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index 5e088f338..f5a4858c5 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -67,6 +67,8 @@ public static List getPathEntryPoints( methods.addAll(MethodUtil.getPageReferenceMethods(g, fileLevelTargets)); // ...and global-exposed methods... methods.addAll(MethodUtil.getGlobalMethods(g, fileLevelTargets)); + // ...and implementations of Messaging.InboundEmailHandler#handleInboundEmail... + methods.addAll(MethodUtil.getInboundEmailHandlerMethods(g, fileLevelTargets)); // ...and exposed methods on VF controllers. methods.addAll(MethodUtil.getExposedControllerMethods(g, fileLevelTargets)); } diff --git a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java index 2f9bcc137..256533d09 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java @@ -1,7 +1,19 @@ package com.salesforce.graph.build; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.core.IsEqual.equalTo; + import com.salesforce.TestUtil; +import com.salesforce.apex.jorje.ASTConstants.NodeType; +import com.salesforce.graph.Schema; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.apache.tinkerpop.gremlin.process.traversal.Order; +import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -14,22 +26,319 @@ public void setup() { } @Test - public void testSimpleExtension() { - // TODO + public void testNoInheritance() { + String[] sourceCodes = {"public class c1 {}", "public interface i1 {}"}; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that nothing has any inheritance edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testSimpleClassExtension() { + String[] sourceCodes = { + "public class c1 {}", "public class c2 extends c1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the extension looks right. + verifyExtension(g, "c1", "c2"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test - public void testInnerTypeExtension() { - // TODO + public void testSimpleInterfaceExtension() { + String[] sourceCodes = { + "public interface i1 {}", "public interface i2 extends i1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the extension looks right. + verifyExtension(g, "i1", "i2"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test public void testSimpleImplementation() { - // TODO + String[] sourceCodes = { + "public interface i1 {}", "public class c1 implements i1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the implementations look right. + verifyImplementation(g, "i1", "c1"); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testImplementationOfExternalInterface() { + String[] sourceCodes = {"public class c1 implements ExternallyDeclaredInterface {}"}; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right properties were set. + verifyInterfaceDefiningTypeProperty( + g, "ExternallyDeclaredInterface", Collections.singletonList("c1")); + // Verify that no edges were created. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testExtensionOfInnerClass() { + String[] sourceCodes = { + // Extensions in this class don't use the outer class name. + "public class OuterClass1 extends InnerClass1 {\n" + + " public class InnerClass1 {}\n" + + " public class InnerClass2 extends InnerClass1 {}\n" + + "}", + // Extensions in this class use the outer class name. + "public class OuterClass2 extends OuterClass2.InnerClass1 {\n" + + " public class InnerClass1 {}\n" + + " public class InnerClass2 extends InnerClass1 {}\n" + + "}", + // Third class extends one of the inner classes in another class. + "public class OuterClass3 extends OuterClass2.InnerClass1 {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Make sure OuterClass1.InnerClass1 is extended by the right classes. + verifyExtension( + g, + "OuterClass1.InnerClass1", + Arrays.asList("OuterClass1", "OuterClass1.InnerClass2")); + + // Make sure OuterClass2.InnerClass1 is extended by the right classes. + verifyExtension( + g, + "OuterClass2.InnerClass1", + Arrays.asList("OuterClass2", "OuterClass2.InnerClass2", "OuterClass3")); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + } + + @Test + public void testImplementationOfInnerInterface() { + String[] sourceCodes = { + // Implementations in this class don't use the outer class name. + "public class OuterClass1 implements InnerInterface1 {\n" + + " public class InnerInterface1 {}\n" + + " public class InnerClass1 implements InnerInterface1 {}\n" + + "}", + // Implementations in this class use the outer class name. + "public class OuterClass2 implements OuterClass2.InnerInterface1 {\n" + + " public class InnerInterface1 {}\n" + + " public class InnerClass1 implements InnerInterface1 {}\n" + + "}", + // Third class extends one of the inner classes in another class. + "public class OuterClass3 implements OuterClass2.InnerInterface1 {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Make sure OuterClass1.InnerInterface1 is implemented by the right classes. + verifyImplementation( + g, + "OuterClass1.InnerInterface1", + Arrays.asList("OuterClass1", "OuterClass1.InnerClass1")); + + // Make sure OuterClass2.InnerInterface1 is implemented by the right classes. + verifyImplementation( + g, + "OuterClass2.InnerInterface1", + Arrays.asList("OuterClass2", "OuterClass2.InnerClass1", "OuterClass3")); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testExtensionOfOwnOuterClass() { + String[] sourceCodes = { + "public class OuterClass1 {\n" + + " public class InnerClass1 extends OuterClass1 {}\n" + + "}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right extension edges exist. + verifyExtension(g, "OuterClass1", "OuterClass1.InnerClass1"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test - public void testInnerTypeImplementation() { - // TODO + public void testClassNameOverlap() { + String[] sourceCodes = { + // This class extends its own inner class without using the outer class name. + "public class OuterClass1 extends NameOverlappedClass {\n" + + " public class NameOverlappedClass {}\n" + + "}", + // This class has the same name as the inner class the other class extends. + "public class NameOverlappedClass {}", + // This class extends the outer class with the conflict name. + "public class OuterClass2 extends NameOverlappedClass {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right extension edges exist. + verifyExtension(g, "NameOverlappedClass", "OuterClass2"); + verifyExtension(g, "OuterClass1.NameOverlappedClass", "OuterClass1"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + } + + @Test + public void testInterfaceNameOverlap() { + String[] sourceCodes = { + // This class implements its own inner interface without using the outer class name. + "public class OuterClass1 implements NameOverlappedInterface {\n" + + " public class NameOverlappedInterface {}\n" + + "}", + // This interface has the same name as the inner interface the other class extends. + "public interface NameOverlappedInterface {}", + // This class implements the outer class with the conflict name. + "public class OuterClass2 implements NameOverlappedInterface {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right implementation edges exist. + verifyImplementation(g, "NameOverlappedInterface", "OuterClass2"); + verifyImplementation(g, "OuterClass1.NameOverlappedInterface", "OuterClass1"); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + private void verifyNoEdges(GraphTraversalSource g, String edgeName) { + List vertices = g.V().out(edgeName).toList(); + MatcherAssert.assertThat(vertices, hasSize(equalTo(0))); + } + + private void verifyExtension( + GraphTraversalSource g, String parentDefiningType, String childDefiningType) { + verifyExtension(g, parentDefiningType, Collections.singletonList(childDefiningType)); + } + + private void verifyExtension( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + verifyEdges( + g, Schema.EXTENDED_BY, Schema.EXTENSION_OF, parentDefiningType, childDefiningTypes); + } + + private void verifyImplementation( + GraphTraversalSource g, String parentDefiningType, String childDefiningType) { + verifyImplementation(g, parentDefiningType, Collections.singletonList(childDefiningType)); + } + + private void verifyImplementation( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + verifyEdges( + g, + Schema.IMPLEMENTED_BY, + Schema.IMPLEMENTATION_OF, + parentDefiningType, + childDefiningTypes); + verifyInterfaceDefiningTypeProperty(g, parentDefiningType, childDefiningTypes); + } + + private void verifyEdges( + GraphTraversalSource g, + String outgoingEdge, + String incomingEdge, + String parentDefiningType, + List childDefiningTypes) { + List outgoingVertices = + g.V().has(Schema.DEFINING_TYPE, parentDefiningType) + .out(outgoingEdge) + .values(Schema.DEFINING_TYPE) + .order(Scope.global) + .by(Order.asc) + .toList(); + List incomingVertices = + g.V().has(Schema.DEFINING_TYPE, parentDefiningType) + .in(incomingEdge) + .values(Schema.DEFINING_TYPE) + .order(Scope.global) + .by(Order.asc) + .toList(); + + // Both lists should have the same vertices in the same order. + MatcherAssert.assertThat(outgoingVertices, hasSize(equalTo(childDefiningTypes.size()))); + MatcherAssert.assertThat(incomingVertices, hasSize(equalTo(childDefiningTypes.size()))); + for (int i = 0; i < childDefiningTypes.size(); i++) { + MatcherAssert.assertThat( + outgoingVertices.get(i).toString(), equalTo(childDefiningTypes.get(i))); + MatcherAssert.assertThat( + incomingVertices.get(i).toString(), equalTo(childDefiningTypes.get(i))); + } + } + + private void verifyInterfaceDefiningTypeProperty( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + // For each child type, get the property on that vertex. + List propValues = + g.V().where( + CaseSafePropertyUtil.H.hasWithin( + NodeType.USER_CLASS, + Schema.DEFINING_TYPE, + childDefiningTypes)) + .values(Schema.INTERFACE_DEFINING_TYPES) + .toList(); + for (Object propValue : propValues) { + MatcherAssert.assertThat( + propValue.toString(), + equalTo(Collections.singletonList(parentDefiningType).toString())); + } } } 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 52336ec4f..8997082ea 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -428,4 +428,34 @@ public void testGetGlobalMethods() { MatcherAssert.assertThat(excludedName, excludedMethod.isTest(), equalTo(true)); } } + + @Test + public void testGetInboundEmailHandlerMethods() { + String[] sourceCode = { + "public class MyClass implements Messaging.InboundEmailHandler {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\n" + + " }\n" + + "}\n", + "public class MyClass2 {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\n" + + " }\n" + + "}\n" + }; + TestUtil.buildGraph(g, sourceCode); + + List methods = MethodUtil.getInboundEmailHandlerMethods(g, new ArrayList<>()); + // The `MyClass#handleInboundEmail` method should be included because it's an implementation + // of the desired interface. + MatcherAssert.assertThat(methods, hasSize(equalTo(1))); + MatcherAssert.assertThat(methods.get(0).getName(), equalTo("handleInboundEmail")); + MatcherAssert.assertThat(methods.get(0).getDefiningType(), equalTo("MyClass")); + } } diff --git a/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java index 434bb67ce..35e946355 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java @@ -2,6 +2,7 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.salesforce.TestUtil; import com.salesforce.apex.jorje.ASTConstants; @@ -11,6 +12,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; @@ -18,6 +21,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.CsvSource; public class TraversalUtilTest { private GraphTraversalSource g; @@ -63,6 +68,47 @@ public class TraversalUtilTest { + " }\n" + "}\n"; + private String[] interfaceTestSourceCodes = { + // An interface + "public interface MyInterface1 {}", + // An interface that extends another interface. + "public interface MyInterface2 extends MyInterface1 {}", + // A class that implements the first interface. + "public class MyClass1 implements MyInterface1 {}", + // A class that extends a class that implements an interface. + "public class MyClass2 extends MyClass1 {}", + // A class that implements an interface that extends another interface. + "public class MyClass3 implements MyInterface2 {}", + // A class that implements its own inner interface without using the outer class syntax. + "public class MyClass4 implements InnerInterface1 {\n" + + " public interface InnerInterface1 {}\n" + // A class that implements an inner interface in the same outer class, without using + // the outer class syntax. + + " public class InnerClass1 implements InnerInterface1 {}\n" + // A class that implements an inner interface in the same outer class, using the + // outer class syntax. + + " public class InnerClass2 implements MyClass4.InnerInterface1 {}\n" + // A class that does nothing in particular. + + " public class InnerClass3 {}\n" + + "}", + // A class that implements its own inner interface using the outer class syntax. + "public class MyClass5 implements MyClass5.InnerInterface1 {\n" + + " public interface InnerInterface1 {}\n" + + "}", + // A class that implements another class's inner interface. + "public class MyClass6 implements MyClass4.InnerInterface1 {}", + // An interface whose name will be shared by an inner interface in another class. + "public interface SharedInterfaceName1 {}", + // A class implementing an inner interface whose name is shared by another outer interface. + "public class MyClass7 implements SharedInterfaceName1 {\n" + + " public interface SharedInterfaceName1 {}\n" + + "}", + // A class implementing the outer interface with the duplicate name. + "public class MyClass8 implements SharedInterfaceName1 {}", + // A class that does nothing in particular. + "public class MyClass9 {}" + }; + @BeforeEach public void setup() { this.g = TestUtil.getGraph(); @@ -213,4 +259,53 @@ public void ruleTargetTraversal_MethodAndFile() { MatcherAssert.assertThat( targetResults, hasSize(expectedWholeFile.size() + expectedSingleMethod.size())); } + + @ParameterizedTest(name = "{displayName}: {0}") + @CsvSource({ + // MyInterface1 is directly implemented by MyClass1, and indirectly implemented by MyClass2 + // and MyClass3. + "MyInterface1, 'MyClass1,MyClass2,MyClass3'", + // MyInterface2 is directly implemented by MyClass3 + "MyInterface2, 'MyClass3'", + // MyClass4.InnerInterface1 is directly implemented by MyClass4, two of its inner classes, + // and MyClass6. + "MyClass4.InnerInterface1, 'MyClass4,MyClass4.InnerClass1,MyClass4.InnerClass2,MyClass6'", + // MyClass5.InnerInterface1 is implemented only by its outer class, MyClass5. + "MyClass5.InnerInterface1, 'MyClass5'", + // SharedInterfaceName1 is implemented by MyClass8. + "SharedInterfaceName1, 'MyClass8'", + // MyClass7.SharedInterfaceName1 is implemented by MyClass7. + "MyClass7.SharedInterfaceName1, 'MyClass7'" + }) + public void traverseImplementationsOf_testWithoutTargetFiles( + String interfaceName, String expectedClassNameString) { + String[] expectedClassNames = expectedClassNameString.split(","); + TestUtil.buildGraph(g, interfaceTestSourceCodes); + + // RUN THE TESTED METHOD. + List traversedVertices = + TraversalUtil.traverseImplementationsOf(g, new ArrayList<>(), interfaceName) + // For testing purposes, get the name of each vertex instead of returning + // the whole vertex. + .values(Schema.DEFINING_TYPE) + .toList(); + // Turn the object list into a string set for comparison purposes. + Set traversedVertexNames = + traversedVertices.stream().map(Object::toString).collect(Collectors.toSet()); + + // Create an array of booleans indicating whether we returned each expected class. + boolean[] expectedClassPresent = new boolean[expectedClassNames.length]; + Arrays.fill(expectedClassPresent, false); + + // Iterate over the expected classes and set each class's boolean based on whether it's in + // the name set. + for (int i = 0; i < expectedClassPresent.length; i++) { + expectedClassPresent[i] = traversedVertexNames.contains(expectedClassNames[i]); + } + + // If any of the booleans are still false, the test fails. + for (int i = 0; i < expectedClassPresent.length; i++) { + assertTrue(expectedClassPresent[i], "Class " + expectedClassNames[i] + " is absent"); + } + } } diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index e55890da4..ec8cb84f9 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -117,6 +117,26 @@ public void getPathEntryPoints_includesPageReferenceMethods() { assertEquals("pageRefMethod", firstVertex.getName()); } + @Test + public void getPathEntryPoints_includesInboundEmailHandlerMethods() { + String sourceCode = + "public class MyClass implements Messaging.InboundEmailHandler {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\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("handleInboundEmail", firstVertex.getName()); + } + @Test public void getPathEntryPoints_includesExposedControllerMethods() { try { From 0ae8880a80331434abd9bf794f30c30c293f44a4 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Tue, 5 Jul 2022 13:35:24 -0500 Subject: [PATCH 19/34] @W-10459675@: Added documentation for methods. --- .../main/java/com/salesforce/graph/ops/TraversalUtil.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java index 127b5195e..31360313a 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java @@ -70,6 +70,11 @@ public static GraphTraversal ruleTargetTraversal( .toArray(GraphTraversal[]::new)); } + /** + * Returns a traversal containing every class in the target files that implements the specified interface, either + * directly or indirectly (i.e., by extending a class that implements it, implementing an interface that extends it, + * or extending a class that does either of those things). An empty target array implicitly targets the whole graph. + */ public static GraphTraversal traverseImplementationsOf( GraphTraversalSource g, List targetFiles, String interfaceName) { // For our traversal, we want to start with every class that implements either the target From 92e2a458c7f33d8177a3202fb3fb689f0f47606a Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 6 Jul 2022 11:12:26 -0500 Subject: [PATCH 20/34] @W-10459675@: Integrated changes from rebase into PR. --- .../build/AbstractApexVertexBuilder.java | 8 +-- .../graph/build/InheritanceEdgeBuilder.java | 13 +++-- .../graph/build/StaticBlockUtil.java | 51 +++++++++---------- .../salesforce/graph/ops/TraversalUtil.java | 7 +-- .../com/salesforce/rules/RuleUtilTest.java | 16 +++--- 5 files changed, 47 insertions(+), 48 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index 2cd405346..ddf737c4b 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -82,9 +82,11 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { final Vertex vChild = g.addV(child.getLabel()).next(); addProperties(g, child, vChild); - /** Handle static block if we are looking at a method that has a block statement. - * See {@linkplain StaticBlockUtil} on why this is needed - * and how we handle it. */ + /** + * Handle static block if we are looking at a method that has a block + * statement. See {@linkplain StaticBlockUtil} on why this is needed and how we handle + * it. + */ if (StaticBlockUtil.isStaticBlockStatement(node, child)) { final Vertex parentVertexForChild = StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, i); diff --git a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java index f366b90e7..5e3670226 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java @@ -17,6 +17,7 @@ import java.util.Map; import java.util.Optional; import java.util.TreeMap; +import java.util.TreeSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tinkerpop.gremlin.process.traversal.P; @@ -182,13 +183,11 @@ private void addEdges(Long inheritableId, Long inheritorId, String fromEdge, Str } private void addProperties(Long inheritorId, List implementedVertexDefiningTypes) { - // Add the base property and its case-safe version. - GraphTraversal traversal = - g.V(inheritorId) - .property(Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); - CaseSafePropertyUtil.addCaseSafeProperty( - traversal, Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); - // Commit the traversal. + GraphTraversal traversal = g.V(inheritorId); + // It's probably not best practice to be using an empty treeset here, but we no longer have access to the treeset + // that was used when adding the base properties, and we're setting a property that definitely isn't set elsewhere, + // so it should be fine. + GremlinVertexUtil.addProperty(new TreeSet<>(), traversal, Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); traversal.iterate(); } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java index 3a2a8756f..339cce69f 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -19,9 +19,7 @@ /** * Handles creation of synthetic methods and vertices to gracefully invoke static code blocks. * - *

Consider this example: - * - * + *

Consider this example: * class StaticBlockClass { * static { * System.debug("inside static block 1"); @@ -30,10 +28,7 @@ * System.debug("inside static block 2"); * } * } - * - * - * In Jorje's compilation structure, static blocks are represented like this: - * + * In Jorje's compilation structure, static blocks are represented like this: * class StaticBlockClass { * private static void () { * { @@ -46,17 +41,17 @@ * } * * - *

Having multiple block statements inside a method breaks SFGE's normal code flow logic. - * This makes handling code blocks in () impossible. - *

As an alternative, we are creating synthetic vertices in the Graph to - * represent the static blocks as individual methods. - *

We also create one top-level synthetic method ("StaticBlockInvoker") that invokes - * individual static block methods. While creating static scope - * for this class, we should invoke the method call expressions inside the top-level synthetic - * method. + *

Having multiple block statements inside a method breaks SFGE's normal code flow logic. This + * makes handling code blocks in () impossible. + * + *

As an alternative, we are creating synthetic vertices in the Graph to represent the static + * blocks as individual methods. + * + *

We also create one top-level synthetic method ("StaticBlockInvoker") that invokes individual + * static block methods. While creating static scope for this class, we should invoke the method + * call expressions inside the top-level synthetic method. * - *

New structure looks like this: - * + *

New structure looks like this: * class StaticBlockClass { * private static void SyntheticStaticBlock_1() { * System.debug("inside static block 1"); @@ -90,12 +85,11 @@ private StaticBlockUtil() {} * parent for blockStatementVertex, and a sibling of () */ public static Vertex createSyntheticStaticBlockMethod( - GraphTraversalSource g, - Vertex clinitVertex, - int staticBlockIndex) { + GraphTraversalSource g, Vertex clinitVertex, int staticBlockIndex) { final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); final String definingType = clinitVertex.value(Schema.DEFINING_TYPE); - final List siblings = GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clinitVertex)); + final List siblings = + GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clinitVertex)); final int nextSiblingIndex = siblings.size(); addSyntheticStaticBlockMethodProperties( @@ -286,7 +280,10 @@ private static void addStaticModifierProperties( } private static void addStaticBlockInvokerProperties( - GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex, int childIndex) { + GraphTraversalSource g, + String definingType, + Vertex staticBlockInvokerVertex, + int childIndex) { verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); @@ -296,11 +293,11 @@ private static void addStaticBlockInvokerProperties( } private static void addSyntheticStaticBlockMethodProperties( - GraphTraversalSource g, - String definingType, - Vertex syntheticMethodVertex, - int staticBlockIndex, - int childIndex) { + GraphTraversalSource g, + String definingType, + Vertex syntheticMethodVertex, + int staticBlockIndex, + int childIndex) { verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put( diff --git a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java index 31360313a..8c98aa4fa 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java @@ -71,9 +71,10 @@ public static GraphTraversal ruleTargetTraversal( } /** - * Returns a traversal containing every class in the target files that implements the specified interface, either - * directly or indirectly (i.e., by extending a class that implements it, implementing an interface that extends it, - * or extending a class that does either of those things). An empty target array implicitly targets the whole graph. + * Returns a traversal containing every class in the target files that implements the specified + * interface, either directly or indirectly (i.e., by extending a class that implements it, + * implementing an interface that extends it, or extending a class that does either of those + * things). An empty target array implicitly targets the whole graph. */ public static GraphTraversal traverseImplementationsOf( GraphTraversalSource g, List targetFiles, String interfaceName) { diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index ec8cb84f9..1e1e03d60 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); From 462eca9b97433dbaa442fdcaaf28f5907d802806 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 6 Jul 2022 12:33:41 -0500 Subject: [PATCH 21/34] @W-10459675@: Fixed failing tests. --- .../graph/build/CaseSafePropertyUtil.java | 6 +++--- .../graph/build/InheritanceEdgeBuilder.java | 14 ++++++++++---- .../graph/build/CustomerApexVertexBuilderTest.java | 5 ++--- .../graph/build/InheritanceEdgeBuilderTest.java | 5 +++-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java index fc1e59e0c..3d068a49c 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java @@ -81,16 +81,16 @@ private static String toCaseSafeValue(String value) { return value.toLowerCase(Locale.ROOT); } - private static ArrayList toCaseSafeValue(ArrayList value) { + private static Object[] toCaseSafeValue(ArrayList value) { if (value.isEmpty() || !(value.get(0) instanceof String)) { // An empty array or non-string array can't be made case-safe. - return value; + return value.toArray(); } else { ArrayList stringList = new ArrayList<>(); for (Object o : value) { stringList.add(((String) o).toLowerCase(Locale.ROOT)); } - return stringList; + return stringList.toArray(); } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java index 5e3670226..686d48eef 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java @@ -183,11 +183,17 @@ private void addEdges(Long inheritableId, Long inheritorId, String fromEdge, Str } private void addProperties(Long inheritorId, List implementedVertexDefiningTypes) { - GraphTraversal traversal = g.V(inheritorId); - // It's probably not best practice to be using an empty treeset here, but we no longer have access to the treeset - // that was used when adding the base properties, and we're setting a property that definitely isn't set elsewhere, + GraphTraversal traversal = g.V(inheritorId); + // It's probably not best practice to be using an empty treeset here, but we no longer have + // access to the treeset + // that was used when adding the base properties, and we're setting a property that + // definitely isn't set elsewhere, // so it should be fine. - GremlinVertexUtil.addProperty(new TreeSet<>(), traversal, Schema.INTERFACE_DEFINING_TYPES, implementedVertexDefiningTypes); + GremlinVertexUtil.addProperty( + new TreeSet<>(), + traversal, + Schema.INTERFACE_DEFINING_TYPES, + implementedVertexDefiningTypes); traversal.iterate(); } } diff --git a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java index 5c63f768a..8716abf0e 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java @@ -117,11 +117,10 @@ public void testCaseSafeProperties() { Schema.SUPER_CLASS_NAME + CaseSafePropertyUtil.CASE_SAFE_SUFFIX)); assertEquals( "someinterface", - ((ArrayList) + ((Object[]) userClassVertex.get( Schema.INTERFACE_NAMES - + CaseSafePropertyUtil.CASE_SAFE_SUFFIX)) - .get(0)); + + CaseSafePropertyUtil.CASE_SAFE_SUFFIX))[0]); Map userInterfaceVertex = g.V().hasLabel(NodeType.USER_INTERFACE).elementMap().next(); diff --git a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java index 256533d09..bceff013f 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java @@ -1,5 +1,6 @@ package com.salesforce.graph.build; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.IsEqual.equalTo; @@ -337,8 +338,8 @@ private void verifyInterfaceDefiningTypeProperty( .toList(); for (Object propValue : propValues) { MatcherAssert.assertThat( - propValue.toString(), - equalTo(Collections.singletonList(parentDefiningType).toString())); + (Object[])propValue, + arrayContaining(parentDefiningType)); } } } From 508aef79e73fdf92f3dc67bf0d0685e905888db4 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Thu, 30 Jun 2022 15:43:43 -0700 Subject: [PATCH 22/34] 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 23/34] 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 24/34] 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) { From 1c2c604acb816a35211ba3b1e48376a59cb2b6fe Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 6 Jul 2022 13:46:49 -0500 Subject: [PATCH 25/34] @W-10459675@: Part 5 of 5. Added InvocableMethod-annotated methods as sources. --- sfge/src/main/java/com/salesforce/graph/Schema.java | 1 + .../java/com/salesforce/graph/ops/MethodUtil.java | 11 +++++++++-- sfge/src/main/java/com/salesforce/rules/RuleUtil.java | 2 ++ .../graph/build/CustomerApexVertexBuilderTest.java | 4 ++-- .../graph/build/InheritanceEdgeBuilderTest.java | 4 +--- .../java/com/salesforce/graph/ops/MethodUtilTest.java | 10 ++++++++-- .../test/java/com/salesforce/rules/RuleUtilTest.java | 8 +++++++- 7 files changed, 30 insertions(+), 10 deletions(-) diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index 9cdaa9a65..8e865d78f 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -30,6 +30,7 @@ public class Schema { public static final String IMPLEMENTED_BY = "ImplementedBy"; public static final String INTERFACE_DEFINING_TYPES = "InterfaceDefiningTypes"; public static final String INTERFACE_NAMES = "InterfaceNames"; + public static final String INVOCABLE_METHOD = "InvocableMethod"; /** True if this vertex is part of the Apex Standard Library */ public static final String IS_STANDARD = "IsStandard"; /** 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 acc1e7fc9..aaad1de86 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -189,10 +189,17 @@ public static List getRemoteActionMethods( return getMethodsWithAnnotation(g, targetFiles, Schema.REMOTE_ACTION); } + /** + * Returns non-test methods in the target files with an @InvocableMethod annotation. An empty + * list implicitly includes all files. + */ + public static List getInvocableMethodMethods( + GraphTraversalSource g, List targetFiles) { + return getMethodsWithAnnotation(g, targetFiles, Schema.INVOCABLE_METHOD); + } + 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, rootMethodTraversal(g, targetFiles) diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index f5a4858c5..1aa76d56f 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -63,6 +63,8 @@ public static List getPathEntryPoints( methods.addAll(MethodUtil.getNamespaceAccessibleMethods(g, fileLevelTargets)); // ...and RemoteAction methods... methods.addAll(MethodUtil.getRemoteActionMethods(g, fileLevelTargets)); + // ...and InvocableMethod methods... + methods.addAll(MethodUtil.getInvocableMethodMethods(g, fileLevelTargets)); // ...and PageReference methods... methods.addAll(MethodUtil.getPageReferenceMethods(g, fileLevelTargets)); // ...and global-exposed methods... diff --git a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java index 8716abf0e..b568e8370 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java @@ -13,7 +13,6 @@ import com.salesforce.graph.ops.MethodUtil; import com.salesforce.graph.vertex.MethodVertex; import com.salesforce.graph.vertex.SFVertexFactory; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -120,7 +119,8 @@ public void testCaseSafeProperties() { ((Object[]) userClassVertex.get( Schema.INTERFACE_NAMES - + CaseSafePropertyUtil.CASE_SAFE_SUFFIX))[0]); + + CaseSafePropertyUtil.CASE_SAFE_SUFFIX)) + [0]); Map userInterfaceVertex = g.V().hasLabel(NodeType.USER_INTERFACE).elementMap().next(); diff --git a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java index bceff013f..cdc491446 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java @@ -337,9 +337,7 @@ private void verifyInterfaceDefiningTypeProperty( .values(Schema.INTERFACE_DEFINING_TYPES) .toList(); for (Object propValue : propValues) { - MatcherAssert.assertThat( - (Object[])propValue, - arrayContaining(parentDefiningType)); + MatcherAssert.assertThat((Object[]) propValue, arrayContaining(parentDefiningType)); } } } 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 8997082ea..b69e82bfc 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -335,7 +335,13 @@ public void getTargetMethods_targetMethodInInnerAndOuterClass() { containsString(EventKey.WARNING_MULTIPLE_METHOD_TARGET_MATCHES.getMessageKey())); } - @ValueSource(strings = {Schema.AURA_ENABLED, Schema.REMOTE_ACTION, Schema.NAMESPACE_ACCESSIBLE}) + @ValueSource( + strings = { + Schema.AURA_ENABLED, + Schema.INVOCABLE_METHOD, + Schema.REMOTE_ACTION, + Schema.NAMESPACE_ACCESSIBLE + }) @ParameterizedTest(name = "{displayName}: {0}") public void testGetMethodsWithAnnotation(String annotation) { String[] sourceCode = { @@ -369,7 +375,7 @@ public void testGetMethodsWithAnnotation(String annotation) { + "}\n", }; - TestUtil.buildGraph(g, sourceCode); + TestUtil.buildGraph(g, sourceCode, true); List methods = MethodUtil.getMethodsWithAnnotation(g, new ArrayList<>(), annotation); diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index 1e1e03d60..16a638670 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -37,7 +37,13 @@ public void setup() { this.g = TestUtil.getGraph(); } - @ValueSource(strings = {Schema.AURA_ENABLED, Schema.REMOTE_ACTION, Schema.NAMESPACE_ACCESSIBLE}) + @ValueSource( + strings = { + Schema.AURA_ENABLED, + Schema.INVOCABLE_METHOD, + Schema.REMOTE_ACTION, + Schema.NAMESPACE_ACCESSIBLE + }) @ParameterizedTest(name = "{displayName}: {0}") public void getPathEntryPoints_includesAnnotatedMethods(String annotation) { String sourceCode = From 941ab87f2815f0eebe6d856185edb639db23c039 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 6 Jul 2022 13:50:01 -0500 Subject: [PATCH 26/34] @W-10459675@: Renamed InheritanceEdgeBuilder to reflect its broadened behavior. --- ...eEdgeBuilder.java => InheritanceInformationBuilder.java} | 6 +++--- sfge/src/main/java/com/salesforce/graph/build/Util.java | 2 +- ...lderTest.java => InheritanceInformationBuilderTest.java} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename sfge/src/main/java/com/salesforce/graph/build/{InheritanceEdgeBuilder.java => InheritanceInformationBuilder.java} (98%) rename sfge/src/test/java/com/salesforce/graph/build/{InheritanceEdgeBuilderTest.java => InheritanceInformationBuilderTest.java} (99%) diff --git a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/InheritanceInformationBuilder.java similarity index 98% rename from sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java rename to sfge/src/main/java/com/salesforce/graph/build/InheritanceInformationBuilder.java index 686d48eef..2442a370a 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/InheritanceInformationBuilder.java @@ -25,13 +25,13 @@ import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.structure.Vertex; -public class InheritanceEdgeBuilder implements GraphBuilder { - private static final Logger LOGGER = LogManager.getLogger(InheritanceEdgeBuilder.class); +public class InheritanceInformationBuilder implements GraphBuilder { + private static final Logger LOGGER = LogManager.getLogger(InheritanceInformationBuilder.class); private final GraphTraversalSource g; private final Map verticesById; private final TreeMap verticesByDefiningType; - public InheritanceEdgeBuilder(GraphTraversalSource g) { + public InheritanceInformationBuilder(GraphTraversalSource g) { this.g = g; this.verticesById = new HashMap<>(); this.verticesByDefiningType = CollectionUtil.newTreeMap(); diff --git a/sfge/src/main/java/com/salesforce/graph/build/Util.java b/sfge/src/main/java/com/salesforce/graph/build/Util.java index cb0854ecf..b0c6b5d37 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/Util.java +++ b/sfge/src/main/java/com/salesforce/graph/build/Util.java @@ -23,7 +23,7 @@ public static void buildGraph(Config config) { new GraphBuilder[] { new ApexStandardLibraryVertexBuilder(config.g), new CustomerApexVertexBuilder(config.g, config.customerCompilations), - new InheritanceEdgeBuilder(config.g) + new InheritanceInformationBuilder(config.g) }) { if (config.ignoreBuilders.contains(graphBuilder.getClass())) { if (LOGGER.isWarnEnabled()) { diff --git a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/InheritanceInformationBuilderTest.java similarity index 99% rename from sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java rename to sfge/src/test/java/com/salesforce/graph/build/InheritanceInformationBuilderTest.java index cdc491446..4213fbc92 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/InheritanceInformationBuilderTest.java @@ -18,7 +18,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class InheritanceEdgeBuilderTest { +public class InheritanceInformationBuilderTest { private GraphTraversalSource g; @BeforeEach From f8aaac796777437fc62f7cea1331812e55ad3eff Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 11 Jul 2022 10:48:55 -0500 Subject: [PATCH 27/34] @W-11404189@: Added VIRTUAL and OVERRIDE keywords to graph. --- .../java/com/salesforce/apex/jorje/ModifierNodeWrapper.java | 2 ++ sfge/src/main/java/com/salesforce/graph/Schema.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/sfge/src/main/java/com/salesforce/apex/jorje/ModifierNodeWrapper.java b/sfge/src/main/java/com/salesforce/apex/jorje/ModifierNodeWrapper.java index 52e0e5b10..d98304b95 100644 --- a/sfge/src/main/java/com/salesforce/apex/jorje/ModifierNodeWrapper.java +++ b/sfge/src/main/java/com/salesforce/apex/jorje/ModifierNodeWrapper.java @@ -23,5 +23,7 @@ protected void fillProperties(Map properties) { properties.put(Schema.ABSTRACT, Modifier.isAbstract(javaModifiers)); properties.put(Schema.STATIC, Modifier.isStatic(javaModifiers)); properties.put(Schema.GLOBAL, getNode().getModifiers().has(ModifierTypeInfos.GLOBAL)); + properties.put(Schema.VIRTUAL, getNode().getModifiers().has(ModifierTypeInfos.VIRTUAL)); + properties.put(Schema.OVERRIDE, getNode().getModifiers().has(ModifierTypeInfos.OVERRIDE)); } } diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index 8e865d78f..f5b011cd1 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -48,6 +48,7 @@ public class Schema { public static final String NAMES = "Names"; public static final String NAMESPACE_ACCESSIBLE = "NamespaceAccessible"; public static final String OPERATOR = "Operator"; + public static final String OVERRIDE = "Override"; public static final String REFERENCE_TYPE = "ReferenceType"; public static final String REMOTE_ACTION = "RemoteAction"; public static final String RETURN_TYPE = "ReturnType"; @@ -60,6 +61,7 @@ public class Schema { public static final String TYPE_REF = "TypeRef"; public static final String VALUE = "Value"; + public static final String VIRTUAL = "Virtual"; public static final String QUERY = "Query"; public static final class JorjeNodeType { From e0416ac3b18626744f2f0f1b936a0eb70881e7e6 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 11 Jul 2022 16:15:25 -0700 Subject: [PATCH 28/34] Adding error message for unreachable code --- .../config/UserFacingErrorMessages.java | 14 +++++++++++ .../build/AbstractApexVertexBuilder.java | 8 +------ .../ApexStandardLibraryVertexBuilder.java | 3 +-- .../salesforce/graph/build/GremlinUtil.java | 24 +++++++++++++++++++ .../graph/build/MethodPathBuilderVisitor.java | 12 ++++++++-- .../graph/build/MethodPathBuilderTest.java | 22 +++++++++++++++++ 6 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 sfge/src/main/java/com/salesforce/config/UserFacingErrorMessages.java diff --git a/sfge/src/main/java/com/salesforce/config/UserFacingErrorMessages.java b/sfge/src/main/java/com/salesforce/config/UserFacingErrorMessages.java new file mode 100644 index 000000000..7d94730d4 --- /dev/null +++ b/sfge/src/main/java/com/salesforce/config/UserFacingErrorMessages.java @@ -0,0 +1,14 @@ +package com.salesforce.config; + +/** + * Contains error message constants that will be displayed to users. TODO: move all other + * user-facing messages here. + */ +public final class UserFacingErrorMessages { + + /** UserActionException * */ + + // format: filename,defined type, line number + public static final String UNREACHABLE_CODE = + "Please remove unreachable code to proceed with analysis: %s,%s:%d"; +} diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index ddf737c4b..30ee78a57 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -1,12 +1,10 @@ package com.salesforce.graph.build; -import com.google.common.collect.ImmutableSet; import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.apex.jorje.JorjeNode; import com.salesforce.collections.CollectionUtil; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -27,10 +25,6 @@ @SuppressWarnings("PMD.AbstractClassWithoutAbstractMethod") abstract class AbstractApexVertexBuilder { private static final Logger LOGGER = LogManager.getLogger(AbstractApexVertexBuilder.class); - protected static final Set ROOT_VERTICES = - ImmutableSet.builder() - .addAll(Arrays.asList(ASTConstants.NodeType.ROOT_VERTICES)) - .build(); protected final GraphTraversalSource g; protected AbstractApexVertexBuilder(GraphTraversalSource g) { @@ -66,7 +60,7 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { // The engine assumes that only certain types of nodes can be roots. We need to enforce // that assumption and // fail noisily if it's violated. - if (!ROOT_VERTICES.contains(vNode.label())) { + if (!GremlinUtil.ROOT_VERTICES.contains(vNode.label())) { throw new UnexpectedException("Unexpected root vertex of type " + vNode.label()); } vNode.property(Schema.FILE_NAME, fileName); diff --git a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java index 8486cd02f..141ad1e71 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/ApexStandardLibraryVertexBuilder.java @@ -51,8 +51,7 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) { Object result = value; final boolean isRootVertexNameName = - AbstractApexVertexBuilder.ROOT_VERTICES.contains(node.getLabel()) - && key.equals(Schema.NAME); + GremlinUtil.ROOT_VERTICES.contains(node.getLabel()) && key.equals(Schema.NAME); if (isRootVertexNameName || key.equals(Schema.DEFINING_TYPE)) { result = getFullName(currentPackage, (String) value); if (LOGGER.isTraceEnabled()) { diff --git a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java index a34fc84eb..ffbfdec16 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java @@ -1,14 +1,19 @@ package com.salesforce.graph.build; +import com.google.common.collect.ImmutableSet; +import com.salesforce.apex.jorje.ASTConstants; import com.salesforce.exception.UnexpectedException; import com.salesforce.graph.Schema; +import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.apache.tinkerpop.gremlin.process.traversal.Order; import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -17,6 +22,11 @@ * and labels. */ public final class GremlinUtil { + protected static final Set ROOT_VERTICES = + ImmutableSet.builder() + .addAll(Arrays.asList(ASTConstants.NodeType.ROOT_VERTICES)) + .build(); + public static Long getId(Map vertexProperties) { return (Long) vertexProperties.get(T.id); } @@ -105,5 +115,19 @@ public static Optional getParent(GraphTraversalSource g, Vertex vertex) } } + public static String getFileName(GraphTraversalSource g, Vertex vertex) { + if (ROOT_VERTICES.contains(vertex.label())) { + return vertex.value(Schema.FILE_NAME); + } + List verticesWithFileName = + g.V(vertex).repeat(__.in(Schema.CHILD)).until(__.has(Schema.FILE_NAME)).toList(); + + if (verticesWithFileName.isEmpty()) { + return "UNKNOWN_FILENAME"; + } + + return verticesWithFileName.get(0).value(Schema.FILE_NAME); + } + private GremlinUtil() {} } diff --git a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java index 5d7ba6c86..247b811e5 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java +++ b/sfge/src/main/java/com/salesforce/graph/build/MethodPathBuilderVisitor.java @@ -2,8 +2,10 @@ import com.google.common.collect.ImmutableSet; import com.salesforce.apex.jorje.ASTConstants.NodeType; +import com.salesforce.config.UserFacingErrorMessages; import com.salesforce.exception.TodoException; import com.salesforce.exception.UnexpectedException; +import com.salesforce.exception.UserActionException; import com.salesforce.graph.Schema; import com.salesforce.graph.vertex.SFVertexFactory; import java.util.ArrayList; @@ -165,7 +167,7 @@ private void _visit(Vertex vertex, Vertex parent, boolean lastChild) { currentInnerScope.pop(); nextVertexInOuterScope.pop(); } - } catch (TodoException | UnexpectedException ex) { + } catch (UserActionException | TodoException | UnexpectedException ex) { throw ex; } catch (Exception ex) { throw new UnexpectedException(g, vertex, ex); @@ -592,7 +594,13 @@ private void addEdgeFromPreviousSibling(Vertex vertex) { private void addEdge(String name, Vertex from, Vertex to) { if (NodeType.TERMINAL_VERTEX_LABELS.contains(from.label())) { - throw new UnexpectedException("Vertex is terminal. vertex=" + vertexToString(from)); + // Ask user to fix unreachable code + throw new UserActionException( + String.format( + UserFacingErrorMessages.UNREACHABLE_CODE, + GremlinUtil.getFileName(g, to), + to.value(Schema.DEFINING_TYPE), + to.value(Schema.BEGIN_LINE))); } if (LOGGER.isTraceEnabled()) { diff --git a/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java index ddef8934d..cbddbcc22 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java @@ -3,16 +3,19 @@ import static org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__.has; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.core.IsEqual.equalTo; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.core.IsNot.not; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; import com.salesforce.TestUtil; import com.salesforce.apex.jorje.ASTConstants.NodeType; +import com.salesforce.exception.UserActionException; import com.salesforce.graph.ApexPath; import com.salesforce.graph.Schema; import com.salesforce.graph.ops.ApexPathUtil; @@ -2736,6 +2739,25 @@ public void testMethodSwitchStatement() { MatcherAssert.assertThat(paths, hasSize(equalTo(3))); } + @Test + public void testUnreachableCodeDetection() { + final String sourceCode = + "public class MyClass {\n" + + " String doSomething() {\n" + + " return 'hello';\n" + + " System.debug('This line is unreachable');\n" + + " }\n" + + "}\n"; + + UserActionException thrown = + assertThrows( + UserActionException.class, + () -> GraphBuildTestUtil.buildGraph(g, sourceCode), + "UserActionException should've been thrown before this point"); + + MatcherAssert.assertThat(thrown.getMessage(), containsString("MyClass:4")); + } + /** * Asserts that the number of {@link Schema#CFG_PATH} edges matches the number of vertex pairs * provided and that the edges match. From 96e597da4d0aa484467f9048fc15b32fb5e1bcbb Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Wed, 13 Jul 2022 11:47:42 -0700 Subject: [PATCH 29/34] Upgrading to PMD 6.47.0 --- pmd-cataloger/build.gradle.kts | 2 +- src/Constants.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pmd-cataloger/build.gradle.kts b/pmd-cataloger/build.gradle.kts index 1c6a061cc..966b43f10 100644 --- a/pmd-cataloger/build.gradle.kts +++ b/pmd-cataloger/build.gradle.kts @@ -9,7 +9,7 @@ group = "sfdx" version = "1.0" val distDir = "$buildDir/../../dist" -val pmdVersion = "6.45.0" +val pmdVersion = "6.47.0" val pmdFile = "pmd-bin-$pmdVersion.zip" val pmdUrl = "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${pmdVersion}/${pmdFile}" val skippableJarRegexes = setOf("""^common_[\d\.-]*\.jar""".toRegex(), diff --git a/src/Constants.ts b/src/Constants.ts index e9f153217..2481a2b4a 100644 --- a/src/Constants.ts +++ b/src/Constants.ts @@ -1,7 +1,7 @@ import os = require('os'); import path = require('path'); -export const PMD_VERSION = '6.45.0'; +export const PMD_VERSION = '6.47.0'; export const SFGE_VERSION = '1.0.1-pilot'; export const CATALOG_FILE = 'Catalog.json'; export const CUSTOM_PATHS_FILE = 'CustomPaths.json'; From bc2758628366e28b37219aec5ba108edbe46d9a0 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 13 Jul 2022 16:09:51 -0500 Subject: [PATCH 30/34] @W-11397711@: Added banner requesting that people take our survey. --- messages/common.js | 3 +++ src/commands/scanner/rule/add.ts | 3 ++- src/commands/scanner/rule/describe.ts | 2 ++ src/commands/scanner/rule/list.ts | 2 ++ src/commands/scanner/rule/remove.ts | 2 ++ src/lib/ScannerRunCommand.ts | 2 ++ test/commands/scanner/rule/list.test.ts | 7 ++++--- test/commands/scanner/run.severity.test.ts | 14 ++++++++------ test/commands/scanner/run.test.ts | 4 ++-- 9 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 messages/common.js diff --git a/messages/common.js b/messages/common.js new file mode 100644 index 000000000..46571ef4f --- /dev/null +++ b/messages/common.js @@ -0,0 +1,3 @@ +module.exports = { + FEEDBACK_SURVEY_BANNER: `What do you think of Salesforce Code Analyzer? Take our feedback survey at https://research.net/r/SalesforceCA.` +}; diff --git a/src/commands/scanner/rule/add.ts b/src/commands/scanner/rule/add.ts index 54e76c231..b4e34515b 100644 --- a/src/commands/scanner/rule/add.ts +++ b/src/commands/scanner/rule/add.ts @@ -13,7 +13,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'add'); - +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); export default class Add extends SfdxCommand { @@ -41,6 +41,7 @@ export default class Add extends SfdxCommand { public async run(): Promise { this.validateFlags(); + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const language = this.flags.language as string; const paths = this.resolvePaths(); diff --git a/src/commands/scanner/rule/describe.ts b/src/commands/scanner/rule/describe.ts index 1e2557305..c5dd74f6a 100644 --- a/src/commands/scanner/rule/describe.ts +++ b/src/commands/scanner/rule/describe.ts @@ -12,6 +12,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'describe'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); type DescribeStyledRule = Rule & { enabled: boolean; @@ -41,6 +42,7 @@ export default class Describe extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const ruleFilters = this.buildRuleFilters(); // It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can // allow to boil over. diff --git a/src/commands/scanner/rule/list.ts b/src/commands/scanner/rule/list.ts index ffe4ddc84..ba5406632 100644 --- a/src/commands/scanner/rule/list.ts +++ b/src/commands/scanner/rule/list.ts @@ -13,6 +13,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'list'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); const columns = [messages.getMessage('columnNames.name'), messages.getMessage('columnNames.languages'), messages.getMessage('columnNames.categories'), @@ -64,6 +65,7 @@ export default class List extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const ruleFilters = this.buildRuleFilters(); // It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can // allow to boil over. diff --git a/src/commands/scanner/rule/remove.ts b/src/commands/scanner/rule/remove.ts index b87f712ec..f121782de 100644 --- a/src/commands/scanner/rule/remove.ts +++ b/src/commands/scanner/rule/remove.ts @@ -15,6 +15,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'remove'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); export default class Remove extends ScannerCommand { // These determine what's displayed when the --help/-h flag is supplied. @@ -44,6 +45,7 @@ export default class Remove extends ScannerCommand { }; public async run(): Promise { + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); // Step 1: Validate our input. this.validateFlags(); diff --git a/src/lib/ScannerRunCommand.ts b/src/lib/ScannerRunCommand.ts index 4e4fb69e3..6cc092fee 100644 --- a/src/lib/ScannerRunCommand.ts +++ b/src/lib/ScannerRunCommand.ts @@ -14,6 +14,7 @@ Messages.importMessagesDirectory(__dirname); // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core, // or any library that is using the messages framework can also be loaded this way. const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'run'); +const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common'); // This code is used for internal errors. export const INTERNAL_ERROR_CODE = 1; @@ -22,6 +23,7 @@ export abstract class ScannerRunCommand extends ScannerCommand { public async run(): Promise { // First, do any validations that can't be handled with out-of-the-box stuff. await this.validateFlags(); + this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); // If severity-threshold is used, that implicitly normalizes the severity. const normalizeSeverity: boolean = (this.flags['normalize-severity'] || this.flags['severity-threshold']) as boolean; diff --git a/test/commands/scanner/rule/list.test.ts b/test/commands/scanner/rule/list.test.ts index 3bfc3ef50..7330f892d 100644 --- a/test/commands/scanner/rule/list.test.ts +++ b/test/commands/scanner/rule/list.test.ts @@ -54,11 +54,12 @@ describe('scanner:rule:list', () => { .it('All rules for enabled engines are returned', async ctx => { const totalRuleCount = await getRulesFilteredByCategoryCount(false); - // Split the output table by newline and throw out the first two rows, since they just contain header information. That + // Split the output table by newline and throw out the first three rows, since they just contain header information. That // should leave us with the actual data. const rows = ctx.stdout.trim().split('\n'); rows.shift(); rows.shift(); + rows.shift(); expect(rows).to.have.lengthOf(totalRuleCount, 'All rules should have been returned'); }); @@ -305,9 +306,9 @@ describe('scanner:rule:list', () => { setupCommandTest .command(['scanner:rule:list', '--category', 'Beebleborp']) .it('Without --json flag, an empty table is printed', ctx => { - // Split the result by newline, and make sure there are two rows. + // Split the result by newline, and make sure there are three rows. const rows = ctx.stdout.trim().split('\n'); - expect(rows).to.have.lengthOf(2, 'Only the header rows should have been printed'); + expect(rows).to.have.lengthOf(3, 'Only the header rows should have been printed'); }); setupCommandTest diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index 6c9384dc4..cddfaf993 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/test/commands/scanner/run.severity.test.ts @@ -31,8 +31,8 @@ describe('scanner:run', function () { '--severity-threshold', '1' ]) .it('When no violations are found equal to or greater than flag value, no error is thrown', ctx => { - - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); // check that test file still has severities of 3 for (let i=0; i { - - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); // check that test file still has severities of 3 for (let i=0; i { - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); for (let i=0; i { - const output = JSON.parse(ctx.stdout); + // The first line is a header. Strip that away, and parse the rest as JSON. + const output = JSON.parse(ctx.stdout.split('\n')[1]); for (let i=0; i { - // Split the output by newline characters and throw away the first entry, so we're left with just the rows. - validateCsvOutput(ctx.stdout, false); + // Throw away the first line of output, since it's a banner. + validateCsvOutput(ctx.stdout.slice(ctx.stdout.indexOf('\n')), false); }); setupCommandTest From ddeb2e5ec955b9ba98e52de3f2d35f06ffd48c65 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Thu, 14 Jul 2022 13:40:48 -0500 Subject: [PATCH 31/34] @W-11397711@: Integrated feedback from code review. --- test/TestUtils.ts | 9 ++- test/commands/scanner/rule/list.test.ts | 15 +++-- test/commands/scanner/run.severity.test.ts | 74 +++++++++++----------- test/commands/scanner/run.test.ts | 6 +- 4 files changed, 56 insertions(+), 48 deletions(-) diff --git a/test/TestUtils.ts b/test/TestUtils.ts index b50b3770d..06da598c4 100644 --- a/test/TestUtils.ts +++ b/test/TestUtils.ts @@ -1,6 +1,7 @@ import fs = require('fs'); import path = require('path'); -import { test } from '@salesforce/command/lib/test'; +import { test, expect } from '@salesforce/command/lib/test'; +import * as CommonMessages from '../messages/common.js'; import * as TestOverrides from './test-related-lib/TestOverrides'; import Sinon = require('sinon'); import LocalCatalog from '../src/lib/services/LocalCatalog'; @@ -15,6 +16,12 @@ export function prettyPrint(obj: any): string { return JSON.stringify(obj, null, 2); } +export function stripExtraneousOutput(stdout: string): string { + const splitOutput = stdout.split('\n'); + expect(splitOutput[0]).to.equal('=== ' + CommonMessages.FEEDBACK_SURVEY_BANNER); + return splitOutput.slice(1).join('\n'); +} + export function stubCatalogFixture(): void { // Make sure all catalogs exist where they're supposed to. if (!fs.existsSync(CATALOG_FIXTURE_PATH)) { diff --git a/test/commands/scanner/rule/list.test.ts b/test/commands/scanner/rule/list.test.ts index 7330f892d..973782dfd 100644 --- a/test/commands/scanner/rule/list.test.ts +++ b/test/commands/scanner/rule/list.test.ts @@ -1,5 +1,5 @@ import {expect} from '@salesforce/command/lib/test'; -import {setupCommandTest} from '../../../TestUtils'; +import {setupCommandTest, stripExtraneousOutput} from '../../../TestUtils'; import {Rule} from '../../../../src/types'; import {CATALOG_FILE, ENGINE} from '../../../../src/Constants'; import fs = require('fs'); @@ -54,10 +54,10 @@ describe('scanner:rule:list', () => { .it('All rules for enabled engines are returned', async ctx => { const totalRuleCount = await getRulesFilteredByCategoryCount(false); - // Split the output table by newline and throw out the first three rows, since they just contain header information. That + // Split the output table by newline and throw out the first two rows, since they just contain header information. That // should leave us with the actual data. - const rows = ctx.stdout.trim().split('\n'); - rows.shift(); + const output = stripExtraneousOutput(ctx.stdout); + const rows = output.trim().split('\n'); rows.shift(); rows.shift(); expect(rows).to.have.lengthOf(totalRuleCount, 'All rules should have been returned'); @@ -306,9 +306,10 @@ describe('scanner:rule:list', () => { setupCommandTest .command(['scanner:rule:list', '--category', 'Beebleborp']) .it('Without --json flag, an empty table is printed', ctx => { - // Split the result by newline, and make sure there are three rows. - const rows = ctx.stdout.trim().split('\n'); - expect(rows).to.have.lengthOf(3, 'Only the header rows should have been printed'); + // Split the result by newline, and make sure there are two rows. + const output = stripExtraneousOutput(ctx.stdout); + const rows = output.trim().split('\n'); + expect(rows).to.have.lengthOf(2, 'Only the header rows should have been printed'); }); setupCommandTest diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index cddfaf993..4e411acff 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/test/commands/scanner/run.severity.test.ts @@ -1,5 +1,5 @@ import {expect} from '@salesforce/command/lib/test'; -import {setupCommandTest} from '../../TestUtils'; +import {setupCommandTest, stripExtraneousOutput} from '../../TestUtils'; import {Messages} from '@salesforce/core'; import path = require('path'); @@ -31,12 +31,12 @@ describe('scanner:run', function () { '--severity-threshold', '1' ]) .it('When no violations are found equal to or greater than flag value, no error is thrown', ctx => { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); // check that test file still has severities of 3 - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); // check that test file still has severities of 3 - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); - - for (let i=0; i { - // The first line is a header. Strip that away, and parse the rest as JSON. - const output = JSON.parse(ctx.stdout.split('\n')[1]); + const output = stripExtraneousOutput(ctx.stdout); + const outputJson = JSON.parse(output); - for (let i=0; i { - // Throw away the first line of output, since it's a banner. - validateCsvOutput(ctx.stdout.slice(ctx.stdout.indexOf('\n')), false); + const output = stripExtraneousOutput(ctx.stdout); + validateCsvOutput(output, false); }); setupCommandTest From c478ef5df624be539874a1a249ca379659ed0d98 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 18 Jul 2022 14:02:35 -0500 Subject: [PATCH 32/34] @W-11397711@: Updated text to match feedback from PR. --- messages/common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/messages/common.js b/messages/common.js index 46571ef4f..c7f46e1e0 100644 --- a/messages/common.js +++ b/messages/common.js @@ -1,3 +1,3 @@ module.exports = { - FEEDBACK_SURVEY_BANNER: `What do you think of Salesforce Code Analyzer? Take our feedback survey at https://research.net/r/SalesforceCA.` + FEEDBACK_SURVEY_BANNER: `We're constantly improving Salesforce Code Analayzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA.` }; From 689236ae6f5fdd7235c0de77cd0f569f56ac6c2c Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Mon, 18 Jul 2022 11:36:41 -0500 Subject: [PATCH 33/34] @W-11445992@: Update version number in preparation for 3.3.0 release. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c29919e27..e35f376fd 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/sfdx-scanner", "description": "Static code scanner that applies quality and security rules to Apex code, and provides feedback.", - "version": "3.2.0", + "version": "3.3.0", "author": "ISV SWAT", "bugs": "https://github.com/forcedotcom/sfdx-scanner/issues", "dependencies": { From 97782e3231e08a9eecf97ca960a56ea02be4a497 Mon Sep 17 00:00:00 2001 From: Roopa Mohan Date: Mon, 18 Jul 2022 16:15:20 -0700 Subject: [PATCH 34/34] @W-11397711@: Fixing minor typo --- messages/common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/messages/common.js b/messages/common.js index c7f46e1e0..be555a676 100644 --- a/messages/common.js +++ b/messages/common.js @@ -1,3 +1,3 @@ module.exports = { - FEEDBACK_SURVEY_BANNER: `We're constantly improving Salesforce Code Analayzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA.` + FEEDBACK_SURVEY_BANNER: `We're constantly improving Salesforce Code Analyzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA.` };