From 5c06479d1ad4614bab4bc34497eec40318c73b73 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:07:02 +0200 Subject: [PATCH] Make `SymbolNode#value_loc` non-optional Sort of a followup for https://github.com/ruby/prism/commit/929aec16504f799dae358757784741ec8b2b1c6a It changed the value for `:''` into nil/null, which is a bit inconvenient. For syntax-valid code I expect it to always be present. It's also inconsistent with `StringNode`. In effect, this reverts the snapshots changes and the two changes in ruby for the ripper/parser compiler. --- config.yml | 2 +- lib/prism/translation/parser/compiler.rb | 12 ++++----- lib/prism/translation/ripper.rb | 12 +++++---- rbi/generated/prism/dsl.rbi | 2 +- rbi/generated/prism/node.rbi | 10 +++---- sig/generated/prism/dsl.rbs | 4 +-- sig/generated/prism/node.rbs | 26 +++++++++---------- snapshots/hashes.txt | 2 +- snapshots/seattlerb/symbol_empty.txt | 2 +- snapshots/spanning_heredoc_newlines.txt | 2 +- snapshots/unparser/corpus/literal/literal.txt | 2 +- 11 files changed, 39 insertions(+), 37 deletions(-) diff --git a/config.yml b/config.yml index 4892089c03..f5ff264693 100644 --- a/config.yml +++ b/config.yml @@ -4553,7 +4553,7 @@ nodes: - name: opening_loc type: location? - name: value_loc - type: location? + type: location - name: closing_loc type: location? - name: unescaped diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index 1ef30dab1b..256d873f66 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -165,13 +165,13 @@ def visit_assoc_node(node) else parts = if key.is_a?(SymbolNode) - value_loc = key.value_loc - if value_loc.nil? + value = key.value + if value == "" [] - elsif value_loc.slice.include?("\n") - string_nodes_from_line_continuations(key.unescaped, value_loc.slice, value_loc.start_offset, key.opening) + elsif value.include?("\n") + string_nodes_from_line_continuations(key.unescaped, value, key.value_loc.start_offset, key.opening) else - [builder.string_internal([key.unescaped, srange(value_loc)])] + [builder.string_internal([key.unescaped, srange(key.value_loc)])] end else visit_all(key.parts) @@ -1775,7 +1775,7 @@ def visit_symbol_node(node) end else parts = - if node.value_loc.nil? + if node.value == "" [] elsif node.value.include?("\n") string_nodes_from_line_continuations(node.unescaped, node.value, node.value_loc.start_offset, node.opening) diff --git a/lib/prism/translation/ripper.rb b/lib/prism/translation/ripper.rb index d9c1e69f92..de178edd2c 100644 --- a/lib/prism/translation/ripper.rb +++ b/lib/prism/translation/ripper.rb @@ -3840,12 +3840,14 @@ def visit_super_node(node) # ^^^^ def visit_symbol_node(node) with_string_bounds(node) do - if node.value_loc.nil? - bounds(node.location) - on_dyna_symbol(on_string_content) - elsif (opening = node.opening)&.match?(/^%s|['"]:?$/) + if (opening = node.opening)&.match?(/^%s|['"]:?$/) bounds(node.value_loc) - content = on_string_add(on_string_content, on_tstring_content(node.value)) + content = on_string_content + + if !(value = node.value).empty? + content = on_string_add(content, on_tstring_content(value)) + end + bounds(node.location) on_dyna_symbol(content) elsif (closing = node.closing) == ":" diff --git a/rbi/generated/prism/dsl.rbi b/rbi/generated/prism/dsl.rbi index 75705a5278..6f60f0fc17 100644 --- a/rbi/generated/prism/dsl.rbi +++ b/rbi/generated/prism/dsl.rbi @@ -634,7 +634,7 @@ module Prism def super_node(source: T.unsafe(nil), node_id: T.unsafe(nil), location: T.unsafe(nil), flags: T.unsafe(nil), keyword_loc: T.unsafe(nil), lparen_loc: T.unsafe(nil), arguments: T.unsafe(nil), rparen_loc: T.unsafe(nil), block: T.unsafe(nil)); end # Create a new SymbolNode node. - sig { params(source: Source, node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: ::T.nilable(Location), closing_loc: ::T.nilable(Location), unescaped: String).returns(SymbolNode) } + sig { params(source: Source, node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: Location, closing_loc: ::T.nilable(Location), unescaped: String).returns(SymbolNode) } def symbol_node(source: T.unsafe(nil), node_id: T.unsafe(nil), location: T.unsafe(nil), flags: T.unsafe(nil), opening_loc: T.unsafe(nil), value_loc: T.unsafe(nil), closing_loc: T.unsafe(nil), unescaped: T.unsafe(nil)); end # Create a new TrueNode node. diff --git a/rbi/generated/prism/node.rbi b/rbi/generated/prism/node.rbi index 5cb71c8192..f524c4e5dd 100644 --- a/rbi/generated/prism/node.rbi +++ b/rbi/generated/prism/node.rbi @@ -13233,7 +13233,7 @@ module Prism # ^^^ class SymbolNode < Node # Initialize a new SymbolNode node. - sig { params(source: Source, node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: ::T.nilable(Location), closing_loc: ::T.nilable(Location), unescaped: String).void } + sig { params(source: Source, node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: Location, closing_loc: ::T.nilable(Location), unescaped: String).void } def initialize(source, node_id, location, flags, opening_loc, value_loc, closing_loc, unescaped); end # See Node.accept. @@ -13258,7 +13258,7 @@ module Prism def comment_targets; end # Creates a copy of self with the given fields, using self as the template. - sig { params(node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: ::T.nilable(Location), closing_loc: ::T.nilable(Location), unescaped: String).returns(SymbolNode) } + sig { params(node_id: Integer, location: Location, flags: Integer, opening_loc: ::T.nilable(Location), value_loc: Location, closing_loc: ::T.nilable(Location), unescaped: String).returns(SymbolNode) } def copy(node_id: T.unsafe(nil), location: T.unsafe(nil), flags: T.unsafe(nil), opening_loc: T.unsafe(nil), value_loc: T.unsafe(nil), closing_loc: T.unsafe(nil), unescaped: T.unsafe(nil)); end sig { override.returns(T::Array[::T.nilable(Node)]) } @@ -13300,12 +13300,12 @@ module Prism def save_opening_loc(repository); end # Returns the Location represented by `value_loc`. - sig { returns(::T.nilable(Location)) } + sig { returns(Location) } def value_loc; end # Save the value_loc location using the given saved source so that # it can be retrieved later. - sig { params(repository: ::T.untyped).returns(::T.nilable(Relocation::Entry)) } + sig { params(repository: ::T.untyped).returns(Relocation::Entry) } def save_value_loc(repository); end # Returns the Location represented by `closing_loc`. @@ -13326,7 +13326,7 @@ module Prism def opening; end # Slice the location of value_loc from the source. - sig { returns(::T.nilable(String)) } + sig { returns(String) } def value; end # Slice the location of closing_loc from the source. diff --git a/sig/generated/prism/dsl.rbs b/sig/generated/prism/dsl.rbs index c9dabf4fa6..3d57662ce2 100644 --- a/sig/generated/prism/dsl.rbs +++ b/sig/generated/prism/dsl.rbs @@ -780,8 +780,8 @@ module Prism # Create a new SymbolNode node. # - # : (?source: Source, ?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location?, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode - def symbol_node: (?source: Source, ?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location?, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode + # : (?source: Source, ?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode + def symbol_node: (?source: Source, ?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode # Create a new TrueNode node. # diff --git a/sig/generated/prism/node.rbs b/sig/generated/prism/node.rbs index 5026873a89..3f3adef533 100644 --- a/sig/generated/prism/node.rbs +++ b/sig/generated/prism/node.rbs @@ -19397,14 +19397,14 @@ module Prism @closing_loc: Location? - @value_loc: Location? + @value_loc: Location @opening_loc: Location? # Initialize a new SymbolNode node. # - # : (Source source, Integer node_id, Location location, Integer flags, Location? opening_loc, Location? value_loc, Location? closing_loc, String unescaped) -> void - def initialize: (Source source, Integer node_id, Location location, Integer flags, Location? opening_loc, Location? value_loc, Location? closing_loc, String unescaped) -> void + # : (Source source, Integer node_id, Location location, Integer flags, Location? opening_loc, Location value_loc, Location? closing_loc, String unescaped) -> void + def initialize: (Source source, Integer node_id, Location location, Integer flags, Location? opening_loc, Location value_loc, Location? closing_loc, String unescaped) -> void # See Node.accept. # @@ -19438,8 +19438,8 @@ module Prism # # Creates a copy of self with the given fields, using self as the template. # - # : (?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location?, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode - def copy: (?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location?, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode + # : (?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode + def copy: (?node_id: Integer, ?location: Location, ?flags: Integer, ?opening_loc: Location?, ?value_loc: Location, ?closing_loc: Location?, ?unescaped: String) -> SymbolNode alias deconstruct child_nodes @@ -19495,19 +19495,19 @@ module Prism # :category: Locations # :call-seq: - # value_loc -> Location | nil + # value_loc -> Location # # Returns the Location represented by `value_loc`. # - # : () -> Location? - def value_loc: () -> Location? + # : () -> Location + def value_loc: () -> Location # :category: Repository # Save the value_loc location using the given saved source so that # it can be retrieved later. # - # : (_Repository repository) -> Relocation::Entry? - def save_value_loc: (_Repository repository) -> Relocation::Entry? + # : (_Repository repository) -> Relocation::Entry + def save_value_loc: (_Repository repository) -> Relocation::Entry # :category: Locations # :call-seq: @@ -19542,12 +19542,12 @@ module Prism def opening: () -> String? # :call-seq: - # value -> String | nil + # value -> String # # Slice the location of value_loc from the source. # - # : () -> String? - def value: () -> String? + # : () -> String + def value: () -> String # :call-seq: # closing -> String | nil diff --git a/snapshots/hashes.txt b/snapshots/hashes.txt index a65a7b0676..ba27c44e06 100644 --- a/snapshots/hashes.txt +++ b/snapshots/hashes.txt @@ -447,7 +447,7 @@ │ │ │ @ SymbolNode (location: (30,2)-(30,5)) │ │ │ ├── flags: static_literal │ │ │ ├── opening_loc: (30,2)-(30,3) = "\"" - │ │ │ ├── value_loc: ∅ + │ │ │ ├── value_loc: (1,0)-(1,0) = "" │ │ │ ├── closing_loc: (30,3)-(30,5) = "\":" │ │ │ └── unescaped: "" │ │ ├── value: diff --git a/snapshots/seattlerb/symbol_empty.txt b/snapshots/seattlerb/symbol_empty.txt index 917b910891..945a0e3a51 100644 --- a/snapshots/seattlerb/symbol_empty.txt +++ b/snapshots/seattlerb/symbol_empty.txt @@ -8,6 +8,6 @@ └── @ SymbolNode (location: (1,0)-(1,3)) ├── flags: newline, static_literal, forced_us_ascii_encoding ├── opening_loc: (1,0)-(1,2) = ":'" - ├── value_loc: ∅ + ├── value_loc: (1,2)-(1,2) = "" ├── closing_loc: (1,2)-(1,3) = "'" └── unescaped: "" diff --git a/snapshots/spanning_heredoc_newlines.txt b/snapshots/spanning_heredoc_newlines.txt index c72956cf7e..976d5d28dc 100644 --- a/snapshots/spanning_heredoc_newlines.txt +++ b/snapshots/spanning_heredoc_newlines.txt @@ -129,7 +129,7 @@ │ │ └── @ SymbolNode (location: (17,4)-(20,0)) │ │ ├── flags: static_literal, forced_us_ascii_encoding │ │ ├── opening_loc: (17,4)-(18,0) = "%s\n" - │ │ ├── value_loc: ∅ + │ │ ├── value_loc: (18,0)-(18,0) = "" │ │ ├── closing_loc: (19,0)-(20,0) = "\n" │ │ └── unescaped: "" │ ├── closing_loc: ∅ diff --git a/snapshots/unparser/corpus/literal/literal.txt b/snapshots/unparser/corpus/literal/literal.txt index eb0bf12d5b..0252179772 100644 --- a/snapshots/unparser/corpus/literal/literal.txt +++ b/snapshots/unparser/corpus/literal/literal.txt @@ -620,7 +620,7 @@ ├── @ SymbolNode (location: (48,0)-(48,3)) │ ├── flags: newline, static_literal │ ├── opening_loc: (48,0)-(48,2) = ":\"" - │ ├── value_loc: ∅ + │ ├── value_loc: (48,2)-(48,2) = "" │ ├── closing_loc: (48,2)-(48,3) = "\"" │ └── unescaped: "" ├── @ RegularExpressionNode (location: (49,0)-(49,5))