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

Make it a syntax error when the keys of keyword and record are duplicated. #1883

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

ksss
Copy link
Collaborator

@ksss ksss commented Jun 11, 2024

Ref #742

I have made it so that a syntax error is raised when required keyword arguments and optional keyword arguments have duplicate keys.

$ bundle exec ruby -r rbs -e 'RBS::Parser.parse_method_type("(a: top, a: top) -> void")'
/Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:15:in `_parse_method_type': a.rbs:1:9...1:10: Syntax error: duplicated keyword argument, token=`a` (tLIDENT) (RBS::ParsingError)

  (a: top, a: top) -> void
           ^

Similarly, for record types, I have made it so that a syntax error is raised when there are duplicate keys between required keys and optional keys.

$ bundle exec ruby -r rbs -e 'RBS::Parser.parse_type("{ a: top, a: top }")'
/Users/ksss/src/github.com/ksss/rbs/lib/rbs/parser_aux.rb:10:in `_parse_type': a.rbs:1:10...1:11: Syntax error: duplicated record key, token=`a` (tLIDENT) (RBS::ParsingError)

  { a: top, a: top }
            ^

Benchmark

There appears to be no noticeable deterioration.

$ bundle exec ruby benchmark/benchmark_new_env.rb

# master
ruby 3.3.2 (2024-05-30 revision e5a195edf6) [arm64-darwin22]
Warming up --------------------------------------
             new_env     1.000 i/100ms
       new_rails_env     1.000 i/100ms
Calculating -------------------------------------
             new_env      8.175 (±12.2%) i/s -     81.000 in  10.027816s
       new_rails_env      1.465 (± 0.0%) i/s -     15.000 in  10.281491s

# duplicate-keyword-arguments
ruby 3.3.2 (2024-05-30 revision e5a195edf6) [arm64-darwin22]
Warming up --------------------------------------
             new_env     1.000 i/100ms
       new_rails_env     1.000 i/100ms
Calculating -------------------------------------
             new_env      8.896 (±11.2%) i/s -     88.000 in  10.020704s
       new_rails_env      1.494 (± 0.0%) i/s -     15.000 in  10.076984s

ksss added 2 commits June 11, 2024 17:59
when the keys of keyword arguments are duplicated.
@soutaro soutaro added this to the RBS 3.6 milestone Jun 12, 2024
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@soutaro soutaro added this pull request to the merge queue Jun 13, 2024
Merged via the queue into ruby:master with commit ab1e613 Jun 13, 2024
18 of 19 checks passed
@ksss ksss deleted the duplicate-keyword-arguments branch June 13, 2024 02:37
@soutaro soutaro added the Released PRs already included in the released version label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants