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

Rails/LexicallyScopedActionFilter outputs error for %I literal #5524

Closed
yskkin opened this issue Jan 29, 2018 · 0 comments
Closed

Rails/LexicallyScopedActionFilter outputs error for %I literal #5524

yskkin opened this issue Jan 29, 2018 · 0 comments

Comments

@yskkin
Copy link

yskkin commented Jan 29, 2018

Expected behavior

No offences are detected.

Actual behavior

Inspecting 1 file
Scanning /Users/yskkin/projects/test/app/controllers/hoge_controller.rb
An error occurred while Rails/LexicallyScopedActionFilter cop was inspecting /Users/yskkin/projects/test/app/controllers/hoge_controller.rb:4:2.
undefined method `value' for s(:sym, :index):RuboCop::AST::StrNode
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb:94:in `block in array_values'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb:89:in `map'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb:89:in `array_values'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/rails/lexically_scoped_action_filter.rb:71:in `on_send'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:44:in `block (2 levels) in on_send'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:109:in `with_cop_error_handling'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:43:in `block in on_send'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:42:in `each'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:42:in `on_send'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/ast/traversal.rb:49:in `block in on_begin'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/ast/traversal.rb:49:in `each'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/ast/traversal.rb:49:in `on_begin'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:48:in `on_begin'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/ast/traversal.rb:88:in `on_class'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:48:in `on_class'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/ast/traversal.rb:12:in `walk'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/commissioner.rb:60:in `investigate'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/team.rb:114:in `investigate'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/team.rb:102:in `offenses'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cop/team.rb:44:in `inspect_file'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:258:in `inspect_file'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:205:in `block in do_inspection_loop'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:237:in `block in iterate_until_no_changes'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:230:in `loop'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:230:in `iterate_until_no_changes'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:201:in `do_inspection_loop'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:111:in `block in file_offenses'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:121:in `file_offense_cache'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:109:in `file_offenses'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:100:in `process_file'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:78:in `block in each_inspected_file'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:75:in `each'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:75:in `reduce'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:75:in `each_inspected_file'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:67:in `inspect_files'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/runner.rb:39:in `run'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cli.rb:150:in `execute_runner'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cli.rb:78:in `execute_runners'
/usr/local/Cellar/rbenv/1.1.1/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/lib/rubocop/cli.rb:38:in `run'
/usr/local/opt/rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/bin/rubocop:13:in `block in <top (required)>'
/usr/local/opt/rbenv/versions/2.4.0/lib/ruby/2.4.0/benchmark.rb:308:in `realtime'
/usr/local/opt/rbenv/versions/2.4.0/lib/ruby/gems/2.4.0/gems/rubocop-0.52.1/bin/rubocop:12:in `<top (required)>'
/usr/local/opt/rbenv/versions/2.4.0/bin/rubocop:22:in `load'
/usr/local/opt/rbenv/versions/2.4.0/bin/rubocop:22:in `<main>'
.

1 file inspected, no offenses detected

1 error occurred:
An error occurred while Rails/LexicallyScopedActionFilter cop was inspecting /Users/yskkin/projects/test/app/controllers/hoge_controller.rb:4:2.
Errors are usually caused by RuboCop bugs.
Please, report your problems to RuboCop's issue tracker.
Mention the following information in the issue report:
0.52.1 (using Parser 2.4.0.2, running on ruby 2.4.0 x86_64-darwin16)
Finished in 0.36732600000686944 seconds

Steps to reproduce the problem

  1. prepare an controller file app/controller/hoge_controller as follows
class HogeController < ApplicationController
  before_action :foo, except: %I[index show]

  def index
  end

  def show
  end
end
  1. execute rubocop -d --only Rails/LexicallyScopedActionFilter app/controllers/hoge_controller.rb

RuboCop version

rubocop -V
0.52.1 (using Parser 2.4.0.2, running on ruby 2.4.0 x86_64-darwin16)

Note

If a line in the controller was before_action :foo, except: %I[index show] (not %I, but %i), behavior is expected.

wata727 added a commit to wata727/rubocop that referenced this issue Jan 31, 2018
Fixes rubocop#5524

When parsing the following, we get `StrNode` instead of `SymbolNode`.

```ruby
%I[index]
```

```
$ pry -rparser -rparser/ruby24 -rrubocop
[1] pry(main)> buffer = Parser::Source::Buffer.new('(string)')
[2] pry(main)> buffer.source = "%I[index]"
[3] pry(main)> builder = RuboCop::AST::Builder.new
[4] pry(main)> parser = Parser::Ruby24.new(builder)
[5] pry(main)> node = parser.parse(buffer)
=> s(:array,
  s(;sym, :index))
[6] pry(main)> node.children.first.type
=> :sym
[7] pry(main)> node.children.first.class
=> RuboCop::AST::StrNode # Why?
```

This is because the parser gem updates the node itself in
`Parser::AST::Node#symbols_compose` when processing `tSYMBOLS_BEG`.

See also:
https://github.com/whitequark/parser/blob/v2.4.0.2/lib/parser/ruby24.y#L1738
https://github.com/whitequark/parser/blob/v2.4.0.2/lib/parser/builders/default.rb#L298

However, since RuboCop overrides `Parser::AST::Node#updated`, even if
the type is updated, it will use the current class as it is.

In order to prevent this, it creates an instance based on type instead
of `self` class.
bbatsov pushed a commit that referenced this issue Feb 2, 2018
Fixes #5524

When parsing the following, we get `StrNode` instead of `SymbolNode`.

```ruby
%I[index]
```

```
$ pry -rparser -rparser/ruby24 -rrubocop
[1] pry(main)> buffer = Parser::Source::Buffer.new('(string)')
[2] pry(main)> buffer.source = "%I[index]"
[3] pry(main)> builder = RuboCop::AST::Builder.new
[4] pry(main)> parser = Parser::Ruby24.new(builder)
[5] pry(main)> node = parser.parse(buffer)
=> s(:array,
  s(;sym, :index))
[6] pry(main)> node.children.first.type
=> :sym
[7] pry(main)> node.children.first.class
=> RuboCop::AST::StrNode # Why?
```

This is because the parser gem updates the node itself in
`Parser::AST::Node#symbols_compose` when processing `tSYMBOLS_BEG`.

See also:
https://github.com/whitequark/parser/blob/v2.4.0.2/lib/parser/ruby24.y#L1738
https://github.com/whitequark/parser/blob/v2.4.0.2/lib/parser/builders/default.rb#L298

However, since RuboCop overrides `Parser::AST::Node#updated`, even if
the type is updated, it will use the current class as it is.

In order to prevent this, it creates an instance based on type instead
of `self` class.
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

No branches or pull requests

1 participant