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 config#check to use loaded_path in warning #5278

Merged
merged 2 commits into from
Dec 21, 2017
Merged

Fix config#check to use loaded_path in warning #5278

merged 2 commits into from
Dec 21, 2017

Conversation

chrishulton
Copy link

@chrishulton chrishulton commented Dec 18, 2017

Due to a configuration refactor, the #path variable is no longer
available in config#check, which causes rubocop to crash with
NameError: undefined local variable or method path if the deprecation
check yields a deprecation warning.

This replaces the warning to use the instance variable #loaded_path
and adds a spec to cover the behavior.


This is a small bug that looks to have been introduced with this commit and is impacting version 0.52.

Without the code change, the spec fails with:

 NameError:
   undefined local variable or method `path' for #<RuboCop::Config:0x007fe191cc9a00>
 # ./lib/rubocop/config.rb:198:in `block in check'
 # ./lib/rubocop/config.rb:288:in `block in deprecation_check'
 # ./lib/rubocop/config.rb:282:in `each'
 # ./lib/rubocop/config.rb:282:in `deprecation_check'
 # ./lib/rubocop/config.rb:197:in `check'
 # ./spec/rubocop/config_spec.rb:533:in `block (4 levels) in <top (required)>'

This is also reproducible by defining the configuration:

AllCops:
  Excludes:
    - 'path'

which results in an exit with the NameError described above.

Please let me know if I should add anything else to this PR! Thanks!


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).
  • 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.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

@volmer
Copy link
Contributor

volmer commented Dec 19, 2017

Thank you! I was experiencing the same problem. I hope the gem gets patched soon 🙏

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 20, 2017

The change looks good to me. I'll leave it to @jonas054 to review it as well before merging this patch , though, as he worked on the change that caused the bug.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 20, 2017

P.S. This needs a changelog entry.

@chrishulton
Copy link
Author

Thanks, changelog added.

Due to a configuration refactor, the `#path` variable is no longer
available in `config#check`, which causes rubocop to crash with
`NameError: undefined local variable or method path` if the deprecation
check yields a deprecation warning.

This replaces the warning to use the instance variable `#loaded_path`
and adds a spec to cover the behavior.
@jonas054
Copy link
Collaborator

Yes, that was my fault. I missed that variable when I moved the method. The change looks good! 👍

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.

5 participants