Crash when handing dynamic constant assignments

This code crashes Sorbet, and block us from type-checking the monolith:

def m
  C = 1
end

It should raise a SyntaxError at runtime, but it shouldn't crash Sorbet statically.

stacktrace

``` + exec test/pipeline_test_runner --single_test=test/prism_regression/dynamic_constant_assignment.rb --parser=prism [doctest] doctest version is "2.4.9" [doctest] run with "--help" for options Parsing with prism [2024-10-22 17:02:41.276] [fatalFallback] [error] Exception::raise(): ast/verifier/Verifier.cc:29 enforced condition methodDepth == 0 has failed: Found constant definition inside method definition [2024-10-22 17:02:41.705] [fatalFallback] [error] Backtrace: sorbet::ast::VerifierWalker::postTransformAssign(sorbet::core::Context, sorbet::ast::Assign const&) (in pipeline_test_runner) + 272 decltype(auto) sorbet::ast::MapFunctions<(sorbet::ast::TreeMapKind)2>::CALL_MEMBER_impl_postTransformAssign::call(sorbet::ast::VerifierWalker&, sorbet::core::Context&, sorbet::ast::Assign const&) (in pipeline_test_runner) + 64 sorbet::ast::TreeMapper::mapAssign(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 172 sorbet::ast::TreeMapper::mapIt(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 1524 sorbet::ast::TreeMapper::mapMethodDef(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 412 sorbet::ast::TreeMapper::mapIt(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 344 sorbet::ast::TreeMapper::mapClassDef(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 228 sorbet::ast::TreeMapper::mapIt(sorbet::ast::ExpressionPtr const&, sorbet::core::Context) (in pipeline_test_runner) + 288 void sorbet::ast::ConstTreeWalk::apply(sorbet::core::Context, sorbet::ast::VerifierWalker&, sorbet::ast::ExpressionPtr const&) (in pipeline_test_runner) + 84 sorbet::ast::Verifier::run(sorbet::core::Context, sorbet::ast::ExpressionPtr) (in pipeline_test_runner) + 72 sorbet::ast::desugar::node2Tree(sorbet::core::MutableContext, std::__1::unique_ptr>, bool) (in pipeline_test_runner) + 444 sorbet::test::index(std::__1::unique_ptr>&, absl::lts_20240722::Span, sorbet::test::ExpectationHandler&, sorbet::test::Expectations&) (in pipeline_test_runner) + 4608 sorbet::test::DOCTEST_ANON_FUNC_10() (in pipeline_test_runner) + 3996 doctest::Context::run() (in pipeline_test_runner) + 4388 main (in pipeline_test_runner) + 1772 start (in dyld) + 2840 =============================================================================== test/pipeline_test_runner.cc:347: TEST CASE: PerPhaseTest test/pipeline_test_runner.cc:347: ERROR: test case THREW exception: ast/verifier/Verifier.cc:29 enforced condition methodDepth == 0 has failed: Found constant definition inside method definition =============================================================================== [doctest] test cases: 1 | 0 passed | 1 failed | 0 skipped [doctest] assertions: 8 | 8 passed | 0 failed | [doctest] Status: FAILURE! ================================================================================ Target //test:test_PosTests/prism_regression/dynamic_constant_assignment up-to-date: bazel-bin/test/test_PosTests/prism_regression/dynamic_constant_assignment ```

Sorbet's handling

Sorbet's parser handles by diagnosing a syntax error early on, and rewriting the dynamic constant assignment into a fake write to a dummy local variable called dynamic-const-assign (core::Names::dynamicConstAssign()).

https://github.com/Shopify/sorbet/blob/97d7fffcb75facccf75306efe8d3850be5bab6d3/parser/Builder.cc#L348-L354

This is the expected parse tree:

DefMethod {
  name = <U m>
  args = NULL
  body = Assign {
    lhs = LVarLhs {
      name = <U <dynamic-const-assign>>
    }
    rhs = Integer {
      val = "1"
    }
  }
}

Thus, by the time this tree reaches the Verifier, there's no more dynamic constant assignment, and this ENFORCE doesn't trip.

https://github.com/Shopify/sorbet/blob/97d7fffcb75facccf75306efe8d3850be5bab6d3/ast/verifier/Verifier.cc#L27-L31

How Prism models this

Prism doesn't model this in the AST itself, but in the Prism::Result#errors field.

Prism.parse "def x; C = 123; end"
#<Prism::ParseResult:0x0000000149c796c0 @value=@ ProgramNode (location: (1,0)-(1,19))
├── flags: ∅
├── locals: []
└── statements:
    @ StatementsNode (location: (1,0)-(1,19))
    ├── flags: ∅
    └── body: (length: 1)
        └── @ DefNode (location: (1,0)-(1,19))
            ├── flags: newline
            ├── name: :x
            ├── name_loc: (1,4)-(1,5) = "x"
            ├── receiver: ∅
            ├── parameters: ∅
            ├── body:
            │   @ StatementsNode (location: (1,7)-(1,14))
            │   ├── flags: ∅
            │   └── body: (length: 1)
            │       └── @ ConstantWriteNode (location: (1,7)-(1,14))
            │           ├── flags: newline
            │           ├── name: :C
            │           ├── name_loc: (1,7)-(1,8) = "C"
            │           ├── value:
            │           │   @ IntegerNode (location: (1,11)-(1,14))
            │           │   ├── flags: static_literal, decimal
            │           │   └── value: 123
            │           └── operator_loc: (1,9)-(1,10) = "="
            ├── locals: []
            ├── def_keyword_loc: (1,0)-(1,3) = "def"
            ├── operator_loc: ∅
            ├── lparen_loc: ∅
            ├── rparen_loc: ∅
            ├── equal_loc: ∅
            └── end_keyword_loc: (1,16)-(1,19) = "end"
, @comments=[], @magic_comments=[], @data_loc=nil, @errors=[#<Prism::ParseError @type=:write_target_in_method @message="dynamic constant assignment" @location=#<Prism::Location @start_offset=7 @length=7 start_line=1> @level=:syntax>], @warnings=[], @source=#<Prism::ASCIISource:0x00000001491d9be8 @source="def x; C = 123; end", @start_line=1, @offsets=[0]>>

Solution ideas

1. Add a flag to Prism's `ConstantWriteNode` and `ConstantTargetNode`

We could define new a new flag that's tagged on the relevant node:

/**
 * Flags for constant write nodes.
 */
typedef enum pm_integer_base_flags {
    /** This constant write will cause "SyntaxError: dynamic constant assignment" */
    PM_CONSTANT_WRITE_FLAGS_DYNAMIC_ASSIGNMENT = 4,
}  pm_constant_write_flags_t

/**
 * Flags for constant target nodes.
 */
typedef enum pm_integer_base_flags {
    /** This constant target will cause "SyntaxError: dynamic constant assignment" */
    PM_CONSTANT_TARGET_FLAGS_DYNAMIC_ASSIGNMENT = 4,
}  pm_constant_target_flags_t

This would let Sorbet's translator inspect the node directly for this information, without needing any other parser/lexing context.

2. Track lexical scopes in `Prism::Translator`

There's no direct way to infer the currently lexical scope for a Prism AST node, because the nodes only have one-way references "down" the tree, and no upwards references to their parent.

So if we want the lexical information, we'll need to track it ourselves. We can add state to Prism::Translator that we update any time we enter/exit methods and classes, to keep track of which lexical scope we're in.

3. Correlate to the Parser errors

We can pass the Parser errors array to the Translator. Every time we do a constant assignment, we can search the errors (by the location information) to see if that assignment is a dynamic constant assignment.

Shopify / sorbet