Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mark Performance/AncestorsInclude as unsafe #149

Merged
merged 1 commit into from
Jul 11, 2020

Conversation

eugeneius
Copy link
Contributor

We can't tell whether the receiver is a class or an object.

In my particular case, the false positive was for Nokogiri::XML::Node#ancestors:
https://github.com/sparklemotion/nokogiri/blob/v1.10.10/lib/nokogiri/xml/node.rb#L601


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.

We can't tell whether the receiver is a class or an object.
@koic koic mentioned this pull request Jul 11, 2020
20 tasks
@koic koic merged commit e7de2ed into rubocop:master Jul 11, 2020
@koic
Copy link
Member

koic commented Jul 11, 2020

Thanks!

koic added a commit to koic/rubocop-performance that referenced this pull request Aug 26, 2020
…ude`

Fixes rubocop#159.

This PR fixes a false positive for `Performance/AncestorsInclude`
when receiver is a variable.

As the cop's example shows, this change mainly targets when the receiver
is a constant. It can cause false negatives, but it may be less than
false positives.
And this change may lead to removing unsafe marked by rubocop#149.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants