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

Fix bug for invalid bytes in UTF-8 in Lint/PercentStringArray cop #3425

Conversation

pocke
Copy link
Collaborator

@pocke pocke commented Aug 18, 2016

When RuboCop analyses the following code, RuboCop crashes.

test.rb

%W(\255)

Reproduce

$ rubocop --debug test.rb

Expected behavior

RuboCop doesn't crash.

Actual behavior

got the following error.

For /Users/masataka-kuwabara/ghq/github.com/bbatsov/rubocop: configuration from /Users/masataka-kuwabara/ghq/github.com/bbatsov/rubocop/.rubocop.yml
Inheriting configuration from /Users/masataka-kuwabara/ghq/github.com/bbatsov/rubocop/.rubocop_todo.yml
Default configuration from /Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/config/default.yml
Inheriting configuration from /Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/config/enabled.yml
Inheriting configuration from /Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/config/disabled.yml
Inspecting 1 file
Scanning /Users/masataka-kuwabara/ghq/github.com/bbatsov/rubocop/test.rb
invalid byte sequence in UTF-8
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:39:in `gsub'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:39:in `block in contains_quotes_or_commas?'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:35:in `any?'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:35:in `contains_quotes_or_commas?'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:25:in `on_percent_literal'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/mixin/percent_literal.rb:15:in `process'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/lint/percent_string_array.rb:21:in `on_array'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:42:in `block (2 levels) in on_array'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:97:in `with_cop_error_handling'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:41:in `block in on_array'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:40:in `each'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:40:in `on_array'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/ast_node/traversal.rb:13:in `walk'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/commissioner.rb:59:in `investigate'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/team.rb:121:in `investigate'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/team.rb:109:in `offenses'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cop/team.rb:52:in `inspect_file'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:228:in `inspect_file'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:198:in `block in do_inspection_loop'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:188:in `loop'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:188:in `do_inspection_loop'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:93:in `block in file_offenses'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:103:in `file_offense_cache'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:91:in `file_offenses'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:82:in `process_file'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:59:in `block in inspect_files'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:57:in `each'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:57:in `inspect_files'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/runner.rb:35:in `run'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cli.rb:72:in `execute_runner'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/lib/rubocop/cli.rb:28:in `run'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/bin/rubocop:14:in `block in <top (required)>'
/Users/masataka-kuwabara/.rbenv/versions/2.3.1/lib/ruby/2.3.0/benchmark.rb:308:in `realtime'
/Users/masataka-kuwabara/.gem/ruby/2.3.0/gems/rubocop-0.42.0/bin/rubocop:13:in `<top (required)>'
/Users/masataka-kuwabara/.rbenv/versions/2.3/bin/rubocop:23:in `load'
/Users/masataka-kuwabara/.rbenv/versions/2.3/bin/rubocop:23:in `<main>'
C

Offenses:

test.rb:1:1: C: Style/Encoding: Missing utf-8 encoding comment.
%W(\255)
^^^^^^^^
test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
%W(\255)
^

1 file inspected, 2 offenses detected
Finished in 0.10227428999860422 seconds

Note

scrub_rb gem is an implementation of String#scrub by pure ruby. String#scrub doesn't exist older than ruby 2.0. So, I've added the gem as a new runtime_dependency.

RuboCop version

current HEAD of master branch.

$ rubocop -V
0.42.0 (using Parser 2.3.1.2, running on ruby 2.3.1 x86_64-darwin15)

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Used the same coding conventions as the rest of the project.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • All tests are passing.
  • The new code doesn't generate RuboCop offenses.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

@pocke pocke force-pushed the fix-crashing-percent_string_array_with_invalid_utf8 branch 2 times, most recently from 54bb0dd to 7c4c306 Compare August 18, 2016 11:59
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 20, 2016

Do we really need a 3rd party gem for this? In general I try to limit RuboCop's dependencies.

@backus
Copy link
Contributor

backus commented Aug 21, 2016

Do we really need a 3rd party gem for this? In general I try to limit RuboCop's dependencies.

Note that scrub_rb also introduces a monkey patch

@pocke pocke force-pushed the fix-crashing-percent_string_array_with_invalid_utf8 branch from 7c4c306 to efe1bd5 Compare August 21, 2016 05:02
@pocke
Copy link
Collaborator Author

pocke commented Aug 21, 2016

@bbatsov Removed dependency for the gem.
I use encode method instead of the gem. ( https://github.com/bbatsov/rubocop/pull/3425/files#diff-61b7d3d18516b6f71f3c6e8aef4992aaR58 )

And I've added a test case.

@bbatsov bbatsov merged commit 02ee7fc into rubocop:master Aug 21, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 21, 2016

That's better. Thanks!

@pocke pocke deleted the fix-crashing-percent_string_array_with_invalid_utf8 branch August 21, 2016 07:53
Neodelf pushed a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016
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.

3 participants