From 2ccb2de677849732181224cb9fd1a831dbaac4c0 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Tue, 26 Aug 2025 18:58:05 +0900 Subject: [PATCH 1/5] Make `RubyVM::AST.of` return a parent node of NODE_SCOPE This change makes `RubyVM::AST.of` and `.node_id_for_backtrace_location` return a parent node of NODE_SCOPE (such as NODE_DEFN) instead of the NODE_SCOPE node itself. (In future, we may remove NODE_SCOPE, which is a bit hacky AST node.) This is preparation for [Feature #21543]. --- compile.c | 1 + iseq.c | 14 ++++++++++++- parse.y | 49 +++++++++++++++++++++++++------------------ rubyparser.h | 1 + test/ruby/test_ast.rb | 4 ++-- vm.c | 1 + 6 files changed, 47 insertions(+), 23 deletions(-) diff --git a/compile.c b/compile.c index 92c5d6dc9e0664..0ef0d1b31a5fa3 100644 --- a/compile.c +++ b/compile.c @@ -9221,6 +9221,7 @@ compile_builtin_mandatory_only_method(rb_iseq_t *iseq, const NODE *node, const N rb_node_init(RNODE(&scope_node), NODE_SCOPE); scope_node.nd_tbl = tbl; scope_node.nd_body = mandatory_node(iseq, node); + scope_node.nd_parent = NULL; scope_node.nd_args = &args_node; VALUE ast_value = rb_ruby_ast_new(RNODE(&scope_node)); diff --git a/iseq.c b/iseq.c index ac59429901ce36..33b37bade5dece 100644 --- a/iseq.c +++ b/iseq.c @@ -634,6 +634,18 @@ new_arena(void) return new_arena; } +static int +prepare_node_id(const NODE *node) +{ + if (!node) return -1; + + if (nd_type(node) == NODE_SCOPE && RNODE_SCOPE(node)->nd_parent) { + return nd_node_id(RNODE_SCOPE(node)->nd_parent); + } + + return nd_node_id(node); +} + static VALUE prepare_iseq_build(rb_iseq_t *iseq, VALUE name, VALUE path, VALUE realpath, int first_lineno, const rb_code_location_t *code_location, const int node_id, @@ -1031,7 +1043,7 @@ rb_iseq_new_with_opt(VALUE ast_value, VALUE name, VALUE path, VALUE realpath, script_lines = ISEQ_BODY(parent)->variable.script_lines; } - prepare_iseq_build(iseq, name, path, realpath, first_lineno, node ? &node->nd_loc : NULL, node ? nd_node_id(node) : -1, + prepare_iseq_build(iseq, name, path, realpath, first_lineno, node ? &node->nd_loc : NULL, prepare_node_id(node), parent, isolated_depth, type, script_lines, option); rb_iseq_compile_node(iseq, node); diff --git a/parse.y b/parse.y index c0f46a395f9f59..765b4bdfd0890d 100644 --- a/parse.y +++ b/parse.y @@ -1061,8 +1061,8 @@ rb_discard_node(struct parser_params *p, NODE *n) rb_ast_delete_node(p->ast, n); } -static rb_node_scope_t *rb_node_scope_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc); -static rb_node_scope_t *rb_node_scope_new2(struct parser_params *p, rb_ast_id_table_t *nd_tbl, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc); +static rb_node_scope_t *rb_node_scope_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, NODE *nd_parent, const YYLTYPE *loc); +static rb_node_scope_t *rb_node_scope_new2(struct parser_params *p, rb_ast_id_table_t *nd_tbl, rb_node_args_t *nd_args, NODE *nd_body, NODE *nd_parent, const YYLTYPE *loc); static rb_node_block_t *rb_node_block_new(struct parser_params *p, NODE *nd_head, const YYLTYPE *loc); static rb_node_if_t *rb_node_if_new(struct parser_params *p, NODE *nd_cond, NODE *nd_body, NODE *nd_else, const YYLTYPE *loc, const YYLTYPE* if_keyword_loc, const YYLTYPE* then_keyword_loc, const YYLTYPE* end_keyword_loc); static rb_node_unless_t *rb_node_unless_new(struct parser_params *p, NODE *nd_cond, NODE *nd_body, NODE *nd_else, const YYLTYPE *loc, const YYLTYPE *keyword_loc, const YYLTYPE *then_keyword_loc, const YYLTYPE *end_keyword_loc); @@ -1169,8 +1169,8 @@ static rb_node_line_t *rb_node_line_new(struct parser_params *p, const YYLTYPE * static rb_node_file_t *rb_node_file_new(struct parser_params *p, VALUE str, const YYLTYPE *loc); static rb_node_error_t *rb_node_error_new(struct parser_params *p, const YYLTYPE *loc); -#define NEW_SCOPE(a,b,loc) (NODE *)rb_node_scope_new(p,a,b,loc) -#define NEW_SCOPE2(t,a,b,loc) (NODE *)rb_node_scope_new2(p,t,a,b,loc) +#define NEW_SCOPE(a,b,c,loc) (NODE *)rb_node_scope_new(p,a,b,c,loc) +#define NEW_SCOPE2(t,a,b,c,loc) (NODE *)rb_node_scope_new2(p,t,a,b,c,loc) #define NEW_BLOCK(a,loc) (NODE *)rb_node_block_new(p,a,loc) #define NEW_IF(c,t,e,loc,ik_loc,tk_loc,ek_loc) (NODE *)rb_node_if_new(p,c,t,e,loc,ik_loc,tk_loc,ek_loc) #define NEW_UNLESS(c,t,e,loc,k_loc,t_loc,e_loc) (NODE *)rb_node_unless_new(p,c,t,e,loc,k_loc,t_loc,e_loc) @@ -1634,11 +1634,11 @@ aryptn_pre_args(struct parser_params *p, VALUE pre_arg, VALUE pre_args) #define KWD2EID(t, v) keyword_##t static NODE * -new_scope_body(struct parser_params *p, rb_node_args_t *args, NODE *body, const YYLTYPE *loc) +new_scope_body(struct parser_params *p, rb_node_args_t *args, NODE *body, NODE *parent, const YYLTYPE *loc) { body = remove_begin(body); reduce_nodes(p, &body); - NODE *n = NEW_SCOPE(args, body, loc); + NODE *n = NEW_SCOPE(args, body, parent, loc); nd_set_line(n, loc->end_pos.lineno); set_line_body(body, loc->beg_pos.lineno); return n; @@ -2949,8 +2949,8 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary) { endless_method_name(p, $head->nd_mid, &@head); restore_defun(p, $head); - $bodystmt = new_scope_body(p, $args, $bodystmt, &@$); ($$ = $head->nd_def)->nd_loc = @$; + $bodystmt = new_scope_body(p, $args, $bodystmt, $$, &@$); RNODE_DEFN($$)->nd_defn = $bodystmt; /*% ripper: bodystmt!($:bodystmt, Qnil, Qnil, Qnil) %*/ /*% ripper: def!($:head, $:args, $:$) %*/ @@ -2960,8 +2960,8 @@ rb_parser_ary_free(rb_parser_t *p, rb_parser_ary_t *ary) { endless_method_name(p, $head->nd_mid, &@head); restore_defun(p, $head); - $bodystmt = new_scope_body(p, $args, $bodystmt, &@$); ($$ = $head->nd_def)->nd_loc = @$; + $bodystmt = new_scope_body(p, $args, $bodystmt, $$, &@$); RNODE_DEFS($$)->nd_defn = $bodystmt; /*% ripper: bodystmt!($:bodystmt, Qnil, Qnil, Qnil) %*/ /*% ripper: defs!(*$:head[0..2], $:args, $:$) %*/ @@ -3176,7 +3176,7 @@ program : { node = remove_begin(node); void_expr(p, node); } - p->eval_tree = NEW_SCOPE(0, block_append(p, p->eval_tree, $2), &@$); + p->eval_tree = NEW_SCOPE(0, block_append(p, p->eval_tree, $2), NULL, &@$); /*% ripper[final]: program!($:2) %*/ local_pop(p); } @@ -3375,8 +3375,9 @@ stmt : keyword_alias fitem {SET_LEX_STATE(EXPR_FNAME|EXPR_FITEM);} fitem restore_block_exit(p, $allow_exits); p->ctxt = $k_END; { - NODE *scope = NEW_SCOPE2(0 /* tbl */, 0 /* args */, $compstmt /* body */, &@$); + NODE *scope = NEW_SCOPE2(0 /* tbl */, 0 /* args */, $compstmt /* body */, NULL /* parent */, &@$); $$ = NEW_POSTEXE(scope, &@$, &@1, &@3, &@5); + RNODE_SCOPE(scope)->nd_parent = $$; } /*% ripper: END!($:compstmt) %*/ } @@ -4567,9 +4568,10 @@ primary : inline_primary } /* {|*internal_id| = internal_id; ... } */ args = new_args(p, m, 0, id, 0, new_args_tail(p, 0, 0, 0, &@for_var), &@for_var); - scope = NEW_SCOPE2(tbl, args, $compstmt, &@$); + scope = NEW_SCOPE2(tbl, args, $compstmt, NULL, &@$); YYLTYPE do_keyword_loc = $do == keyword_do_cond ? @do : NULL_LOC; $$ = NEW_FOR($5, scope, &@$, &@k_for, &@keyword_in, &do_keyword_loc, &@k_end); + RNODE_SCOPE(scope)->nd_parent = $$; fixpos($$, $for_var); /*% ripper: for!($:for_var, $:expr_value, $:compstmt) %*/ } @@ -4640,8 +4642,8 @@ primary : inline_primary k_end { restore_defun(p, $head); - $bodystmt = new_scope_body(p, $args, $bodystmt, &@$); ($$ = $head->nd_def)->nd_loc = @$; + $bodystmt = new_scope_body(p, $args, $bodystmt, $$, &@$); RNODE_DEFN($$)->nd_defn = $bodystmt; /*% ripper: def!($:head, $:args, $:bodystmt) %*/ local_pop(p); @@ -4655,8 +4657,8 @@ primary : inline_primary k_end { restore_defun(p, $head); - $bodystmt = new_scope_body(p, $args, $bodystmt, &@$); ($$ = $head->nd_def)->nd_loc = @$; + $bodystmt = new_scope_body(p, $args, $bodystmt, $$, &@$); RNODE_DEFS($$)->nd_defn = $bodystmt; /*% ripper: defs!(*$:head[0..2], $:args, $:bodystmt) %*/ local_pop(p); @@ -11208,24 +11210,26 @@ node_newnode(struct parser_params *p, enum node_type type, size_t size, size_t a #define NODE_NEWNODE(node_type, type, loc) (type *)(node_newnode(p, node_type, sizeof(type), RUBY_ALIGNOF(type), loc)) static rb_node_scope_t * -rb_node_scope_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc) +rb_node_scope_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, NODE *nd_parent, const YYLTYPE *loc) { rb_ast_id_table_t *nd_tbl; nd_tbl = local_tbl(p); rb_node_scope_t *n = NODE_NEWNODE(NODE_SCOPE, rb_node_scope_t, loc); n->nd_tbl = nd_tbl; n->nd_body = nd_body; + n->nd_parent = nd_parent; n->nd_args = nd_args; return n; } static rb_node_scope_t * -rb_node_scope_new2(struct parser_params *p, rb_ast_id_table_t *nd_tbl, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc) +rb_node_scope_new2(struct parser_params *p, rb_ast_id_table_t *nd_tbl, rb_node_args_t *nd_args, NODE *nd_body, NODE *nd_parent, const YYLTYPE *loc) { rb_node_scope_t *n = NODE_NEWNODE(NODE_SCOPE, rb_node_scope_t, loc); n->nd_tbl = nd_tbl; n->nd_body = nd_body; + n->nd_parent = nd_parent; n->nd_args = nd_args; return n; @@ -11413,8 +11417,9 @@ static rb_node_class_t * rb_node_class_new(struct parser_params *p, NODE *nd_cpath, NODE *nd_body, NODE *nd_super, const YYLTYPE *loc, const YYLTYPE *class_keyword_loc, const YYLTYPE *inheritance_operator_loc, const YYLTYPE *end_keyword_loc) { /* Keep the order of node creation */ - NODE *scope = NEW_SCOPE(0, nd_body, loc); + NODE *scope = NEW_SCOPE(0, nd_body, NULL, loc); rb_node_class_t *n = NODE_NEWNODE(NODE_CLASS, rb_node_class_t, loc); + RNODE_SCOPE(scope)->nd_parent = &n->node; n->nd_cpath = nd_cpath; n->nd_body = scope; n->nd_super = nd_super; @@ -11429,8 +11434,9 @@ static rb_node_sclass_t * rb_node_sclass_new(struct parser_params *p, NODE *nd_recv, NODE *nd_body, const YYLTYPE *loc) { /* Keep the order of node creation */ - NODE *scope = NEW_SCOPE(0, nd_body, loc); + NODE *scope = NEW_SCOPE(0, nd_body, NULL, loc); rb_node_sclass_t *n = NODE_NEWNODE(NODE_SCLASS, rb_node_sclass_t, loc); + RNODE_SCOPE(scope)->nd_parent = &n->node; n->nd_recv = nd_recv; n->nd_body = scope; @@ -11441,8 +11447,9 @@ static rb_node_module_t * rb_node_module_new(struct parser_params *p, NODE *nd_cpath, NODE *nd_body, const YYLTYPE *loc, const YYLTYPE *module_keyword_loc, const YYLTYPE *end_keyword_loc) { /* Keep the order of node creation */ - NODE *scope = NEW_SCOPE(0, nd_body, loc); + NODE *scope = NEW_SCOPE(0, nd_body, NULL, loc); rb_node_module_t *n = NODE_NEWNODE(NODE_MODULE, rb_node_module_t, loc); + RNODE_SCOPE(scope)->nd_parent = &n->node; n->nd_cpath = nd_cpath; n->nd_body = scope; n->module_keyword_loc = *module_keyword_loc; @@ -11455,8 +11462,9 @@ static rb_node_iter_t * rb_node_iter_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc) { /* Keep the order of node creation */ - NODE *scope = NEW_SCOPE(nd_args, nd_body, loc); + NODE *scope = NEW_SCOPE(nd_args, nd_body, NULL, loc); rb_node_iter_t *n = NODE_NEWNODE(NODE_ITER, rb_node_iter_t, loc); + RNODE_SCOPE(scope)->nd_parent = &n->node; n->nd_body = scope; n->nd_iter = 0; @@ -11467,9 +11475,10 @@ static rb_node_lambda_t * rb_node_lambda_new(struct parser_params *p, rb_node_args_t *nd_args, NODE *nd_body, const YYLTYPE *loc, const YYLTYPE *operator_loc, const YYLTYPE *opening_loc, const YYLTYPE *closing_loc) { /* Keep the order of node creation */ - NODE *scope = NEW_SCOPE(nd_args, nd_body, loc); + NODE *scope = NEW_SCOPE(nd_args, nd_body, NULL, loc); YYLTYPE lambda_loc = code_loc_gen(operator_loc, closing_loc); rb_node_lambda_t *n = NODE_NEWNODE(NODE_LAMBDA, rb_node_lambda_t, &lambda_loc); + RNODE_SCOPE(scope)->nd_parent = &n->node; n->nd_body = scope; n->operator_loc = *operator_loc; n->opening_loc = *opening_loc; diff --git a/rubyparser.h b/rubyparser.h index cc63efd3f85998..4ab2480b7de770 100644 --- a/rubyparser.h +++ b/rubyparser.h @@ -248,6 +248,7 @@ typedef struct RNode_SCOPE { rb_ast_id_table_t *nd_tbl; struct RNode *nd_body; + struct RNode *nd_parent; struct RNode_ARGS *nd_args; } rb_node_scope_t; diff --git a/test/ruby/test_ast.rb b/test/ruby/test_ast.rb index 9a7d75c270b661..f5c4b8d6b98f54 100644 --- a/test/ruby/test_ast.rb +++ b/test/ruby/test_ast.rb @@ -801,7 +801,7 @@ def test_keep_script_lines_for_of node_proc = RubyVM::AbstractSyntaxTree.of(proc, keep_script_lines: true) node_method = RubyVM::AbstractSyntaxTree.of(method, keep_script_lines: true) - assert_equal("{ 1 + 2 }", node_proc.source) + assert_equal("Proc.new { 1 + 2 }", node_proc.source) assert_equal("def test_keep_script_lines_for_of\n", node_method.source.lines.first) end @@ -878,7 +878,7 @@ def test_e_option omit if ParserSupport.prism_enabled? || ParserSupport.prism_enabled_in_subprocess? assert_in_out_err(["-e", "def foo; end; pp RubyVM::AbstractSyntaxTree.of(method(:foo)).type"], - "", [":SCOPE"], []) + "", [":DEFN"], []) end def test_error_tolerant diff --git a/vm.c b/vm.c index 479b3be94fec23..60c26cf0188d8a 100644 --- a/vm.c +++ b/vm.c @@ -1541,6 +1541,7 @@ rb_binding_add_dynavars(VALUE bindval, rb_binding_t *bind, int dyncount, const I rb_node_init(RNODE(&tmp_node), NODE_SCOPE); tmp_node.nd_tbl = dyns; tmp_node.nd_body = 0; + tmp_node.nd_parent = NULL; tmp_node.nd_args = 0; VALUE ast_value = rb_ruby_ast_new(RNODE(&tmp_node)); From ed8fe53e80e16f9bff592333a3082981f39216e1 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Tue, 26 Aug 2025 19:05:57 +0900 Subject: [PATCH 2/5] Allow to get a NODE_SCOPE node of dummy stack frame of ArgumentError Previously, it was not possible to obtain a node of the callee's `Thread::Backtrace::Location` for cases like "wrong number of arguments" by using `RubyVM::AST.of`. This change allows that retrieval. This is preparation for [Feature #21543]. --- test/ruby/test_ast.rb | 44 +++++++++++++++++++++++++++++++++++++++++++ vm_backtrace.c | 4 ++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_ast.rb b/test/ruby/test_ast.rb index f5c4b8d6b98f54..ef078c35759a1e 100644 --- a/test/ruby/test_ast.rb +++ b/test/ruby/test_ast.rb @@ -365,6 +365,50 @@ def test_node_id_for_location assert_equal node.node_id, node_id end + def add(x, y) + end + + def test_node_id_for_backtrace_location_of_method_definition + omit if ParserSupport.prism_enabled? + + begin + add(1) + rescue ArgumentError => exc + loc = exc.backtrace_locations.first + node_id = RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(loc) + node = RubyVM::AbstractSyntaxTree.of(method(:add)) + assert_equal node.node_id, node_id + end + end + + def test_node_id_for_backtrace_location_of_lambda + omit if ParserSupport.prism_enabled? + + v = -> {} + begin + v.call(1) + rescue ArgumentError => exc + loc = exc.backtrace_locations.first + node_id = RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(loc) + node = RubyVM::AbstractSyntaxTree.of(v) + assert_equal node.node_id, node_id + end + end + + def test_node_id_for_backtrace_location_of_lambda_method + omit if ParserSupport.prism_enabled? + + v = lambda {} + begin + v.call(1) + rescue ArgumentError => exc + loc = exc.backtrace_locations.first + node_id = RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(loc) + node = RubyVM::AbstractSyntaxTree.of(v) + assert_equal node.node_id, node_id + end + end + def test_node_id_for_backtrace_location_raises_argument_error bug19262 = '[ruby-core:111435]' diff --git a/vm_backtrace.c b/vm_backtrace.c index 12e4b771e22060..cc8607b2d724c7 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -44,7 +44,7 @@ calc_pos(const rb_iseq_t *iseq, const VALUE *pc, int *lineno, int *node_id) } if (lineno) *lineno = ISEQ_BODY(iseq)->location.first_lineno; #ifdef USE_ISEQ_NODE_ID - if (node_id) *node_id = -1; + if (node_id) *node_id = ISEQ_BODY(iseq)->location.node_id; #endif return 1; } @@ -400,7 +400,7 @@ location_path_m(VALUE self) static int location_node_id(rb_backtrace_location_t *loc) { - if (loc->iseq && loc->pc) { + if (loc->iseq) { return calc_node_id(loc->iseq, loc->pc); } return -1; From 85e0c98cf0537c2049bfbbc2e21228264db90e00 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Tue, 26 Aug 2025 19:11:28 +0900 Subject: [PATCH 3/5] [ruby/error_highlight] Show a dedicated snippet for "wrong number of arguments" error This is an experimental implementation for https://bugs.ruby-lang.org/issues/21543. ``` test.rb:2:in 'Object#foo': wrong number of arguments (given 1, expected 2) (ArgumentError) caller: test.rb:6 | foo(1) ^^^ callee: test.rb:2 | def foo(x, y) ^^^ from test.rb:6:in 'Object#bar' from test.rb:10:in 'Object#baz' from test.rb:13:in '
' ``` https://github.com/ruby/error_highlight/commit/21e974e1c4 --- lib/error_highlight/base.rb | 112 ++++++++++ lib/error_highlight/core_ext.rb | 35 ++- test/error_highlight/test_error_highlight.rb | 219 +++++++++++++++++-- 3 files changed, 346 insertions(+), 20 deletions(-) diff --git a/lib/error_highlight/base.rb b/lib/error_highlight/base.rb index 14e0ce5785a354..b4a31f8e802019 100644 --- a/lib/error_highlight/base.rb +++ b/lib/error_highlight/base.rb @@ -239,6 +239,20 @@ def spot when :OP_CDECL spot_op_cdecl + when :DEFN + raise NotImplementedError if @point_type != :name + spot_defn + + when :DEFS + raise NotImplementedError if @point_type != :name + spot_defs + + when :LAMBDA + spot_lambda + + when :ITER + spot_iter + when :call_node case @point_type when :name @@ -280,6 +294,30 @@ def spot when :constant_path_operator_write_node prism_spot_constant_path_operator_write + when :def_node + case @point_type + when :name + prism_spot_def_for_name + when :args + raise NotImplementedError + end + + when :lambda_node + case @point_type + when :name + prism_spot_lambda_for_name + when :args + raise NotImplementedError + end + + when :block_node + case @point_type + when :name + prism_spot_block_for_name + when :args + raise NotImplementedError + end + end if @snippet && @beg_column && @end_column && @beg_column < @end_column @@ -621,6 +659,55 @@ def spot_op_cdecl end end + # Example: + # def bar; end + # ^^^ + def spot_defn + mid, = @node.children + fetch_line(@node.first_lineno) + if @snippet.match(/\Gdef\s+(#{ Regexp.quote(mid) }\b)/, @node.first_column) + @beg_column = $~.begin(1) + @end_column = $~.end(1) + end + end + + # Example: + # def Foo.bar; end + # ^^^^ + def spot_defs + nd_recv, mid, = @node.children + fetch_line(nd_recv.last_lineno) + if @snippet.match(/\G\s*(\.\s*#{ Regexp.quote(mid) }\b)/, nd_recv.last_column) + @beg_column = $~.begin(1) + @end_column = $~.end(1) + end + end + + # Example: + # -> { ... } + # ^^ + def spot_lambda + fetch_line(@node.first_lineno) + if @snippet.match(/\G->/, @node.first_column) + @beg_column = $~.begin(0) + @end_column = $~.end(0) + end + end + + # Example: + # lambda { ... } + # ^ + # define_method :foo do + # ^^ + def spot_iter + _nd_fcall, nd_scope = @node.children + fetch_line(nd_scope.first_lineno) + if @snippet.match(/\G(?:do\b|\{)/, nd_scope.first_column) + @beg_column = $~.begin(0) + @end_column = $~.end(0) + end + end + def fetch_line(lineno) @beg_lineno = @end_lineno = lineno @snippet = @fetch[lineno] @@ -826,6 +913,31 @@ def prism_spot_constant_path_operator_write prism_location(@node.binary_operator_loc.chop) end end + + # Example: + # def foo() + # ^^^ + def prism_spot_def_for_name + location = @node.name_loc + location = location.join(@node.operator_loc) if @node.operator_loc + prism_location(location) + end + + # Example: + # -> x, y { } + # ^^ + def prism_spot_lambda_for_name + prism_location(@node.operator_loc) + end + + # Example: + # lambda { } + # ^ + # define_method :foo do |x, y| + # ^ + def prism_spot_block_for_name + prism_location(@node.opening_loc) + end end private_constant :Spotter diff --git a/lib/error_highlight/core_ext.rb b/lib/error_highlight/core_ext.rb index b69093f74ecc07..2fb07f2e65bc7c 100644 --- a/lib/error_highlight/core_ext.rb +++ b/lib/error_highlight/core_ext.rb @@ -3,9 +3,38 @@ module ErrorHighlight module CoreExt private def generate_snippet - spot = ErrorHighlight.spot(self) - return "" unless spot - return ErrorHighlight.formatter.message_for(spot) + if ArgumentError === self && message =~ /\A(?:wrong number of arguments|missing keyword|unknown keyword|no keywords accepted)\b/ + locs = self.backtrace_locations + return "" if locs.size < 2 + callee_loc, caller_loc = locs + callee_spot = ErrorHighlight.spot(self, backtrace_location: callee_loc, point_type: :name) + caller_spot = ErrorHighlight.spot(self, backtrace_location: caller_loc, point_type: :name) + if caller_spot && callee_spot && + caller_loc.path == callee_loc.path && + caller_loc.lineno == callee_loc.lineno && + caller_spot == callee_spot + callee_loc = callee_spot = nil + end + ret = +"\n" + [["caller", caller_loc, caller_spot], ["callee", callee_loc, callee_spot]].each do |header, loc, spot| + out = nil + if loc + out = " #{ header }: #{ loc.path }:#{ loc.lineno }" + if spot + _, _, snippet, highlight = ErrorHighlight.formatter.message_for(spot).lines + out += "\n | #{ snippet } #{ highlight }" + else + out += "\n (cannot create a snippet of the method definition; use Ruby 3.5 or later)" + end + end + ret << "\n" + out if out + end + ret + else + spot = ErrorHighlight.spot(self) + return "" unless spot + return ErrorHighlight.formatter.message_for(spot) + end end if Exception.method_defined?(:detailed_message) diff --git a/test/error_highlight/test_error_highlight.rb b/test/error_highlight/test_error_highlight.rb index 8aa5eb9c8dd45f..09b0579f8aabdf 100644 --- a/test/error_highlight/test_error_highlight.rb +++ b/test/error_highlight/test_error_highlight.rb @@ -44,14 +44,16 @@ def preprocess(msg) def assert_error_message(klass, expected_msg, &blk) omit unless klass < ErrorHighlight::CoreExt err = assert_raise(klass, &blk) - spot = ErrorHighlight.spot(err) - if spot - assert_kind_of(Integer, spot[:first_lineno]) - assert_kind_of(Integer, spot[:first_column]) - assert_kind_of(Integer, spot[:last_lineno]) - assert_kind_of(Integer, spot[:last_column]) - assert_kind_of(String, spot[:snippet]) - assert_kind_of(Array, spot[:script_lines]) + unless klass == ArgumentError && err.message =~ /\A(?:wrong number of arguments|missing keyword|unknown keyword|no keywords accepted)\b/ + spot = ErrorHighlight.spot(err) + if spot + assert_kind_of(Integer, spot[:first_lineno]) + assert_kind_of(Integer, spot[:first_column]) + assert_kind_of(Integer, spot[:last_lineno]) + assert_kind_of(Integer, spot[:last_column]) + assert_kind_of(String, spot[:snippet]) + assert_kind_of(Array, spot[:script_lines]) + end end assert_equal(preprocess(expected_msg).chomp, err.detailed_message(highlight: false).sub(/ \((?:NoMethod|Name)Error\)/, "")) end @@ -1111,12 +1113,13 @@ def test_args_CALL_2 end def test_args_ATTRASGN_1 - v = [] - assert_error_message(ArgumentError, <<~END) do -wrong number of arguments (given 1, expected 2..3) (ArgumentError) + v = method(:raise).to_proc + recv = NEW_MESSAGE_FORMAT ? "an instance of Proc" : v.inspect + assert_error_message(NoMethodError, <<~END) do +undefined method `[]=' for #{ recv } v [ ] = 1 - ^^^^^^ + ^^^^^ END v [ ] = 1 @@ -1199,16 +1202,16 @@ def test_args_OP_ASGN1_aref_1 end def test_args_OP_ASGN1_aref_2 - v = [] + v = method(:raise).to_proc assert_error_message(ArgumentError, <<~END) do -wrong number of arguments (given 0, expected 1..2) (ArgumentError) +ArgumentError (ArgumentError) - v [ ] += 42 - ^^^^^^^^ + v [ArgumentError] += 42 + ^^^^^^^^^^^^^^^^^^^^ END - v [ ] += 42 + v [ArgumentError] += 42 end end @@ -1453,6 +1456,188 @@ def exc.backtrace_locations = [] end end + begin + ->{}.call(1) + rescue ArgumentError => exc + MethodDefLocationSupported = + RubyVM::AbstractSyntaxTree.respond_to?(:node_id_for_backtrace_location) && + RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(exc.backtrace_locations.first) + end + + WRONG_NUMBER_OF_ARGUMENTS_LIENO = __LINE__ + 1 + def wrong_number_of_arguments_test(x, y) + x + y + end + + def test_wrong_number_of_arguments_for_method + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +wrong number of arguments (given 1, expected 2) (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | wrong_number_of_arguments_test(1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + callee: #{ __FILE__ }:#{ WRONG_NUMBER_OF_ARGUMENTS_LIENO } + #{ + MethodDefLocationSupported ? + "| def wrong_number_of_arguments_test(x, y) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + wrong_number_of_arguments_test(1) + end + end + + KEYWORD_TEST_LINENO = __LINE__ + 1 + def keyword_test(kw1:, kw2:, kw3:) + kw1 + kw2 + kw3 + end + + def test_missing_keyword + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +missing keyword: :kw3 (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | keyword_test(kw1: 1, kw2: 2) + ^^^^^^^^^^^^ + callee: #{ __FILE__ }:#{ KEYWORD_TEST_LINENO } + #{ + MethodDefLocationSupported ? + "| def keyword_test(kw1:, kw2:, kw3:) + ^^^^^^^^^^^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + keyword_test(kw1: 1, kw2: 2) + end + end + + def test_unknown_keyword + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +unknown keyword: :kw4 (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | keyword_test(kw1: 1, kw2: 2, kw3: 3, kw4: 4) + ^^^^^^^^^^^^ + callee: #{ __FILE__ }:#{ KEYWORD_TEST_LINENO } + #{ + MethodDefLocationSupported ? + "| def keyword_test(kw1:, kw2:, kw3:) + ^^^^^^^^^^^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + keyword_test(kw1: 1, kw2: 2, kw3: 3, kw4: 4) + end + end + + WRONG_NUBMER_OF_ARGUMENTS_TEST2_LINENO = __LINE__ + 1 + def wrong_number_of_arguments_test2( + long_argument_name_x, + long_argument_name_y, + long_argument_name_z + ) + long_argument_name_x + long_argument_name_y + long_argument_name_z + end + + def test_wrong_number_of_arguments_for_method2 + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +wrong number of arguments (given 1, expected 3) (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | wrong_number_of_arguments_test2(1) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + callee: #{ __FILE__ }:#{ WRONG_NUBMER_OF_ARGUMENTS_TEST2_LINENO } + #{ + MethodDefLocationSupported ? + "| def wrong_number_of_arguments_test2( + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + wrong_number_of_arguments_test2(1) + end + end + + def test_wrong_number_of_arguments_for_lambda_literal + v = -> {} + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +wrong number of arguments (given 1, expected 0) (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | v.call(1) + ^^^^^ + callee: #{ __FILE__ }:#{ lineno - 1 } + #{ + MethodDefLocationSupported ? + "| v = -> {} + ^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + v.call(1) + end + end + + def test_wrong_number_of_arguments_for_lambda_method + v = lambda { } + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +wrong number of arguments (given 1, expected 0) (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | v.call(1) + ^^^^^ + callee: #{ __FILE__ }:#{ lineno - 1 } + #{ + MethodDefLocationSupported ? + "| v = lambda { } + ^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + v.call(1) + end + end + + DEFINE_METHOD_TEST_LINENO = __LINE__ + 1 + define_method :define_method_test do |x, y| + x + y + end + + def test_wrong_number_of_arguments_for_define_method + v = lambda { } + lineno = __LINE__ + assert_error_message(ArgumentError, <<~END) do +wrong number of arguments (given 1, expected 2) (ArgumentError) + + caller: #{ __FILE__ }:#{ lineno + 16 } + | define_method_test(1) + ^^^^^^^^^^^^^^^^^^ + callee: #{ __FILE__ }:#{ DEFINE_METHOD_TEST_LINENO } + #{ + MethodDefLocationSupported ? + "| define_method :define_method_test do |x, y| + ^^" : + "(cannot create a snippet of the method definition; use Ruby 3.5 or later)" + } + END + + define_method_test(1) + end + end + def test_spoofed_filename Tempfile.create(["error_highlight_test", ".rb"], binmode: true) do |tmp| tmp << "module Dummy\nend\n" From 2e27f6e1f11745669c0a52e5c68b2f01f99c6a0b Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Thu, 28 Aug 2025 15:29:18 +0900 Subject: [PATCH 4/5] [ruby/error_highlight] Drop Ruby 3.1 support ... as it is already EOL https://github.com/ruby/error_highlight/commit/f15489216a --- lib/error_highlight/error_highlight.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/error_highlight/error_highlight.gemspec b/lib/error_highlight/error_highlight.gemspec index b2da18df830985..edfc4b776f3bb8 100644 --- a/lib/error_highlight/error_highlight.gemspec +++ b/lib/error_highlight/error_highlight.gemspec @@ -18,7 +18,7 @@ Gem::Specification.new do |spec| spec.homepage = "https://github.com/ruby/error_highlight" spec.license = "MIT" - spec.required_ruby_version = Gem::Requirement.new(">= 3.1.0.dev") + spec.required_ruby_version = Gem::Requirement.new(">= 3.2.0") spec.files = Dir.chdir(File.expand_path(__dir__)) do `git ls-files -z`.split("\x0").reject { |f| f.match(%r{\A(?:test|spec|features)/}) } From b85b2b84ad407b4653b6762ee61db0676655b21d Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Thu, 28 Aug 2025 15:32:08 +0900 Subject: [PATCH 5/5] [ruby/error_highlight] Remove a branch for Ruby 3.1 https://github.com/ruby/error_highlight/commit/d3063cde62 --- lib/error_highlight/base.rb | 75 +++++++++----------- test/error_highlight/test_error_highlight.rb | 25 ++----- 2 files changed, 40 insertions(+), 60 deletions(-) diff --git a/lib/error_highlight/base.rb b/lib/error_highlight/base.rb index b4a31f8e802019..a4c65c63e687fd 100644 --- a/lib/error_highlight/base.rb +++ b/lib/error_highlight/base.rb @@ -122,56 +122,51 @@ def initialize(node, point_type: :name, name: nil) end end - OPT_GETCONSTANT_PATH = (RUBY_VERSION.split(".").map {|s| s.to_i } <=> [3, 2]) >= 0 - private_constant :OPT_GETCONSTANT_PATH - def spot return nil unless @node - if OPT_GETCONSTANT_PATH - # In Ruby 3.2 or later, a nested constant access (like `Foo::Bar::Baz`) - # is compiled to one instruction (opt_getconstant_path). - # @node points to the node of the whole `Foo::Bar::Baz` even if `Foo` - # or `Foo::Bar` causes NameError. - # So we try to spot the sub-node that causes the NameError by using - # `NameError#name`. - case @node.type - when :COLON2 - subnodes = [] - node = @node - while node.type == :COLON2 - node2, const = node.children - subnodes << node if const == @name - node = node2 - end - if node.type == :CONST || node.type == :COLON3 - if node.children.first == @name - subnodes << node - end - - # If we found only one sub-node whose name is equal to @name, use it - return nil if subnodes.size != 1 - @node = subnodes.first - else - # Do nothing; opt_getconstant_path is used only when the const base is - # NODE_CONST (`Foo`) or NODE_COLON3 (`::Foo`) - end - when :constant_path_node - subnodes = [] - node = @node - - begin - subnodes << node if node.name == @name - end while (node = node.parent).is_a?(Prism::ConstantPathNode) - - if node.is_a?(Prism::ConstantReadNode) && node.name == @name + # In Ruby 3.2 or later, a nested constant access (like `Foo::Bar::Baz`) + # is compiled to one instruction (opt_getconstant_path). + # @node points to the node of the whole `Foo::Bar::Baz` even if `Foo` + # or `Foo::Bar` causes NameError. + # So we try to spot the sub-node that causes the NameError by using + # `NameError#name`. + case @node.type + when :COLON2 + subnodes = [] + node = @node + while node.type == :COLON2 + node2, const = node.children + subnodes << node if const == @name + node = node2 + end + if node.type == :CONST || node.type == :COLON3 + if node.children.first == @name subnodes << node end # If we found only one sub-node whose name is equal to @name, use it return nil if subnodes.size != 1 @node = subnodes.first + else + # Do nothing; opt_getconstant_path is used only when the const base is + # NODE_CONST (`Foo`) or NODE_COLON3 (`::Foo`) end + when :constant_path_node + subnodes = [] + node = @node + + begin + subnodes << node if node.name == @name + end while (node = node.parent).is_a?(Prism::ConstantPathNode) + + if node.is_a?(Prism::ConstantReadNode) && node.name == @name + subnodes << node + end + + # If we found only one sub-node whose name is equal to @name, use it + return nil if subnodes.size != 1 + @node = subnodes.first end case @node.type diff --git a/test/error_highlight/test_error_highlight.rb b/test/error_highlight/test_error_highlight.rb index 09b0579f8aabdf..be3e3607332ca4 100644 --- a/test/error_highlight/test_error_highlight.rb +++ b/test/error_highlight/test_error_highlight.rb @@ -891,27 +891,13 @@ def test_COLON2_4 end end - if ErrorHighlight.const_get(:Spotter).const_get(:OPT_GETCONSTANT_PATH) - def test_COLON2_5 - # Unfortunately, we cannot identify which `NotDefined` caused the NameError - assert_error_message(NameError, <<~END) do - uninitialized constant ErrorHighlightTest::NotDefined - END - - ErrorHighlightTest::NotDefined::NotDefined - end - end - else - def test_COLON2_5 - assert_error_message(NameError, <<~END) do + def test_COLON2_5 + # Unfortunately, we cannot identify which `NotDefined` caused the NameError + assert_error_message(NameError, <<~END) do uninitialized constant ErrorHighlightTest::NotDefined + END - ErrorHighlightTest::NotDefined::NotDefined - ^^^^^^^^^^^^ - END - - ErrorHighlightTest::NotDefined::NotDefined - end + ErrorHighlightTest::NotDefined::NotDefined end end @@ -1617,7 +1603,6 @@ def test_wrong_number_of_arguments_for_lambda_method end def test_wrong_number_of_arguments_for_define_method - v = lambda { } lineno = __LINE__ assert_error_message(ArgumentError, <<~END) do wrong number of arguments (given 1, expected 2) (ArgumentError)