@W-11261233@ Handle static blocks in classes #740

rmohan20 · 2022-06-30T18:03:47Z

As a general stop-gap for cryptic errors, this PR catches all UnexpectedExceptions at Main and displays a more friendly message to users while adding the actual stacktrace to logs.

Static blocks are replaced by new synthetic methods that represent them. These synthetic methods are invoked by another top-level synthetic method. More detailed example/explanation here.
While initializing a class and creating its static fields, we now invoke the new top-level method to execute the static blocks.

Even though this PR leaves us in a better position than where we were (throwing illegible errors when static blocks were encountered), there are some open issues to handle in future:

Static fields set in classes along with static blocks throw an exception while setting up class static scope.
When multiple static blocks are present, one of them gets invoked twice. The same problem happens when both class and its super class contain static blocks.

rmohan20 · 2022-06-30T18:04:42Z

sfge/src/main/java/com/salesforce/Main.java

            System.err.println(formatError(ex));
            return INTERNAL_ERROR;
+        } catch (UnexpectedException ex) {
+            LOGGER.error("Unexpected exception while loading graph", ex);


New catch-all for Unexpected exceptions so that the error message is less cryptic.

rmohan20 · 2022-06-30T18:07:02Z

sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java

-            g.addE(Schema.PARENT).from(vChild).to(vNode).iterate();
-            g.addE(Schema.CHILD).from(vNode).to(vChild).iterate();
+
+            // Handle static block if needed


If a block statement is found in the default <clint>() method, we know it's a static block. This redirection leads to the creation of a new synthetic static block method.

rmohan20 · 2022-06-30T18:08:49Z

sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java

        if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) {
            MethodPathBuilderVisitor.apply(g, vNode);
+        } else if (ROOT_VERTICES.contains(node.getLabel())) {
+            StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode);


If we are at the end of handling a root (class/interface/trigger etc) node, check if any synthetic static block methods were created earlier. If yes, add a synthetic static block invoker method to invoke the static block methods.

rmohan20 · 2022-06-30T18:09:19Z

sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java

    }
-
-    /** Add a property to the traversal, throwing an exception if any keys are duplicated. */
-    protected void addProperty(


Extracted to GremlinVertexUtil for reuse.

rmohan20 · 2022-06-30T18:09:58Z

sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java

        } else if (children.size() > 1) {
-            throw new UnexpectedException(children);
+            throw new UnexpectedException(
+                    "Did not expect more than one child node of type "


Adding more information to the exception.

rmohan20 · 2022-06-30T18:11:19Z

sfge/src/main/java/com/salesforce/graph/build/GremlinVertexUtil.java

+import org.apache.tinkerpop.gremlin.structure.Vertex;
+
+/** Handles common operations performed while creating vertices on Graph */
+public final class GremlinVertexUtil {


Refactored existing code into this util for core reuse.

rmohan20 · 2022-06-30T18:11:52Z

sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java

+ * <p>private static void StaticBlockInvoker() { SyntheticStaticBlock_1(); SyntheticStaticBlock_2();
+ * } }
+ */
+public final class StaticBlockUtil {


Most of the new logic is here.

rmohan20 · 2022-06-30T18:34:28Z

sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java

+            String definingType) {
+        // Create new synthetic method StaticBlockInvoker to invoke each synthetic static block
+        // method
+        final Vertex invokerMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next();


Artificially creating a method and its structure to invoke each static block method.

rmohan20 · 2022-06-30T18:35:59Z

sfge/src/main/java/com/salesforce/graph/ops/expander/ApexPathExpander.java

                    }
                    symbolProviderVisitor.popScope(classStaticScope);
                    classStaticScope.setState(AbstractClassScope.State.INITIALIZED);
+                    alreadyInitializedStaticClasses.add(fullClassName);


Now tracking classes that were already initialized.

rmohan20 · 2022-06-30T18:36:36Z

sfge/src/main/java/com/salesforce/graph/symbols/ClassStaticScope.java

        List<BaseSFVertex> vertices = new ArrayList<>();
        vertices.addAll(classStaticScope.getFields());
        vertices.addAll(getFieldDeclarations(g, classStaticScope.userClass));
+		vertices.addAll(getStaticBlocks(g, classStaticScope.userClass));


Adding calls to static blocks as another item needed to set a class's static scope.

rmohan20 · 2022-06-30T18:37:45Z

sfge/src/main/java/com/salesforce/graph/vertex/MethodVertex.java

        }
    }

+    public static final class StaticBlockVertex extends MethodVertex


Intermediary code that's not needed anymore. Will remove this.

rmohan20 · 2022-06-30T18:38:05Z

sfge/src/test/java/com/salesforce/graph/build/GraphBuildTestUtil.java

+import java.util.List;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+
+public class GraphBuildTestUtil {


Extracting existing methods for reusability

rmohan20 · 2022-06-30T18:38:53Z

sfge/src/test/java/com/salesforce/graph/build/MethodPathBuilderTest.java

                        + "}";

-        buildGraph(g, sourceCode);
+        GraphBuildTestUtil.buildGraph(g, sourceCode);


Changes from extracting methods to GraphBuiltTestUtil

rmohan20 · 2022-06-30T18:39:38Z

sfge/src/test/java/com/salesforce/graph/build/StaticBlockUtilTest.java

+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class StaticBlockUtilTest {


New tests to ascertain structure of synthetic methods.

rmohan20 · 2022-06-30T18:40:43Z

sfge/src/test/java/com/salesforce/graph/symbols/StaticCodeBlockInvocationTest.java

+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+
+public class StaticCodeBlockInvocationTest {


Tests to verify static block invocation while setting static scope of class. I've added and disabled test cases that need future work.

jfeingold35 · 2022-07-05T22:01:35Z

sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java

+
+            // Handle static block if needed
+            if (StaticBlockUtil.isStaticBlockStatement(node, child)) {
+                final Vertex parentVertexForChild =


It looks like what's happening is that we detect that the child is a static block, and if so we create our synthetic method block to hold it, make that a child of the current parent, and make the block statement a child of that. Am I reading that right?
I'd love some commenting about what is being done here, and why it was necessary.

Added more comments. Let me know if it explains sufficiently or I'll try expanding it some more.

jfeingold35 · 2022-07-05T22:02:51Z

sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java

        if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) {
            MethodPathBuilderVisitor.apply(g, vNode);
+        } else if (ROOT_VERTICES.contains(node.getLabel())) {
+            StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode);


Again, I'd love a comment explaining what's going on here.

jfeingold35 · 2022-07-05T22:04:57Z

sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java

+     * Creates a synthetic method vertex to represent a static code block
+     *
+     * @param g traversal graph
+     * @param clintVertex <clint>() method's vertex


It's clinit, not clint. It's a little thing, but it's in a bunch of places and it'll bug me if I don't mention it.

Thanks for the catch, I'll fix it.

Fixed everywhere

jfeingold35 · 2022-07-05T22:07:35Z

sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java

+ * static blocks are represented like this: class StaticBlockClass { private static void <clint>() {
+ * { System.debug("inside static block 1"); } { System.debug("inside static block 2"); } } }
+ *
+ * <p>Having multiple block statements inside a method breaks SFGE's makes handling code blocks in


This sentence seems like it's missing a bit?

Looks like google formatter mauled the documentation. Let me try fixing this.

jfeingold35 · 2022-07-05T22:09:09Z

sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java

+ *
+ * <p>private static void SyntheticStaticBlock_2() { System.debug("inside static block 2"); }
+ *
+ * <p>private static void StaticBlockInvoker() { SyntheticStaticBlock_1(); SyntheticStaticBlock_2();


Does now get treated as if it had invoked StaticBlockInvoker?

rmohan20 added 4 commits June 30, 2022 10:50

Add more information while handling error

d64825d

work in progress - static block recognition part

1dfce6a

Solution to handle static blocks in code - draft

d1eb1c5

Invoking static block from static class scope

81ba383

rmohan20 commented Jun 30, 2022

View reviewed changes

rmohan20 added 3 commits June 30, 2022 12:01

Removing intermediary code now unused

ecc2bdd

Adding child index to synthetic methods

a9017ba

Attempting to remove compilation error in CircleCI

89f7cd6

jfeingold35 reviewed Jul 5, 2022

View reviewed changes

Addressing PR feedback

d7fd360

jfeingold35 approved these changes Jul 6, 2022

View reviewed changes

jfeingold35 merged commit ddfed9e into dev-3 Jul 6, 2022

stephen-carter-at-sf deleted the rm/handleStaticBlock branch May 17, 2024 14:29

@W-11261233@ Handle static blocks in classes #740

@W-11261233@ Handle static blocks in classes #740

Uh oh!

Conversation

rmohan20 commented Jun 30, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants