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 an error for Style/RedundantStringEscape cop #11095

Merged

Conversation

koic
Copy link
Member

@koic koic commented Oct 21, 2022

This PR fixes the following error for Style/RedundantStringEscape cop when using ?\n string character literal.

% cat example.rb
?\n

% bundle exec rubocop --only Style/RedundantStringEscape -d
(snip)

An error occurred while Style/RedundantStringEscape cop was inspecting /Users/koic/src/github.com/koic/rubocop-issues/11090/example.rb:1:0.
undefined method `source' for nil:NilClass

          delimiters = [node.loc.begin.source[-1], node.loc.end.source[0]]
                                                               ^^^^^^^
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:162:in `delimiter?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:110:in `single_quoted?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:103:in `interpolation_not_enabled?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:85:in `allowed_escape?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:51:in `block in on_str'

Similar case to #11089, but not resolved by #11090. It also removes a bit of unnecessary implementation logic I found.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • 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.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

This PR fixes the following error for `Style/RedundantStringEscape` cop
when using `?\n` string character literal.

```console
% cat example.rb
?\n

% bundle exec rubocop --only Style/RedundantStringEscape -d
(snip)

An error occurred while Style/RedundantStringEscape cop was inspecting /Users/koic/src/github.com/koic/rubocop-issues/11090/example.rb:1:0.
undefined method `source' for nil:NilClass

          delimiters = [node.loc.begin.source[-1], node.loc.end.source[0]]
                                                               ^^^^^^^
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:162:in `delimiter?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:110:in `single_quoted?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:103:in `interpolation_not_enabled?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:85:in `allowed_escape?'
/Users/koic/src/github.com/rubocop/rubocop/lib/rubocop/cop/style/redundant_string_escape.rb:51:in `block in on_str'
```

Similar case to rubocop#11089, but not resolved by rubocop#11090.
It also removes a bit of unnecessary implementation logic I found.
@koic koic force-pushed the fix_an_error_for_style_redundant_string_escape branch from d063145 to 5583d05 Compare October 21, 2022 06:40
koic added a commit to koic/rubocop-ast that referenced this pull request Oct 21, 2022
This PR adds `character_literal?` to `StrNode`.

`...begin.is?('?')` used in rubocop/rubocop#11095
and `Style/CharacterLiteral` can be rewritten by `node.character_literal?`.
koic added a commit to koic/rubocop-ast that referenced this pull request Oct 21, 2022
This PR adds `character_literal?` to `StrNode`.

`...begin.is?('?')` used in rubocop/rubocop#11095
and `Style/CharacterLiteral` can be rewritten by `node.character_literal?`.
@@ -42,10 +42,9 @@ class RedundantStringEscape < Base
MSG = 'Redundant escape of %<char>s inside string literal.'

def on_str(node)
return if node.parent&.regexp_type? || node.parent&.xstr_type?
return if node.parent&.regexp_type? || node.parent&.xstr_type? || character_literal?(node)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If rubocop/rubocop-ast#242 becomes available, it can be rewritten as:

Suggested change
return if node.parent&.regexp_type? || node.parent&.xstr_type? || character_literal?(node)
return if node.parent&.regexp_type? || node.parent&.xstr_type? || node.character_literal?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be nice - we have several checks for char literals in the codebase that can be simplified. And one cop that discourages them. :-)

@bbatsov bbatsov merged commit 296205e into rubocop:master Oct 21, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 21, 2022

Thanks!

@koic koic deleted the fix_an_error_for_style_redundant_string_escape branch October 21, 2022 08:30
marcandre pushed a commit to rubocop/rubocop-ast that referenced this pull request Oct 21, 2022
This PR adds `character_literal?` to `StrNode`.

`...begin.is?('?')` used in rubocop/rubocop#11095
and `Style/CharacterLiteral` can be rewritten by `node.character_literal?`.
koic added a commit that referenced this pull request Oct 21, 2022
Follow up #11095 (comment)
and rubocop/rubocop-ast#242.

The API was introduced in rubocop-ast 1.23.0.
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.

None yet

2 participants