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

Lint/UriRegexp misses common usecase of URI.regexp #7834

Closed
gravitystorm opened this issue Apr 1, 2020 · 0 comments
Closed

Lint/UriRegexp misses common usecase of URI.regexp #7834

gravitystorm opened this issue Apr 1, 2020 · 0 comments

Comments

@gravitystorm
Copy link

@gravitystorm gravitystorm commented Apr 1, 2020

In #4694 the Lint/UriRegexp cop was introduced, and tests were added for two cases:

  • Without arguments, e.g. URI.regexp
  • With a string argument, e.g. URI.regexp('http://example.com')

However, a common argument passed to URI.regexp is an array of schemes to use in the uri matching, e.g.

  • URI.regexp(%w[http https])

Such usage isn't flagged up by RuboCop, since it appears to me to only check for string arguments.

Also, the existing example for string arguments should be changed, since it is confusing. It only makes sense to pass a schema to URI::DEFAULT_PARSER.make_regexp, e.g. URI::DEFAULT_PARSER.make_regexp('http'), rather than a full url.


Expected behavior

Lint/UriRegexp should warn on the common usecase of URI.regexp(%w[http https])

Actual behavior

Lint/UriRegexp does not warn when using URI.regexp(%w[http https])

Steps to reproduce the problem

$ cat test.rb 
# frozen_string_literal: true

URI.regexp
URI.regexp('http')
URI.regexp(%w[http https])

$ bundle exec rubocop
Inspecting 2 files
.W

Offenses:

test.rb:3:5: W: Lint/UriRegexp: URI.regexp is obsolete and should not be used. Instead, use URI::DEFAULT_PARSER.make_regexp.
URI.regexp
    ^^^^^^
test.rb:4:5: W: Lint/UriRegexp: URI.regexp('http') is obsolete and should not be used. Instead, use URI::DEFAULT_PARSER.make_regexp('http').
URI.regexp('http')
    ^^^^^^

2 files inspected, 2 offenses detected


It should have detected 3 offenses

RuboCop version

$ bundle exec rubocop -v
0.81.0
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Apr 1, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Apr 1, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Apr 4, 2020
@koic koic closed this in #7837 Apr 4, 2020
saurabhmaurya15 pushed a commit to saurabhmaurya15/rubocop that referenced this issue Apr 7, 2020
Neodelf added a commit to Neodelf/rubocop that referenced this issue Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

1 participant
You can’t perform that action at this time.