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

More powerful SlicingWithRange #7967

Merged
merged 1 commit into from May 14, 2020

Conversation

zverok
Copy link
Contributor

@zverok zverok commented May 13, 2020

Supports any expression as start index (only int previously), as noticed here: #7921 (comment).
Still correctly ignores startless range.


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.

@zverok zverok force-pushed the slice_with_range_with_variables branch from 5fd1db6 to d5de7bd Compare May 13, 2020 07:40
CHANGELOG.md Show resolved Hide resolved
@zverok zverok force-pushed the slice_with_range_with_variables branch from d5de7bd to d830465 Compare May 13, 2020 07:46
@marcandre
Copy link
Contributor

I think either (...)or !nil? will do, right? Which do you find clearer?

@koic
Copy link
Member

koic commented May 14, 2020

That suggestion fine to me. @zverok Can you update this one?

diff --git a/lib/rubocop/cop/style/slicing_with_range.rb b/lib/rubocop/cop/style/slicing_with_range.rb
index 1611b61a4..de3769d9b 100644
--- a/lib/rubocop/cop/style/slicing_with_range.rb
+++ b/lib/rubocop/cop/style/slicing_with_range.rb
@@ -19,7 +19,7 @@ module RuboCop

         MSG = 'Prefer ary[n..] over ary[n..-1].'

-        def_node_matcher :range_till_minus_one?, '(irange (...) (int -1))'
+        def_node_matcher :range_till_minus_one?, '(irange !nil? (int -1))'

         def on_send(node)
           return unless node.method?(:[]) && node.arguments.count == 1

Supports any expression as start index (only int previously).
Still correctly ignores startless range.
@zverok zverok force-pushed the slice_with_range_with_variables branch from d830465 to 1e0b730 Compare May 14, 2020 08:48
@zverok
Copy link
Contributor Author

zverok commented May 14, 2020

@koic np, I've updated the PR. I liked (...) slightly better, but that's not something to spend time fighting about :)

(BTW, I believe that GitHub now allows maintainers to introduce such small changes themselves right on the web. Not that I am too lazy to do it myself, just interesting what the ethics around this would be, in the future.)

@bbatsov bbatsov merged commit 9272fda into rubocop:master May 14, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented May 14, 2020

Thanks!

@zverok zverok deleted the slice_with_range_with_variables branch May 14, 2020 09:33
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

4 participants