Skip to content

Commit

Permalink
Fix incorrect return type in Walker.normalize_ast
Browse files Browse the repository at this point in the history
As of Solargraph 0.51.0, I started hitting exceptions from walker.rb like this:

```
[WARN] wrong number of arguments (given 3, expected 1..2)
.../solargraph-rails/lib/solargraph/rails/walker.rb:78:in `initialize'
/Users/broz/src/solargraph-rails/lib/solargraph/rails/walker.rb:67:in `new'
/Users/broz/src/solargraph-rails/lib/solargraph/rails/walker.rb:67:in `from_source'
/Users/broz/src/solargraph-rails/lib/solargraph/rails/model.rb:15:in `process'
/Users/broz/src/solargraph-rails/lib/solargraph-rails.rb:48:in `block in local'
/Users/broz/src/solargraph-rails/lib/solargraph-rails.rb:61:in `run_feature'
/Users/broz/src/solargraph-rails/lib/solargraph-rails.rb:48:in `local'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/convention.rb:28:in `block in for_local'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/3.1.0/set.rb:511:in `each_key'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/3.1.0/set.rb:511:in `each'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/convention.rb:27:in `for_local'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/source_map.rb:32:in `initialize'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/source_map.rb:158:in `new'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/source_map.rb:158:in `map'
/Users/broz/.rbenv/versions/3.1.6/lib/ruby/gems/3.1.0/gems/solargraph-0.51.0/lib/solargraph/api_map.rb:54:in `map'
/Users/broz/src/solargraph-rails/spec/helpers.rb:4:in `load_string'
/Users/broz/src/solargraph-rails/spec/solargraph-rails/model_spec.rb:80:in `block (3 levels) in <top (required)>'
```

It looks like it happens in cases where Solargraph's source.node is a ::Parser::AST::Node.  There's a special case in Walker.normalize_ast() that is triggering that does not look correct on its face - it should always return an array of nodes, since the result is splatted in Walker.from_source.

The spec added reproduces this in newer solargraphs.
  • Loading branch information
apiology committed Feb 14, 2025
1 parent 144cec0 commit d649d69
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/solargraph/rails/walker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def self.normalize_ast(source)
ast = source.node

if ast.is_a?(::Parser::AST::Node)
ast
[ast]
else
NodeParser.parse_with_comments(source.code, source.filename)
end
Expand Down
11 changes: 11 additions & 0 deletions spec/solargraph-rails/model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@ class Transaction < ActiveRecord::Base
assert_class_method(api_map, 'Transaction.positive', ['Class<Transaction>'])
end

it 'handles primary_abstract_class without breaking' do
expect do
load_string 'app/models/application_record.rb',
<<-RUBY
class ApplicationRecord < ActiveRecord::Base
primary_abstract_class
end
RUBY
end.not_to raise_error
end

it 'generates scope methods with parameters' do
load_string 'app/models/person.rb',
<<-RUBY
Expand Down

0 comments on commit d649d69

Please sign in to comment.