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

Bump rbs from v3.6.0 to v3.7.0 and steep from v1.8.0 to v1.8.1 #481

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ydah
Copy link
Collaborator

@ydah ydah commented Dec 9, 2024

The following warnings and errors have been fixed as a result of the version update:

bundle exec steep check
# Type checking files:

............F.........................F..................................F...FF.......F......

lib/lrama/grammar/parameterizing_rule/rhs.rb:19:23: [warning] Empty hash doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└           var_to_arg = {}
                         ~~

lib/lrama/bitmap.rb:16:10: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└       a = []
            ~~

lib/lrama/grammar/rule_builder.rb:70:28: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└         resolved_builders = []
                              ~~

lib/lrama/lexer/token/user_code.rb:19:23: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└           references = []
                         ~~

lib/lrama/state/reduce.rb:30:36: [error] Type annotation has a syntax error: Failed to parse a type in annotation
│ Diagnostic ID: Ruby::AnnotationSyntaxError
│
└           # @type ivar @look_ahead: Array<Grammar::Symbol>
                                      ~~~~~~~~~~~~~~~~~~~~~~

lib/lrama/grammar.rb:385:15: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└       errors = []
                 ~~

Detected 6 problems from 6 files
rake aborted!

@yui-knk
Copy link
Collaborator

yui-knk commented Dec 15, 2024

Is it possible to use inline type annotation for var = [] and var = {}?
I feel it's tricky to use Enumerator.new and each_with_object to avoid type check.

@@ -26,8 +26,7 @@ def add_not_selected_symbol(sym)
end

def selected_look_ahead
if @look_ahead
# @type ivar @look_ahead: Array<Grammar::Symbol>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect lib/lrama/state/reduce.rb:30:36: [error] Type annotation has a syntax error: Failed to parse a type in annotation might be a bug on steep. Do you have any idea?

Copy link
Collaborator

@yui-knk yui-knk Dec 15, 2024

Choose a reason for hiding this comment

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

For example, these diff makes the code syntax valid, even so type check is failed.

diff --git a/Gemfile b/Gemfile
index 58c28f7..870fde0 100644
--- a/Gemfile
+++ b/Gemfile
@@ -15,6 +15,6 @@ gem "memory_profiler"
 # Recent steep requires Ruby >= 3.0.0.
 # Then skip install on some CI jobs.
 if !ENV['GITHUB_ACTION'] || ENV['INSTALL_STEEP'] == 'true'
-  gem "rbs", "3.6.0", require: false
-  gem "steep", "1.8.0", require: false
+  gem "rbs", "3.7.0", require: false
+  gem "steep", "1.9.1", require: false
 end
diff --git a/lib/lrama/state/reduce.rb b/lib/lrama/state/reduce.rb
index a2b7c26..0e67b77 100644
--- a/lib/lrama/state/reduce.rb
+++ b/lib/lrama/state/reduce.rb
@@ -27,7 +27,7 @@ module Lrama

       def selected_look_ahead
         if @look_ahead
-          # @type ivar @look_ahead: Array<Grammar::Symbol>
+          # @type ivar @look_ahead: Array
           @look_ahead - @not_selected_symbols
         else
           []
bundle exec steep check
# Type checking files:

.................F.............................FF.......F....................F...F...........

lib/lrama/grammar/parameterizing_rule/rhs.rb:19:23: [warning] Empty hash doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection

└           var_to_arg = {}
                         ~~

lib/lrama/state/reduce.rb:31:22: [error] Type `(::Array[::Lrama::Grammar::Symbol] | nil)` does not have method `-`
│ Diagnostic ID: Ruby::NoMethod

└           @look_ahead - @not_selected_symbols
                        ~

lib/lrama/lexer/token/user_code.rb:19:23: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└           references = []
                         ~~

lib/lrama/bitmap.rb:16:10: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection

└       a = []
            ~~

lib/lrama/grammar/rule_builder.rb:70:28: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└         resolved_builders = []
                              ~~

lib/lrama/grammar.rb:385:15: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection

└       errors = []
                 ~~

Copy link
Collaborator

Choose a reason for hiding this comment

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

I applied monkey patches like below to steep-1.9.1/lib/steep/annotation_parser.rb.
It seems RBS parses codes without syntax error however type and string have different value.
So that "Failed to parse a type in annotation" SyntaxError is raised.

    def parse_type(match, name = :type, location:)
      string = match[name] or raise
      st, en = match.offset(name)
      st or raise
      en or raise
      loc = RBS::Location.new(location.buffer, location.start_pos + st, location.start_pos + en)

      type =
        begin
          RBS::Parser.parse_type(string)
        rescue RBS::ParsingError => exn
          p exn
          raise SyntaxError.new(source: string, location: loc, exn: exn)
        end or raise

      unless (type.location || raise).source == string.strip
        p type
        p type.location
        p type.location.source
        p string
        raise SyntaxError.new(source: string, location: loc, message: "Failed to parse a type in annotation")
      end

      factory.type(type)
    end
$ bundle exec steep check
# Type checking files:
...
#<RBS::Types::ClassInstance:0x000000012703b9e8 @name=#<RBS::TypeName:0x000000012703ba38 @namespace=#<RBS::Namespace:0x000000012703ba60 @path=[], @absolute=false>, @name=:Array, @kind=:class>, @args=[], @location=#<RBS::Location:32740 buffer=a.rbs, start=1:0, pos=0...5, children=name,?args source="Array">>
#<RBS::Location:32740 buffer=a.rbs, start=1:0, pos=0...5, children=name,?args source="Array">
"Array"
"Array<Grammar::Symbol>"

ydah added 5 commits December 15, 2024 10:31
```
lib/lrama/grammar/parameterizing_rule/rhs.rb:19:23: [warning] Empty hash doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└           var_to_arg = {}
                         ~~
```
```
lib/lrama/bitmap.rb:16:10: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└       a = []
            ~~
```
```
lib/lrama/grammar/rule_builder.rb:70:28: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└         resolved_builders = []
                              ~~
```
```
lib/lrama/lexer/token/user_code.rb:19:23: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└           references = []
                         ~~
```
```
lib/lrama/grammar.rb:385:15: [warning] Empty array doesn't have type annotation
│ Diagnostic ID: Ruby::UnannotatedEmptyCollection
│
└       errors = []
                 ~~
```
@ydah ydah force-pushed the bump-rbs-and-steep branch from 1507cf9 to a48c27d Compare December 15, 2024 01:35
@ydah ydah marked this pull request as draft December 15, 2024 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants