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

Handle more matching methods in Lint/OutOfRangeRegexpRef #8464

Merged

Conversation

@eugeneius
Copy link
Contributor

@eugeneius eugeneius commented Aug 5, 2020

Followup to @marcandre's comment here: #8463 (comment)

when, grep, gsub, gsub! sub, sub!, [], slice, slice! scan, index, rindex, partition, rpartition, start_with?, and end_with? can all be used to match regexps and set last match global variables, so we should update the range of valid references when we encounter them.

These were all of the relevant methods I could find from a quick grep of ruby/spec, although there could well be more.

Update: I read through the String class' docs and added several more methods.


Before submitting the PR make sure the following are checked:

  • 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.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.
@eugeneius eugeneius force-pushed the eugeneius:out_of_range_regexp_ref_more_methods branch 5 times, most recently from f1873d0 to aebb623 Aug 6, 2020
Copy link
Contributor

@marcandre marcandre left a comment

Good stuff 💪

`when`, `grep`, `gsub`, `gsub!` `sub`, `sub!`, `[]`, `slice`, `slice!`,
`scan`, `index`, `rindex`, `partition`, `rpartition`, `start_with?`, and
`end_with?` can all be used to match regexps, so we should update the
range of valid references when we encounter them.
@eugeneius eugeneius force-pushed the eugeneius:out_of_range_regexp_ref_more_methods branch from aebb623 to fa1fc0b Aug 6, 2020
@marcandre marcandre merged commit 7552fed into rubocop-hq:master Aug 7, 2020
26 checks passed
26 checks passed
windows 2.4
Details
windows 2.5
Details
windows 2.6
Details
windows 2.7
Details
windows mingw
Details
ci/circleci: cc-setup Your tests passed on CircleCI!
Details
ci/circleci: cc-upload-coverage Your tests passed on CircleCI!
Details
ci/circleci: documentation-checks Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-rubocop Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-spec Your tests passed on CircleCI!
Details
@marcandre
Copy link
Contributor

@marcandre marcandre commented Aug 7, 2020

Looks great, thanks for the PR! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.