Add StrNode#single_quoted?, StrNode#double_quoted? and StrNode#percent_literal? to simplify checking for string delimiters
#343
Conversation
dvandersluis
changed the title
StrNode#single_quoted?, StrNode#double_quoted? and StrNode#percent_literal?` to simplify checking for string delimitersStrNode#single_quoted?, StrNode#double_quoted? and StrNode#percent_literal? to simplify checking for string delimiters
dvandersluis
force-pushed
the
str-node-percent-literal
branch
2 times, most recently
from
1db7de5 to
c61b2e6
Compare
lib/rubocop/ast/node/str_node.rb
Outdated
| :% => /^%(?![qQ])/, | ||
| :q => /^%q/, | ||
| :Q => /^%Q/ |
There was a problem hiding this comment.
It seems that matching the beginning of lines in a multiline string is not expected.
| :% => /^%(?![qQ])/, | |
| :q => /^%q/, | |
| :Q => /^%Q/ | |
| :% => /\A%(?![qQ])/, | |
| :q => /\A%q/, | |
| :Q => /\A%Q/ |
lib/rubocop/ast/node/str_node.rb
Outdated
| return false unless opening_delimiter | ||
|
|
||
| if type | ||
| opening_delimiter.source =~ PERCENT_LITERAL_TYPES[type] |
There was a problem hiding this comment.
Since the captured value is not used, match? seems to be the best choice.
| opening_delimiter.source =~ PERCENT_LITERAL_TYPES[type] | |
| opening_delimiter.source.match?(PERCENT_LITERAL_TYPES[type]) |
lib/rubocop/ast/node/str_node.rb
Outdated
| @@ -8,13 +8,56 @@ module AST | |||
| class StrNode < Node | |||
| include BasicLiteralNode | |||
|
|
|||
| PERCENT_LITERAL_TYPES = { | |||
| :% => /^%(?![qQ])/, | |||
There was a problem hiding this comment.
%%foo%, probably to the horror of some, is a valid string. I'm not sure if any cop currently cares, just wanted to point it out. I'm not 100% on what would be a valid delimiter for these, maybe [^a-zA-Z] would be good enough?
There was a problem hiding this comment.
That'd still be a %(...) string though, wouldn't it? The extra %s replace brackets but the macro type is still a single %.
There was a problem hiding this comment.
Yes, I think so. It's in a similar vein as %w!a! being a valid array (hopefully no one actually writes such code).
dvandersluis
force-pushed
the
str-node-percent-literal
branch
from
c61b2e6 to
d015111
Compare
|
Thanks @koic and @Earlopain, the requested changes have been made. |
lib/rubocop/ast/node/str_node.rb
Outdated
| return false unless opening_delimiter | ||
|
|
||
| if type | ||
| opening_delimiter.source.match?(PERCENT_LITERAL_TYPES[type]) |
There was a problem hiding this comment.
Let's use fetch for a better error message in case type doesn't have an expected value
lib/rubocop/ast/node/str_node.rb
Outdated
| private | ||
|
|
||
| def opening_delimiter | ||
| return unless loc.respond_to?(:begin) |
There was a problem hiding this comment.
Is it possible that a str node's location does not respond to begin?
There was a problem hiding this comment.
(We could use loc_is? that I propose in #345)
There was a problem hiding this comment.
Is it possible that a
strnode's location does not respond to begin?
Yes, there are str nodes without delimiters inside other nodes (such as dstr or regexp, not an exhaustive list) that have no delimiters and thus no loc.begin. I added a test to cover this.
There was a problem hiding this comment.
(We could use
loc_is?that I propose in #345)
I like it!
There was a problem hiding this comment.
Updated to use loc_is? now.
…ercent_literal?` to simplify checking for string delimiters
dvandersluis
force-pushed
the
str-node-percent-literal
branch
from
d015111 to
1df9114
Compare
This would allow rubocop code that checks for different string delimiter types to do so in a simpler way.
I took inspiration from
ArrayNode#percent_literal?for the interface forStrNode#percent_literal?.