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

Auto correct of RSpec/ExampleWording crashes #402

Closed
phansch opened this issue Apr 25, 2017 · 4 comments · Fixed by #403
Closed

Auto correct of RSpec/ExampleWording crashes #402

phansch opened this issue Apr 25, 2017 · 4 comments · Fixed by #403
Labels

Comments

@phansch
Copy link

phansch commented Apr 25, 2017

Hello! I have been using rubocop-rspec without any issues so far and like it a lot. However, today I started to use it on a bigger project and I found the following example that crashes the auto correct:

# model_spec.rb
it 'It does something' do
  # ^ Note the uppercase i
end

If I run it without the autocorrect, the offense is registered correctly:

$ rubocop --only RSpec/ExampleWording model_spec.rb 
Inspecting 1 file
C

Offenses:

model_spec.rb:1:5: C: Do not repeat 'it' when describing your tests.
it 'It does something' do
    ^^^^^^^^^^^^^^^^^

However, it breaks with the --auto-correct flag:

$ rubocop --only RSpec/ExampleWording --auto-correct model_spec.rb
Inspecting 1 file


0 files inspected, no offenses detected
no implicit conversion of nil into String
/home/phansch/.rvm/gems/ruby-2.3.1/gems/parser-2.3.3.1/lib/parser/source/rewriter.rb:156:in `[]='
/home/phansch/.rvm/gems/ruby-2.3.1/gems/parser-2.3.3.1/lib/parser/source/rewriter.rb:156:in `block in process'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/parser-2.3.3.1/lib/parser/source/rewriter.rb:152:in `each'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/parser-2.3.3.1/lib/parser/source/rewriter.rb:152:in `process'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/corrector.rb:63:in `rewrite'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:143:in `autocorrect_all_cops'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:80:in `autocorrect'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:108:in `block in offenses'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:125:in `investigate'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:104:in `offenses'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/cop/team.rb:54:in `inspect_file'
/home/phansch/.rvm/gems/ruby-2.3.1/gems/rubocop-0.48.1/lib/rubocop/runner.rb:248:in `inspect_file'

I guess that's because replacement_text returns nil in that case. I probably have time to look into a fix later today.

@bquorning
Copy link
Collaborator

That cop has a few inconsistencies:

  • if text.start_with?('should') should use SHOULD_PREFIX instead.
  • #on_block downcases the message, which #replacement_text doesn’t do, which causes the error you’re seeing.
  • IT_PREFIX includes a whitespace, but SHOULD_PREFIX doesn’t.

@dgollahon
Copy link
Contributor

dgollahon commented Apr 25, 2017

Thanks for the report, @phansch! I added the it functionality so I'm happy to fix the bug after work tonight. If you prefer to submit a patch before tonight, that's fine too!

@bquorning:

IT_PREFIX includes a whitespace, but SHOULD_PREFIX doesn’t.

iirc, this is necessary for some cases of shouldn't or similar words that include should. I deliberately included the space for the it since we only care about exactly that word followed by a space.

@dgollahon
Copy link
Contributor

Slightly o/t, but this cop is a good argument for #232 or something kind of like it. I realized after implementing the repeated it behavior that it's a shame that the offenses can't easily be independently flagged or corrected. They do both belong to the idea of "example wording" though, so it didn't feel right to have a separate cop either.

dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  'it' and 'should' case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased 'Should's, 'IT's, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves "shouldn't be" handling
  * original text: shouldn't be true
  * correction before: does not be true
  * correction after: is not true
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: should WISH ME LUCK, should WORRIES
  * corrections before:  WISHs ME LUCK, WORRYs
  * corrections after: WISHES ME LUCK, WORRIES
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRIES`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRY`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRY`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
- Adds pluralization support for words ending in `z`
  * ex: `buzz` -> `buzzes`
dgollahon added a commit that referenced this issue Apr 26, 2017
- Fixes #402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRY`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
- Adds pluralization support for words ending in `z`
  * ex: `buzz` -> `buzzes`
@dgollahon dgollahon added the bug label Apr 26, 2017
@phansch
Copy link
Author

phansch commented Apr 26, 2017

@dgollahon I won't have more time for this unfortunately and I see you already have a PR open. Thanks for the quick response 🎉

pirj pushed a commit to pirj/rubocop-rspec that referenced this issue Nov 7, 2017
- Fixes rubocop#402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRY`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
- Adds pluralization support for words ending in `z`
  * ex: `buzz` -> `buzzes`
markburns pushed a commit to markburns/rubocop-rails-ddd that referenced this issue Nov 7, 2017
- Fixes rubocop#402
- Previously the #on_block method would correctly flag cases of leading
  `it` and `should` case-insensitively by downcasing the docstring but
  in the autocorrect phase the message casing  was left untouched which
  caused a nil bug for upper cased `Should`s, `IT`s, etc.
- Prevents words starting with `should` from being flagged
  * ex: `it 'shoulders the burden' {}`
- Improves `shouldn't be` handling
  * original docstring: `shouldn't be true`
  * correction before: `does not be true`
  * correction after: `is not true`
- Fixes the pluralizer so that it is not confused by uppercase letters
  * original docstrings: `should WISH ME LUCK`, `should WORRY`
  * corrections before: `WISHs ME LUCK`, `WORRYs`
  * corrections after: `WISHES ME LUCK`, `WORRIES`
- If a whole word being corrected is completely uppercase, the suffix
  appended will also be uppercase. If the word is mixed case, the
  lowercase suffix will be appended.
- Performs general refactoring.enter the commit message for your changes. Lines starting
- Adds pluralization support for words ending in `z`
  * ex: `buzz` -> `buzzes`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants