Skip to content

Conversation

@thomasmarshall
Copy link

This fixes a crash when Prism parses a malformed sig as a multi target node:

sig,{params(x: Integer).void}

This is just an example that came out of the fuzzer, it's not specific to sigs, it's how Prism parses the comma in the malformed code:

@ MultiTargetNode (location: (1,0)-(1,29))
├── flags: newline
├── lefts: (length: 2)
│   ├── @ LocalVariableTargetNode (location: (1,0)-(1,3))
│   │   ├── flags: ∅
│   │   ├── name: :sig
│   │   └── depth: 0
│   └── @ HashNode (location: (1,4)-(1,29))
│       ├── flags: ∅
│       ├── opening_loc: (1,4)-(1,5) = "{"
│       ├── elements: (length: 1)
│       │   └── @ AssocNode (location: (1,5)-(1,28))
│       │       ├── flags: ∅
│       │       ├── key:
│       │       │   @ CallNode (location: (1,5)-(1,28))
│       │       │   ├── flags: ∅
│       │       │   ├── receiver:
│       │       │   │   @ CallNode (location: (1,5)-(1,23))
│       │       │   │   ├── flags: ignore_visibility
│       │       │   │   ├── receiver: ∅
│       │       │   │   ├── call_operator_loc: ∅
│       │       │   │   ├── name: :params
│       │       │   │   ├── message_loc: (1,5)-(1,11) = "params"
│       │       │   │   ├── opening_loc: (1,11)-(1,12) = "("
│       │       │   │   ├── arguments:
│       │       │   │   │   @ ArgumentsNode (location: (1,12)-(1,22))
│       │       │   │   │   ├── flags: contains_keywords
│       │       │   │   │   └── arguments: (length: 1)
│       │       │   │   │       └── @ KeywordHashNode (location: (1,12)-(1,22))
│       │       │   │   │           ├── flags: symbol_keys
│       │       │   │   │           └── elements: (length: 1)
│       │       │   │   │               └── @ AssocNode (location: (1,12)-(1,22))
│       │       │   │   │                   ├── flags: ∅
│       │       │   │   │                   ├── key:
│       │       │   │   │                   │   @ SymbolNode (location: (1,12)-(1,14))
│       │       │   │   │                   │   ├── flags: static_literal, forced_us_ascii_encoding
│       │       │   │   │                   │   ├── opening_loc: ∅
│       │       │   │   │                   │   ├── value_loc: (1,12)-(1,13) = "x"
│       │       │   │   │                   │   ├── closing_loc: (1,13)-(1,14) = ":"
│       │       │   │   │                   │   └── unescaped: "x"
│       │       │   │   │                   ├── value:
│       │       │   │   │                   │   @ ConstantReadNode (location: (1,15)-(1,22))
│       │       │   │   │                   │   ├── flags: ∅
│       │       │   │   │                   │   └── name: :Integer
│       │       │   │   │                   └── operator_loc: ∅
│       │       │   │   ├── closing_loc: (1,22)-(1,23) = ")"
│       │       │   │   └── block: ∅
│       │       │   ├── call_operator_loc: (1,23)-(1,24) = "."
│       │       │   ├── name: :void
│       │       │   ├── message_loc: (1,24)-(1,28) = "void"
│       │       │   ├── opening_loc: ∅
│       │       │   ├── arguments: ∅
│       │       │   ├── closing_loc: ∅
│       │       │   └── block: ∅
│       │       ├── value:
│       │       │   @ MissingNode (location: (1,28)-(1,28))
│       │       │   └── flags: ∅
│       │       └── operator_loc: (1,28)-(1,28) = ""
│       └── closing_loc: (1,28)-(1,29) = "}"
├── rest: ∅
├── rights: (length: 0)
├── lparen_loc: ∅
└── rparen_loc: ∅

Here, one of the lefts is a HashNode (the {params…} part), which is obviously not valid.

The fix I came up with, is to check each of the lefts and rights to ensure they are valid nodes, and otherwise just return an empty tree. I'm not 100% sure this is the best approach, and it's different to how the original parser handles it.

Original desugar tree:

class <emptyTree><<C <root>>> < (::<todo sym>)
  def f<<todo method>>(x, &<blk>)
    <emptyTree>
  end
end

Original errors:

test/prism_regression/invalid/malformed_sig.rb:4: unexpected token tRCURLY https://srb.help/2001
     4 |sig,{params(x: Integer).void}
                                    ^

test/prism_regression/invalid/malformed_sig.rb:5: The method f does not have a sig https://srb.help/7017
     5 |def f(x)
        ^^^^^^^^
  Autocorrect: Use -a to autocorrect
    test/prism_regression/invalid/malformed_sig.rb:5: Insert extend T::Sig
    sig { params(x: T.untyped).returns(NilClass) }
     5 |def f(x)
        ^
Errors: 2

Prism desugar tree:

class <emptyTree><<C <root>>> < (::<todo sym>)
  <emptyTree>

  def f<<todo method>>(x, &<blk>)
    <emptyTree>
  end
end

Prism errors:

test/prism_regression/invalid/malformed_sig.rb:4: expected a `=>` between the hash key and value https://srb.help/2001
     4 |sig,{params(x: Integer).void}
                                    ^

test/prism_regression/invalid/malformed_sig.rb:4: unexpected '}'; expected a value in the hash literal https://srb.help/2001
     4 |sig,{params(x: Integer).void}
                                    ^

test/prism_regression/invalid/malformed_sig.rb:4: unexpected write target https://srb.help/2001
     4 |sig,{params(x: Integer).void}
            ^^^^^^^^^^^^^^^^^^^^^^^^^

test/prism_regression/invalid/malformed_sig.rb:4: unexpected write target https://srb.help/2001
     4 |sig,{params(x: Integer).void}
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

test/prism_regression/invalid/malformed_sig.rb:5: unexpected 'def', expecting end-of-input https://srb.help/2001
     5 |def f(x)
        ^^^

test/prism_regression/invalid/malformed_sig.rb:5: The method f does not have a sig https://srb.help/7017
     5 |def f(x)
        ^^^^^^^^
  Autocorrect: Use -a to autocorrect
    test/prism_regression/invalid/malformed_sig.rb:5: Insert extend T::Sig
    sig { params(x: T.untyped).returns(NilClass) }
     5 |def f(x)
        ^
Errors: 6

@amomchilov amomchilov force-pushed the desugar-remaining branch 2 times, most recently from d96acf3 to 7f35d5d Compare December 19, 2025 19:25
@thomasmarshall thomasmarshall force-pushed the fix-malformed-sig-as-mlhs branch from 9e7c27f to 13e7e63 Compare January 7, 2026 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant