From c20ead10ed8bfba0bf9d6324c451a4b944ffcbdf Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 4 Oct 2026 15:08:13 +0200 Subject: [PATCH] ast: record the line a block's opener sits on A Block had no line of its own. Program.Lines stamps STATEMENT nodes, and a Block is not one -- it is not even a Node, being a field of Call -- so a consumer had to take the line of whatever statement enclosed the block. That is the same line only when the block opens on that statement's first line. Measured against ruby 4.0.5, which reports the opener's line as a block's location.first_lineno: h = { # line 1 :a => proc { # line 2 1 }, :b => proc { 2 }, # line 5 :c => proc { # line 6 }, } h[:a].source_location[1] # ruby 2 from the statement, 1 h[:b].source_location[1] # ruby 5 from the statement, 1 h[:c].source_location[1] # ruby 6 from the statement, 1 The body's first line is NOT a substitute, which is why this needs the opener and not a cheaper approximation: ruby answers 2 for :a, whose first statement is on line 3, and :c has no body statement at all. So Block carries a Line field rather than a position field being added to all 55 node types, and rather than Program.Lines being stretched to keys that are not Nodes. 0 means "not recorded". Three sites set it: the `{` of parseBraceBlock, the `do` of parseDoBlock, and the `->` of parseLambda -- the arrow itself, since that is the line ruby reports for an arrow lambda (measured; they coincide in all but deliberately odd layouts). Observable downstream as Proc#source_location, Method#source_location for a define_method, and the line a Proc prints when it inspects itself. Also gofmt'd paren_singleton_def_test.go, which was unformatted on main. Co-Authored-By: Claude Opus 5 --- ast/ast.go | 26 ++++++++ block_line_test.go | 127 ++++++++++++++++++++++++++++++++++++ paren_singleton_def_test.go | 10 +-- parser.go | 16 ++++- 4 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 block_line_test.go diff --git a/ast/ast.go b/ast/ast.go index 6237ffe..c5a9a9e 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -172,6 +172,32 @@ type Call struct { // *splat param; non-nil for an optional `name = expr` param), exactly as // MethodDef records its method-parameter defaults. type Block struct { + // Line is the 1-based source line the block's OPENER sits on -- its `{`, its + // `do`, or the `->` of an arrow lambda. It lives here rather than in + // Program.Lines because a Block is not a Node (it is a field of Call), so + // that map cannot key on it. + // + // It is observable as MRI's location.first_lineno: Proc#source_location, + // Method#source_location for a define_method, and the line a Proc prints + // when it inspects itself. A consumer previously had to take the line of the + // enclosing STATEMENT, which is the same only when the block opens on that + // statement's first line. Measured against ruby 4.0.5: + // + // h = { # line 1 + // :a => proc { # line 2 + // 1 + // }, + // :b => proc { 2 }, # line 5 + // } + // h[:a].source_location[1] # ruby 2 from the statement, 1 + // h[:b].source_location[1] # ruby 5 from the statement, 1 + // + // The body's first line is not a substitute either: ruby answers 2 for :a, + // whose first statement is on line 3, and an empty block has no statement at + // all. + // + // 0 means "not recorded" -- read it as unknown, not as line 0. + Line int Params []string Defaults []Node // parallel to Params; nil for a required or *splat param SplatIndex int // index of the top-level *splat param in Params, or -1 diff --git a/block_line_test.go b/block_line_test.go new file mode 100644 index 0000000..11aef8b --- /dev/null +++ b/block_line_test.go @@ -0,0 +1,127 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// collectBlocks returns every Block in the tree, in source order, for the node +// shapes these cases use. +func collectBlocks(n ast.Node, out *[]*ast.Block) { + switch v := n.(type) { + case *ast.Program: + for _, s := range v.Body { + collectBlocks(s, out) + } + case *ast.Call: + if v.Recv != nil { + collectBlocks(v.Recv, out) + } + for _, a := range v.Args { + collectBlocks(a, out) + } + if v.Block != nil { + *out = append(*out, v.Block) + for _, s := range v.Block.Body { + collectBlocks(s, out) + } + } + case *ast.Assign: + collectBlocks(v.Value, out) + case *ast.HashLit: + for _, k := range v.Keys { + collectBlocks(k, out) + } + for _, val := range v.Values { + collectBlocks(val, out) + } + case *ast.ArrayLit: + for _, e := range v.Elems { + collectBlocks(e, out) + } + } +} + +// TestBlockLineIsItsOpener: a Block carries the line its `{`, `do` or `->` sits +// on, which is MRI's location.first_lineno for the block. +// +// A block is not a statement, so Program.Lines never held this, and a consumer +// had to take the enclosing statement's line -- the same answer only when the +// block opens on that statement's first line. Every want below is the line +// ruby 4.0.5 reports through Proc#source_location for the corresponding block. +func TestBlockLineIsItsOpener(t *testing.T) { + for _, tc := range []struct { + name string + src string + want []int + }{ + { + // The discriminating case: the body's first line would be 3, 6 and 7, + // and ruby answers 2, 5 and 6 -- the openers. The third block is empty, + // so it has no body line at all. + name: "multi-line hash", + src: "h = {\n" + // 1 + " :a => proc {\n" + // 2 + " 1\n" + // 3 + " },\n" + // 4 + " :b => proc { 2 },\n" + // 5 + " :c => proc {\n" + // 6 + " },\n" + // 7 + "}\n", + want: []int{2, 5, 6}, + }, + { + name: "multi-line array with do...end", + src: "x = [\n" + // 1 + " proc do\n" + // 2 + " 3\n" + // 3 + " end,\n" + // 4 + "]\n", + want: []int{2}, + }, + { + // The arrow's own line, not its `{`. + name: "arrow lambda", + src: "h = {\n" + // 1 + " :a =>\n" + // 2 + " -> {\n" + // 3 + " 1\n" + // 4 + " },\n" + // 5 + " :b => ->() { 2 },\n" + // 6 + "}\n", + want: []int{3, 6}, + }, + { + name: "single line, unchanged", + src: "h = { :a => proc { 1 } }\n", + want: []int{1}, + }, + { + name: "method call with a trailing block", + src: "foo(1,\n" + // 1 + " 2) do\n" + // 2 + " 3\n" + // 3 + "end\n", + want: []int{2}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + prog, err := parser.Parse(tc.src) + if err != nil { + t.Fatalf("parse: %v", err) + } + var blocks []*ast.Block + collectBlocks(prog, &blocks) + if len(blocks) != len(tc.want) { + t.Fatalf("found %d blocks, want %d", len(blocks), len(tc.want)) + } + for i, b := range blocks { + if b.Line != tc.want[i] { + t.Errorf("block %d: Line = %d, want %d (ruby 4.0.5)", i, b.Line, tc.want[i]) + } + } + }) + } +} diff --git a/paren_singleton_def_test.go b/paren_singleton_def_test.go index 74a6b44..9086b81 100644 --- a/paren_singleton_def_test.go +++ b/paren_singleton_def_test.go @@ -123,11 +123,11 @@ func TestParenSingletonDefMethodCallReceiver(t *testing.T) { // applies unchanged after the receiver. func TestParenSingletonDefParamForms(t *testing.T) { cases := []struct { - src string - name string - params []string - kw []string - bodyLen int + src string + name string + params []string + kw []string + bodyLen int }{ {src: "def (obj).m(a, b); a; end", name: "m", params: []string{"a", "b"}, bodyLen: 1}, {src: "def (obj).m(a:); a; end", name: "m", kw: []string{"a"}, bodyLen: 1}, diff --git a/parser.go b/parser.go index 1884760..0e9eaf5 100644 --- a/parser.go +++ b/parser.go @@ -3422,6 +3422,10 @@ func (p *Parser) stringKeyColon(key ast.Node) (ast.Node, bool) { // parseLambda parses a stabby lambda `->(params) { body }` / `-> { body }` / // `->(params) do body end`, desugaring it to `lambda { |params| body }`. func (p *Parser) parseLambda() ast.Node { + // The `->` itself, not its `{`: ruby reports the arrow's line as the lambda's + // location.first_lineno (measured; they are the same line in all but + // deliberately odd layouts). + lambdaLine := p.cur().Line p.expect(token.ARROW) p.pushBlockScope() var params []string @@ -3490,20 +3494,26 @@ func (p *Parser) parseLambda() ast.Node { if len(prepends) > 0 { body = append(prepends, body...) } - return &ast.Call{Name: "lambda", Block: &ast.Block{Params: params, Defaults: defaults, SplatIndex: splat, BlockParam: blockParam, Body: body}} + return &ast.Call{Name: "lambda", Block: &ast.Block{Line: lambdaLine, Params: params, Defaults: defaults, SplatIndex: splat, BlockParam: blockParam, Body: body}} } // parseBraceBlock parses `{ [|params|] body }`. func (p *Parser) parseBraceBlock() *ast.Block { + line := p.cur().Line p.expect(token.LBRACE) - return p.parseBlockRest(map[token.Type]bool{token.RBRACE: true}, token.RBRACE, false) + b := p.parseBlockRest(map[token.Type]bool{token.RBRACE: true}, token.RBRACE, false) + b.Line = line + return b } // parseDoBlock parses `do [|params|] body end`. A do…end block body may carry // rescue/else/ensure clauses without an explicit begin (unlike a brace block). func (p *Parser) parseDoBlock() *ast.Block { + line := p.cur().Line p.expect(token.DO) - return p.parseBlockRest(beginBodyEnd, token.END, true) + b := p.parseBlockRest(beginBodyEnd, token.END, true) + b.Line = line + return b } // parseBlockRest parses a block's optional `|params|` and body, having already