fix(semantic): connect test expression of for statements to the cfg.

[rzvxa]

I don't know if it is correct or not, Fixes my issues with dangling cfg nodes created in for statements. #3071

[rzvxa]

• feat(linter/react): add the rules_of_hooks rule. #3071
• improvement(semantic/cfg): add explicit break block element. #3223
• fix(semantic): add cfg nodes for ConditionalExpressions. #3127
• fix(semantic): connect test expression of for statements to the cfg. #3122 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite

[codspeed-hq]

CodSpeed Performance Report

Merging #3122 will improve performances by 18.42%

_{Comparing 04-28-fix_connect_test_expression_of_for_statements_to_the_cfg (2a8148a) with main (9525653)}

Summary

⚡ 1 improvements
✅ 26 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	04-28-fix_connect_test_expression_of_for_statements_to_the_cfg	Change
⚡	semantic[RadixUIAdoptionSection.jsx]	411 µs	347.1 µs	+18.42%

[rzvxa]

Given this block of code:

            function t1() {
              const res = [];
              const additionalCond = true;
              for (let i = 0; i !== 10 && additionalCond; ++i ) {
                res.push(i);
              }
              React.useLayoutEffect(() => {});
            }

the cfg used to look like this:

                                  (0) Program--------------------------- (9) Dummy cfg
                                        |
                                        |
                                        V
                                  (1) Function
                                        |
                                        |
                                        V
   |------------------------->(2) Logical Expression
   |                                  /       \--------------------------------------------------------------------------------------------------
   |                                 /         \                                                                  \                              \
   |                                /           \                                                                  \                              \
   |                               /             \                                                                  \                              \
   |                              /               \                                                                  \                              \
   |                             /                 \                                                                  \                              \
   |                            V                   V                                                                  \                              \
   |      (3) Ident(`additionalCond`)------------>(4) Dummy cfg Created by the Binary Expression or others              \                              \
   |                                                                                                                     \                              \
   |                                                                                                                      \                              \
   |                                                                                                                       \                              \
   |                                                                                                                        \                              \
   |                                                                                                                         \                              \
   |                                                                                                                          \                              \
   |                                                                                                                          V                               V
   ---------------(5) UpdateExpression<------------------------------------------------------------------------------(6) BlockStatement            (7) ExpressionStatement
                                                                                                                                                               \
                                                                                                                                                                \
                                                                                                                                                                 \
                                                                                                                                                           (8) ArrowFunctionExpression

With this change, it would look like this:

                                  (0) Program--------------------------- (9) Dummy cfg
                                        |
                                        |
                                        V
                                  (1) Function
                                        |
                                        |
                                        V
   |------------------------->(2) Logical Expression
   |                                  /       \
   |                                 /         \
   |                                /           \
   |                               /             \
   |                              /               \
   |                             /                 \
   |                            V                   V
   |      (3) Ident(`additionalCond`)------------>(4) Dummy cfg Created by the Binary Expression or others
   |                                                            /              \
   |                                                           /                \
   |                                                          /                  \
   |                                                         /                    \
   |                                                        /                      \
   |                                                       /                        \
   |                                                      V                         V
   ---------------(5) UpdateExpression<----------(6) BlockStatement            (7) ExpressionStatement
                                                                                            \
                                                                                             \
                                                                                              \
                                                                                        (8) ArrowFunctionExpression

I'm unsure if this is illegal, Or would break anything in the type system, etc. But it would simplify the cfg traversal since every node has a follower node and we can check if any 2 nodes share a subtree by checking both are connected to the same parent, and If they have a connection they are also always on the same subtree.

@Boshen Please give it a look I'm not really familiar with what a cfg should look like so it might be just wrong.

What I'm looking for is simplifying checks but I can always find another way to solve it. Right now I'm implementing #3071 with off-the-shelf algorithms so if this PR doesn't work I can just write a specialized one.

[rzvxa]

Please don't merge it yet, Since if it gets approved there are other places that need the same treatment

[Boshen]

There's also a graphical output from https://github.com/oxc-project/oxc/blob/main/crates/oxc_semantic/examples/cfg.rs

Your change seems to be more correct than the previous one.

This can be verifed with the biome cfg,

There's a loop and a exit edge, which is your graph after the change.

[rzvxa]

There's also a graphical output from https://github.com/oxc-project/oxc/blob/main/crates/oxc_semantic/examples/cfg.rs

Your change seems to be more correct than the previous one.

This can be verifed with the biome cfg,

There's a loop and a exit edge, which is your graph after the change.

Oh, I didn't know about either of these visualization tools, They sure would've come in handy last day when I was trying to figure out this.

I'll check the biome control flow and compare it with ours to find other edge cases that can be improved.

PS: I might mark this one "ready" a bit sooner than the rest since it is a dependency of the react hooks rule

[Boshen]

Merge activity

• May 10, 10:11 AM EDT: @Boshen started a stack merge that includes this pull request via Graphite.
• May 10, 10:11 AM EDT: @Boshen merged this pull request with Graphite.