Fix NoMethodError when RBS file contains class/module alias decl#443
Closed
rhiroe wants to merge 1 commit intoruby:masterfrom
Closed
Fix NoMethodError when RBS file contains class/module alias decl#443rhiroe wants to merge 1 commit intoruby:masterfrom
rhiroe wants to merge 1 commit intoruby:masterfrom
Conversation
`AST.create_rbs_decl` has an empty `when RBS::AST::Declarations::AliasDecl`
branch that returns nil, but `AST.parse_rbs` did not `.compact` the result,
so the nil reached `decls.each {|decl| decl.define(@GenV) }` and crashed
the LSP server. The two other callers of `create_rbs_decl` /
`create_rbs_member` already use `.compact`; this aligns `parse_rbs` with
the same pattern.
4 tasks
Member
Contributor
Author
|
@mame Yes, you're right — I think it will become unnecessary. I'll leave it up to you whether to merge or close this PR, but since typeprof/lib/typeprof/core/env.rb Lines 265 to 267 in 1aedffb I think it would also be fine to either remove that one as well for consistency, or merge this PR to align with the existing similar code. |
Member
|
I have merged #444, so closing this. Thanks!
That's sloppy ;-P |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
NoMethodError: undefined method 'define' for nilraised inService#update_rbs_filewhenever the workspace contains an RBS file with a class alias or module alias declaration (e.g.module YAML = Psych,class HashWithIndifferentAccess = ActiveSupport::HashWithIndifferentAccess).Background
AST.create_rbs_declhas an intentionally emptywhen RBS::AST::Declarations::AliasDeclbranch (class/module aliases are not yet implemented and are silently skipped).However the empty
whenbody returnsnil, and the calling siteAST.parse_rbscallsraw_decls.map { ... }without.compact, so thenilends up in the resulting array and later crashes atdecls.each {|decl| decl.define(@genv) }.The two other callers of
create_rbs_decl/create_rbs_memberalready use.compactafter themap(seeGenv#load_core_rbsincore/env.rbandSigModuleBaseNode#initializeincore/ast/sig_decl.rb). This PR alignsparse_rbswith the same pattern.Reproduction
Put a single line in
sig/foo.rbs:module YAML = PsychStart the LSP server: it crashes during workspace initialization with
Fix
Add
.compactto themapinAST.parse_rbs. Class/module aliases remain unimplemented (skipped) — proper alias resolution is out of scope for this PR.Test plan
scenario/rbs/class-module-alias.rbregression testbundle exec rake testpasses (415 tests, 0 failures)module YAML = Psychno longer crashes the LSP server