From bf8d550616b8afb5a2dc9b8f5aad5bc8b10acfcc Mon Sep 17 00:00:00 2001 From: ksss Date: Mon, 19 Oct 2026 11:14:19 +0900 Subject: [PATCH 1/2] Attach comments above annotations to constant and global declarations parse_const_decl and parse_global_decl did not receive comment_pos, so they looked up the comment only from the line of the name token. Other declarations (class, module, interface, type alias, members) use the start of the annotations as the base line, so a comment placed above the annotations was silently dropped for constants and globals: module M # const doc %a{deprecated} C: bool # => comment was nil end # doc %a{deprecated} $g: String # => comment was nil `rbs annotate` inserts new comments above existing annotations, so it produced this layout itself and the result could not be read back. Pass the annotation start position to both functions. When no comment is found above the annotations, fall back to the line of the name so that the existing `%a{...}` / `# comment` / `CONST: T` layout keeps working. Co-Authored-By: Claude Opus 5.5 --- src/parser.c | 31 ++++++--- test/rbs/signature_parsing_test.rb | 105 +++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 8 deletions(-) diff --git a/src/parser.c b/src/parser.c index 5effb219e..48844bb9e 100644 --- a/src/parser.c +++ b/src/parser.c @@ -1667,15 +1667,30 @@ bool rbs_parse_method_type(rbs_parser_t *parser, rbs_method_type_t **method_type return true; } +/* + Constant and global declarations accept the comment either above the annotations + (like other declarations) or between the annotations and the name. +*/ +static rbs_ast_comment_t *parse_const_global_comment(rbs_parser_t *parser, rbs_position_t comment_pos, rbs_position_t decl_start) { + comment_pos = rbs_nonnull_pos_or(comment_pos, decl_start); + + rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, comment_pos.line); + if (comment == NULL && comment_pos.line != decl_start.line) { + comment = rbs_parser_get_comment(parser, decl_start.line); + } + + return comment; +} + /* global_decl ::= {tGIDENT} `:` */ RBS_NODISCARD -static bool parse_global_decl(rbs_parser_t *parser, rbs_node_list_t *annotations, rbs_ast_declarations_global_t **global) { +static bool parse_global_decl(rbs_parser_t *parser, rbs_position_t comment_pos, rbs_node_list_t *annotations, rbs_ast_declarations_global_t **global) { rbs_range_t decl_range; decl_range.start = parser->current_token.range.start; - rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, decl_range.start.line); + rbs_ast_comment_t *comment = parse_const_global_comment(parser, comment_pos, decl_range.start); rbs_range_t name_range = parser->current_token.range; @@ -1696,11 +1711,11 @@ static bool parse_global_decl(rbs_parser_t *parser, rbs_node_list_t *annotations const_decl ::= {const_name} `:` */ RBS_NODISCARD -static bool parse_const_decl(rbs_parser_t *parser, rbs_node_list_t *annotations, rbs_ast_declarations_constant_t **constant) { +static bool parse_const_decl(rbs_parser_t *parser, rbs_position_t comment_pos, rbs_node_list_t *annotations, rbs_ast_declarations_constant_t **constant) { rbs_range_t decl_range; decl_range.start = parser->current_token.range.start; - rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, decl_range.start.line); + rbs_ast_comment_t *comment = parse_const_global_comment(parser, comment_pos, decl_range.start); rbs_range_t name_range; rbs_type_name_t *type_name = NULL; @@ -2999,13 +3014,13 @@ static bool parse_nested_decl(rbs_parser_t *parser, const char *nested_in, rbs_p case tUIDENT: case pCOLON2: { rbs_ast_declarations_constant_t *constant = NULL; - CHECK_PARSE(parse_const_decl(parser, annotations, &constant)); + CHECK_PARSE(parse_const_decl(parser, annot_pos, annotations, &constant)); *decl = (rbs_node_t *) constant; break; } case tGIDENT: { rbs_ast_declarations_global_t *global = NULL; - CHECK_PARSE(parse_global_decl(parser, annotations, &global)); + CHECK_PARSE(parse_global_decl(parser, annot_pos, annotations, &global)); *decl = (rbs_node_t *) global; break; } @@ -3055,13 +3070,13 @@ static bool parse_decl(rbs_parser_t *parser, rbs_node_t **decl) { case tUIDENT: case pCOLON2: { rbs_ast_declarations_constant_t *constant = NULL; - CHECK_PARSE(parse_const_decl(parser, annotations, &constant)); + CHECK_PARSE(parse_const_decl(parser, annot_pos, annotations, &constant)); *decl = (rbs_node_t *) constant; return true; } case tGIDENT: { rbs_ast_declarations_global_t *global = NULL; - CHECK_PARSE(parse_global_decl(parser, annotations, &global)); + CHECK_PARSE(parse_global_decl(parser, annot_pos, annotations, &global)); *decl = (rbs_node_t *) global; return true; } diff --git a/test/rbs/signature_parsing_test.rb b/test/rbs/signature_parsing_test.rb index bcf666f5a..53825c76d 100644 --- a/test/rbs/signature_parsing_test.rb +++ b/test/rbs/signature_parsing_test.rb @@ -2340,6 +2340,111 @@ def test_constant__annotation end end + def test_constant__comment_and_annotation + Parser.parse_signature(<<~RBS).tap do |_, _, decls| + # Comment above annotation + %a{deprecated} + Foo::BAR: String + + %a{deprecated} + # Comment below annotation + Foo::BAZ: String + + # Comment without annotation + Foo::QUX: String + RBS + + assert_equal 3, decls.size + + decls[0].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment above annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + decls[1].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment below annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + decls[2].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment without annotation\n", decl.comment&.string + assert_empty decl.annotations + end + end + end + + def test_nested_constant__comment_and_annotation + Parser.parse_signature(<<~RBS).tap do |_, _, decls| + module M + # Comment above annotation + %a{deprecated} + BAR: String + + %a{deprecated} + # Comment below annotation + BAZ: String + + # Comment without annotation + QUX: String + end + RBS + + members = decls[0].members + assert_equal 3, members.size + + members[0].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment above annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + members[1].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment below annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + members[2].tap do |decl| + assert_instance_of RBS::AST::Declarations::Constant, decl + assert_equal "Comment without annotation\n", decl.comment&.string + assert_empty decl.annotations + end + end + end + + def test_global__comment_and_annotation + Parser.parse_signature(<<~RBS).tap do |_, _, decls| + # Comment above annotation + %a{deprecated} + $bar: String + + %a{deprecated} + # Comment below annotation + $baz: String + + # Comment without annotation + $qux: String + RBS + + assert_equal 3, decls.size + + decls[0].tap do |decl| + assert_instance_of RBS::AST::Declarations::Global, decl + assert_equal "Comment above annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + decls[1].tap do |decl| + assert_instance_of RBS::AST::Declarations::Global, decl + assert_equal "Comment below annotation\n", decl.comment&.string + assert_equal ["deprecated"], decl.annotations.map(&:string) + end + decls[2].tap do |decl| + assert_instance_of RBS::AST::Declarations::Global, decl + assert_equal "Comment without annotation\n", decl.comment&.string + assert_empty decl.annotations + end + end + end + def test__method_type__untyped_function_and_block assert_raises(RBS::ParsingError) do Parser.parse_signature(<<~RBS).tap do |_, _, decls| From d2db25f3cf547bc949b351277aa1ae68cd7e45ff Mon Sep 17 00:00:00 2001 From: ksss Date: Mon, 19 Oct 2026 14:53:23 +0900 Subject: [PATCH 2/2] Drop the fallback and treat comments the same as other declarations The fallback kept a comment placed between the annotations and the name of a constant or global. That layout only worked because comment_pos was not passed when annotations were added to constants and globals (1bc9bb4b, 075d27c8). Class, module, interface, type alias, and members have never accepted it. No annotated constant or global exists in core, stdlib, or gem_rbs_collection, so nothing relies on the old layout. Read the comment from the line above the annotations, the same as the other declarations. Co-Authored-By: Claude Opus 5.5 --- src/parser.c | 21 ++++----------------- test/rbs/signature_parsing_test.rb | 9 ++++++--- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/parser.c b/src/parser.c index 48844bb9e..fc6f59c72 100644 --- a/src/parser.c +++ b/src/parser.c @@ -1667,21 +1667,6 @@ bool rbs_parse_method_type(rbs_parser_t *parser, rbs_method_type_t **method_type return true; } -/* - Constant and global declarations accept the comment either above the annotations - (like other declarations) or between the annotations and the name. -*/ -static rbs_ast_comment_t *parse_const_global_comment(rbs_parser_t *parser, rbs_position_t comment_pos, rbs_position_t decl_start) { - comment_pos = rbs_nonnull_pos_or(comment_pos, decl_start); - - rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, comment_pos.line); - if (comment == NULL && comment_pos.line != decl_start.line) { - comment = rbs_parser_get_comment(parser, decl_start.line); - } - - return comment; -} - /* global_decl ::= {tGIDENT} `:` */ @@ -1689,8 +1674,9 @@ RBS_NODISCARD static bool parse_global_decl(rbs_parser_t *parser, rbs_position_t comment_pos, rbs_node_list_t *annotations, rbs_ast_declarations_global_t **global) { rbs_range_t decl_range; decl_range.start = parser->current_token.range.start; + comment_pos = rbs_nonnull_pos_or(comment_pos, decl_range.start); - rbs_ast_comment_t *comment = parse_const_global_comment(parser, comment_pos, decl_range.start); + rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, comment_pos.line); rbs_range_t name_range = parser->current_token.range; @@ -1715,7 +1701,8 @@ static bool parse_const_decl(rbs_parser_t *parser, rbs_position_t comment_pos, r rbs_range_t decl_range; decl_range.start = parser->current_token.range.start; - rbs_ast_comment_t *comment = parse_const_global_comment(parser, comment_pos, decl_range.start); + comment_pos = rbs_nonnull_pos_or(comment_pos, decl_range.start); + rbs_ast_comment_t *comment = rbs_parser_get_comment(parser, comment_pos.line); rbs_range_t name_range; rbs_type_name_t *type_name = NULL; diff --git a/test/rbs/signature_parsing_test.rb b/test/rbs/signature_parsing_test.rb index 53825c76d..04eec0ab0 100644 --- a/test/rbs/signature_parsing_test.rb +++ b/test/rbs/signature_parsing_test.rb @@ -2363,7 +2363,8 @@ def test_constant__comment_and_annotation end decls[1].tap do |decl| assert_instance_of RBS::AST::Declarations::Constant, decl - assert_equal "Comment below annotation\n", decl.comment&.string + # Same as class, module, and other declarations + assert_nil decl.comment assert_equal ["deprecated"], decl.annotations.map(&:string) end decls[2].tap do |decl| @@ -2400,7 +2401,8 @@ module M end members[1].tap do |decl| assert_instance_of RBS::AST::Declarations::Constant, decl - assert_equal "Comment below annotation\n", decl.comment&.string + # Same as class, module, and other declarations + assert_nil decl.comment assert_equal ["deprecated"], decl.annotations.map(&:string) end members[2].tap do |decl| @@ -2434,7 +2436,8 @@ def test_global__comment_and_annotation end decls[1].tap do |decl| assert_instance_of RBS::AST::Declarations::Global, decl - assert_equal "Comment below annotation\n", decl.comment&.string + # Same as class, module, and other declarations + assert_nil decl.comment assert_equal ["deprecated"], decl.annotations.map(&:string) end decls[2].tap do |decl|