-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add new Lint/UriRegexp
cop
#4694
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
bce9d69
Add new `Lint/UriRegexp` cop
koic c8eb0a9
Match when there is a top-level double colon
koic 9355f20
Use `URI::DEFAULT_PARER` faster than `URI::Parser.new`
koic 099fa61
Match only the expected pattern
koic c03bbc4
Remove a message arguments redundancy
koic 949410f
Fix a typo
koic c5bf278
Use single quote instead of double quote
koic 00835fb
Add to the offense message that `URI.regexp` is obsolete
koic File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Lint | ||
# This cop identifies places where `URI.regexp` is obsolete and should | ||
# not be used. Instead, use `URI::DEFAULT_PARSER.make_regexp`. | ||
# | ||
# @example | ||
# # bad | ||
# URI.regexp('http://example.com') | ||
# | ||
# # good | ||
# URI::DEFAULT_PARSER.make_regexp('http://example.com') | ||
# | ||
class UriRegexp < Cop | ||
MSG = '`%<top_level>sURI.regexp%<arg>s` is obsolete and should not ' \ | ||
'be used. Instead, use `%<top_level>sURI::DEFAULT_PARSER.' \ | ||
'make_regexp%<arg>s`.'.freeze | ||
|
||
def_node_matcher :uri_regexp_with_argument?, <<-PATTERN | ||
(send | ||
(const ${nil cbase} :URI) :regexp | ||
(str $_)) | ||
PATTERN | ||
|
||
def_node_matcher :uri_regexp_without_argument?, <<-PATTERN | ||
(send | ||
(const ${nil cbase} :URI) :regexp) | ||
PATTERN | ||
|
||
def on_send(node) | ||
uri_regexp_with_argument?(node) do |double_colon, arg| | ||
register_offense( | ||
node, top_level: double_colon ? '::' : '', arg: "('#{arg}')" | ||
) | ||
end | ||
|
||
uri_regexp_without_argument?(node) do |double_colon| | ||
register_offense(node, top_level: double_colon ? '::' : '') | ||
end | ||
end | ||
|
||
def autocorrect(node) | ||
lambda do |corrector| | ||
if (captured_values = uri_regexp_with_argument?(node)) | ||
else | ||
captured_values = uri_regexp_without_argument?(node) | ||
end | ||
|
||
double_colon, arg = captured_values | ||
|
||
top_level = double_colon ? '::' : '' | ||
argument = arg ? "('#{arg}')" : '' | ||
|
||
corrector.replace( | ||
node.loc.expression, | ||
"#{top_level}URI::DEFAULT_PARSER.make_regexp#{argument}" | ||
) | ||
end | ||
end | ||
|
||
private | ||
|
||
def register_offense(node, top_level: '', arg: '') | ||
format = format(MSG, top_level: top_level, arg: arg) | ||
|
||
add_offense(node, :selector, format) | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
# frozen_string_literal: true | ||
|
||
describe RuboCop::Cop::Lint::UriRegexp do | ||
let(:config) { RuboCop::Config.new } | ||
subject(:cop) { described_class.new(config) } | ||
|
||
it 'registers an offense when using `URI.regexp` with argument' do | ||
expect_offense(<<-RUBY.strip_indent) | ||
URI.regexp('http://example.com') | ||
^^^^^^ `URI.regexp('http://example.com')` is obsolete and should not be used. Instead, use `URI::DEFAULT_PARSER.make_regexp('http://example.com')`. | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `::URI.regexp` with argument' do | ||
expect_offense(<<-RUBY.strip_indent) | ||
::URI.regexp('http://example.com') | ||
^^^^^^ `::URI.regexp('http://example.com')` is obsolete and should not be used. Instead, use `::URI::DEFAULT_PARSER.make_regexp('http://example.com')`. | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `URI.regexp` without argument' do | ||
expect_offense(<<-RUBY.strip_indent) | ||
URI.regexp | ||
^^^^^^ `URI.regexp` is obsolete and should not be used. Instead, use `URI::DEFAULT_PARSER.make_regexp`. | ||
RUBY | ||
end | ||
|
||
it 'registers an offense when using `::URI.regexp` without argument' do | ||
expect_offense(<<-RUBY.strip_indent) | ||
::URI.regexp | ||
^^^^^^ `::URI.regexp` is obsolete and should not be used. Instead, use `::URI::DEFAULT_PARSER.make_regexp`. | ||
RUBY | ||
end | ||
|
||
it "autocorrects URI::DEFAULT_PARSER.make_regexp('http://example.com')" do | ||
new_source = autocorrect_source("URI.regexp('http://example.com')") | ||
|
||
expect( | ||
new_source | ||
).to eq "URI::DEFAULT_PARSER.make_regexp('http://example.com')" | ||
end | ||
|
||
it "autocorrects ::URI::DEFAULT_PARSER.make_regexp('http://example.com')" do | ||
new_source = autocorrect_source("::URI.regexp('http://example.com')") | ||
|
||
expect( | ||
new_source | ||
).to eq "::URI::DEFAULT_PARSER.make_regexp('http://example.com')" | ||
end | ||
|
||
it 'autocorrects URI::DEFAULT_PARSER.make_regexp' do | ||
new_source = autocorrect_source('URI.regexp') | ||
|
||
expect(new_source).to eq 'URI::DEFAULT_PARSER.make_regexp' | ||
end | ||
|
||
it 'autocorrects ::URI::DEFAULT_PARSER.make_regexp' do | ||
new_source = autocorrect_source('::URI.regexp') | ||
|
||
expect(new_source).to eq '::URI::DEFAULT_PARSER.make_regexp' | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about having the offense highlight more of the expression? It seems like this should highlight
URI.regexp
orURI.regexp('http://example.com')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know if that is correct, but since
URI
module does not change, this cop focused on onlyregexp
.