Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support #detailed_message for RBS::ParsingError #1166

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

ksss
Copy link
Collaborator

@ksss ksss commented Dec 1, 2022

Exception#detailed_message is a new feature supported from Ruby v3.2.0 and above.
https://bugs.ruby-lang.org/issues/18564

I would like to propose that this feature be applied to RBS.

CLI usage.

$ ruby -v
ruby 3.2.0preview2 (2022-09-09 master 35cfc9a3bb) [arm64-darwin21]

$ cat parsing_error.rbs
class Foo
  deff aaa: () -> void
end

$ rbs -I parsing_error.rbs validate --silent
/Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:17:in `_parse_signature': parsing_error.rbs:2:2...2:6: Syntax error: unexpected token for class/module declaration member, token=`deff` (tLIDENT) (RBS::ParsingError)

    deff aaa: () -> void
    ^^^^

	from /Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:17:in `parse_signature'
	from /Users/ksss/src/github.com/ksss/rbs/lib/rbs/environment_loader.rb:162:in `block (2 levels) in each_decl'
	from /Users/ksss/src/github.com/ksss/rbs/lib/rbs/environment_loader.rb:132:in `each_file'
        (snip)

Script usage.

$ bundle exec irb -r rbs
irb(main):001:0> RBS::Parser.parse_type("singleton(foo)")
/Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:7:in `_parse_type': a.rbs:1:10...1:13: Syntax error: expected one of class/module/constant name, token=`foo` (tLIDENT) (RBS::ParsingError)

  singleton(foo)
            ^^^

	from /Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:7:in `parse_type'
	from (irb):1:in `<main>'
        (snip)

This feature is inspired by error_highlight.

Scope is limited to Ruby 3.2.0 or later and ParsingError combinations.

To support Ruby 3.1 or earlier, incompatible changes need to be made, so they are out of scope.

It is likely that other errors could also be supported, but I have limited the scope to ParsingError only to keep the PR scope small.

@soutaro
Copy link
Member

soutaro commented Dec 2, 2022

@ksss Thank you for this PR. Looks great!

Could you explain about the incompatible changes to support Ruby 3.1? It's totally fine to provide this feature for only 3.2+, but introducing incompatible changes also would be ok when we release a major update -- RBS 3.0.

@ksss
Copy link
Collaborator Author

ksss commented Dec 2, 2022

An incompatibility in Ruby 3.1 and earlier is a change in the #to_s of each error class.
The return value of #to_s will be multi-line and tests for matching error messages will fail.

e.g.

rbs/test/rbs/parser_test.rb

Lines 494 to 497 in 57c07bb

assert_equal(
'test.rbs:1:13...1:15: Syntax error: comma delimited type list is expected, token=`::` (pCOLON2)',
exn.message
)

More detailed explanation of the incompatibility can be found here.
https://bugs.ruby-lang.org/issues/18564#Motivation

However, since RBS is a relatively new project, the impact may be minimal.

If we accept this incompatibility, we would put in a branch like error_highlight.

https://github.com/ruby/error_highlight/blob/5275078dc6f33148c0241160bed0b43753aae863/lib/error_highlight/core_ext.rb#L11-L36

`Exception#detailed_message` is a feature supported from Ruby v3.2.0 and above.
https://bugs.ruby-lang.org/issues/18564

It would help fix problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants