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

build(deps): bump rubocop-rails from 2.17.4 to 2.18.0 in /Library/Homebrew #14842

Merged

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Feb 28, 2023

Bumps rubocop-rails from 2.17.4 to 2.18.0.

Changelog

Sourced from rubocop-rails's changelog.

2.18.0 (2023-02-25)

New features

Bug fixes

  • #824: Fix a false negative for Rails/ActionControllerTestCase when the class is namespaced. ([@​vlad-pisanov][])
  • #909: Fix a false positive for Rails/ActionControllerFlashBeforeRender when using flash before redirect_to in if branch. ([@​koic][])
  • #898: Fix a false positive for Rails/ActiveRecordAliases when arguments of update_attributes is empty. ([@​koic][])
  • #920: Fix an error for Rails/NegateInclude when there is no receiver. ([@​fatkodima][])
  • #931: Fix error in Rails/Presence when ternary operators are used in multiple lines. ([@​r7kamura][])
  • #687: Fix Rails/HasManyOrHasOneDependent to correctly handle association methods with receiver. ([@​fatkodima][])
  • #929: Prevent Rails/SquishedSQLHeredocs applying when single-line comments are present. ([@​john-h-k][])
  • #887: Fix a false positive for Rails/NotNullColumn when adding a :virtual column. ([@​fatkodima][])
  • #918: Fix Rails/FreezeTime running against Rails < 5.2 apps. ([@​DRBragg][])
  • #895: Fix Rails/UnusedIgnoredColumns not recognizing columns added via +=. ([@​lucthev][])

Changes

  • #263: Accept actions defined via alias in Rails/LexicallyScopedActionFilter. ([@​fatkodima][])
  • #902: Ignore redirect method for Style/FormatStringToken by default. ([@​javierjulio][])
  • #935: Make Style/InverseMethods aware of Active Support's present?, blank?, include?, and exclude? methods. ([@​koic][])
  • #914: Make Style/InverseMethods aware of valid? and invalid? methods. ([@​koic][])
  • #826: Mark Rails/Pluck as unsafe. ([@​fatkodima][])
  • #896: Raise severity of Rails/ActiveRecordOverride, Rails/DeprecatedActiveModelErrorsMethods, Rails/DuplicateAssociation, Rails/DuplicateScope, Rails/TopLevelHashWithIndifferentAccess, and Rails/WhereNotWithMultipleConditions cops to warning. ([@​koic][])
Commits
  • fa6662f Cut 2.18.0
  • 1a791ec Update Changelog
  • c532db1 Fix a false negative for Rails/ActionControllerTestCase
  • e179054 Merge pull request #936 from koic/make_style_inverse_methods_aware_of_valid_a...
  • 9ff0c4d Merge pull request #937 from koic/fix_a_false_positive_for_rails_action_contr...
  • e9bb19f Suppress InternalAffairs/ProcessedSourceBufferName's offense
  • ad0fec1 [Fix #909] Fix a false positive for Rails/ActionControllerFlashBeforeRender
  • ecd9fbe [Fix #914] Make Style/InverseMethods aware of valid? and invalid? methods
  • 81b0fdc Merge pull request #935 from koic/make_style_inverse_methods_aware_of_active_...
  • b74740a Make Style/InverseMethods aware of some Active Support methods
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added dependencies Bumping Gemfile dependencies ruby Pull requests that update Ruby code labels Feb 28, 2023
@nandahkrishna nandahkrishna force-pushed the dependabot/bundler/Library/Homebrew/rubocop-rails-2.18.0 branch from 14d62b7 to 66e3adf Compare February 28, 2023 20:37
@MikeMcQuaid
Copy link
Member

Something weird going on here, can't reproduce this issues locally. @Bo98 @dduugg @issyl0 any ideas?

@dduugg dduugg force-pushed the dependabot/bundler/Library/Homebrew/rubocop-rails-2.18.0 branch from 928d916 to ab82489 Compare March 1, 2023 15:42
@@ -1,11 +1,13 @@
# typed: false
# frozen_string_literal: true

# Disable Rails cops, as we haven't required active_support yet.
# rubocop:disable Rails
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tangential, but I've disabled Rails cops in this file based on the comments on the previous disables below

@@ -53,7 +53,7 @@ def audit_desc(type, name, desc_call)
desc_problem "Description shouldn't start with an article." if regex_match_group(desc, /^(the|an?)(?=\s)/i)

# Check if invalid lowercase words are at the start of a desc.
if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) &&
if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) && # rubocop:disable Style/InverseMethods (false positive)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This triggered errors when i applied the autocorrect, so I've disabled the cop instead

@@ -112,7 +112,7 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node)
end

def check_on_system_block_content(component_precedence_list, on_system_block)
if on_system_block.body.block_type? && !on_system_methods.include?(on_system_block.body.method_name)
if on_system_block.body.block_type? && on_system_methods.exclude?(on_system_block.body.method_name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is also a false positive, as in the other violation in this directory below. We might want to disable it anyway, as we have comments elsewhere to avoid using ActiveSupport in rubocops (though we make extensive use of present?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dduugg
Copy link
Member

dduugg commented Mar 1, 2023

@MikeMcQuaid PTAL, I was able to reproduce and fix the violations locally. (A related question: why isn't --display-cop-names enabled by default in brew style?)

@MikeMcQuaid
Copy link
Member

why isn't --display-cop-names enabled by default in brew style

because it's pretty "rubocop internals" heavy and most folks using this tools don't care about rubocop

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work fixing this, thanks @dduugg!

@MikeMcQuaid MikeMcQuaid enabled auto-merge March 1, 2023 15:59
@dduugg
Copy link
Member

dduugg commented Mar 1, 2023

why isn't --display-cop-names enabled by default in brew style

because it's pretty "rubocop internals" heavy and most folks using this tools don't care about rubocop

Ah ok, FWIW, I work with rubocop often, and I still need to google the cop name regularly to figure out what to do with a violation.

@dduugg dduugg force-pushed the dependabot/bundler/Library/Homebrew/rubocop-rails-2.18.0 branch from ab82489 to 733813e Compare March 1, 2023 17:16
@@ -53,7 +53,7 @@ def audit_desc(type, name, desc_call)
desc_problem "Description shouldn't start with an article." if regex_match_group(desc, /^(the|an?)(?=\s)/i)

# Check if invalid lowercase words are at the start of a desc.
if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) &&
if !VALID_LOWERCASE_WORDS.include?(string_content(desc).split.first) && # rubocop:disable Style/InverseMethods (false positive)
Copy link
Member

@Bo98 Bo98 Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a false positive? Is it not just because we don't have that part of ActiveSupport required for cops and we've not excluded these files from this check like we already do with Rails?

Edit: Oh wait I see - they've taken a native RuboCop check and extended it... To be honest I don't understand why they've done that but also have Rails/NegateInclude - is there any difference? It is possible to disable the ActiveSupport parts of Style/InverseMethods

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a false positive? Is it not just because we don't have that part of ActiveSupport required for cops and we've not excluded these files from this check like we already do with Rails?

Yes to both, I consider the latter to be a false positive (meaning "this code is correct, rubocop should be ignored" not "rubocop has a point, but we've chosen to do something else"), but i'm open to a different wording. What do you suggest?

Edit: Oh wait I see - they've taken a native RuboCop check and extended it... To be honest I don't understand why they've done that but also have Rails/NegateInclude - is there any difference? It is possible to disable the ActiveSupport parts of Style/InverseMethods

I don't think there's a difference. I've filed rubocop/rubocop-rails#941 in the meantime. I describe a couple solutions there, I think I'd prefer the inline disable here in the meantime. Let me know what you'd like to do here.

@dduugg dduugg force-pushed the dependabot/bundler/Library/Homebrew/rubocop-rails-2.18.0 branch from 733813e to ef2baee Compare March 2, 2023 19:49
@MikeMcQuaid MikeMcQuaid merged commit fe89c0c into master Mar 2, 2023
@MikeMcQuaid MikeMcQuaid deleted the dependabot/bundler/Library/Homebrew/rubocop-rails-2.18.0 branch March 2, 2023 20:16
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Bumping Gemfile dependencies outdated PR was locked due to age ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants