From 177c92ff64b305bd0439235f443bcc5b6640afd7 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 22 Oct 2022 12:56:15 +0900 Subject: [PATCH] Cut 2.17.0 --- CHANGELOG.md | 2 + config/default.yml | 6 +- docs/antora.yml | 2 +- docs/modules/ROOT/pages/cops.adoc | 2 + docs/modules/ROOT/pages/cops_rails.adoc | 120 +++++++++++++++++++++--- lib/rubocop/rails/version.rb | 2 +- relnotes/v2.17.0.md | 40 ++++++++ 7 files changed, 155 insertions(+), 19 deletions(-) create mode 100644 relnotes/v2.17.0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 65493f84ae..6958155d00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## master (unreleased) +## 2.17.0 (2022-10-22) + ### New features * [#547](https://github.com/rubocop/rubocop-rails/pull/547): Add new `Rails/ActionOrder` cop. ([@mollerhoj][]) diff --git a/config/default.yml b/config/default.yml index 52b8736409..581f365900 100644 --- a/config/default.yml +++ b/config/default.yml @@ -101,7 +101,7 @@ Rails/ActionFilter: Rails/ActionOrder: Description: 'Enforce consistent ordering of controller actions.' Enabled: pending - VersionAdded: '<>' + VersionAdded: '2.17' ExpectedOrder: - index - show @@ -561,7 +561,7 @@ Rails/IgnoredColumnsAssignment: StyleGuide: 'https://rails.rubystyle.guide/#append-ignored-columns' Enabled: pending SafeAutoCorrect: false - VersionAdded: '<>' + VersionAdded: '2.17' Rails/IgnoredSkipActionFilterOption: Description: 'Checks that `if` and `only` (or `except`) are not used together as options of `skip_*` action filter.' @@ -1121,7 +1121,7 @@ Rails/WhereNot: Rails/WhereNotWithMultipleConditions: Description: 'Do not use `where.not(...)` with multiple conditions.' Enabled: 'pending' - VersionAdded: '<>' + VersionAdded: '2.17' # Accept `redirect_to(...) and return` and similar cases. Style/AndOr: diff --git a/docs/antora.yml b/docs/antora.yml index 9e0ff48acb..f804e59817 100644 --- a/docs/antora.yml +++ b/docs/antora.yml @@ -2,6 +2,6 @@ name: rubocop-rails title: RuboCop Rails # We always provide version without patch here (e.g. 1.1), # as patch versions should not appear in the docs. -version: ~ +version: '2.17' nav: - modules/ROOT/nav.adoc diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 97fe8f4310..c1a6a2a247 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -70,6 +70,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide]. * xref:cops_rails.adoc#railsi18nlazylookup[Rails/I18nLazyLookup] * xref:cops_rails.adoc#railsi18nlocaleassignment[Rails/I18nLocaleAssignment] * xref:cops_rails.adoc#railsi18nlocaletexts[Rails/I18nLocaleTexts] +* xref:cops_rails.adoc#railsignoredcolumnsassignment[Rails/IgnoredColumnsAssignment] * xref:cops_rails.adoc#railsignoredskipactionfilteroption[Rails/IgnoredSkipActionFilterOption] * xref:cops_rails.adoc#railsindexby[Rails/IndexBy] * xref:cops_rails.adoc#railsindexwith[Rails/IndexWith] @@ -136,5 +137,6 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide]. * xref:cops_rails.adoc#railswhereexists[Rails/WhereExists] * xref:cops_rails.adoc#railswheremissing[Rails/WhereMissing] * xref:cops_rails.adoc#railswherenot[Rails/WhereNot] +* xref:cops_rails.adoc#railswherenotwithmultipleconditions[Rails/WhereNotWithMultipleConditions] // END_COP_LIST diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index 842229f453..21ce7cb8ea 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -175,20 +175,19 @@ skip_after_filter :do_stuff == Rails/ActionOrder |=== -| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed -| Disabled +| Pending | Yes | Yes -| 2.13 +| 2.17 | - |=== -This cop enforces consistent ordering of the standard Rails RESTful -controller actions. +Enforces consistent ordering of the standard Rails RESTful controller actions. -The cop is configurable and can enforce any ordering of the standard -actions. All other methods are ignored. +The cop is configurable and can enforce any ordering of the standard actions. +All other methods are ignored. [source,yaml] ---- @@ -228,7 +227,7 @@ def destroy; end | Array | Include -| `app/controllers/**/*.rb` +| `+app/controllers/**/*.rb+` | Array |=== @@ -1672,26 +1671,30 @@ User.find_by(name: name, email: email) User.find_by!(email: email) ---- -==== AllowedMethods: find_by_sql +==== AllowedMethods: ['find_by_sql', 'find_by_token_for'] (default) [source,ruby] ---- # bad User.find_by_query(users_query) +User.find_by_token_for(:password_reset, token) # good User.find_by_sql(users_sql) +User.find_by_token_for(:password_reset, token) ---- -==== AllowedReceivers: Gem::Specification +==== AllowedReceivers: ['Gem::Specification', 'page'] (default) [source,ruby] ---- # bad Specification.find_by_name('backend').gem_dir +page.find_by_id('a_dom_id').click # good Gem::Specification.find_by_name('backend').gem_dir +page.find_by_id('a_dom_id').click ---- === Configurable attributes @@ -1700,15 +1703,15 @@ Gem::Specification.find_by_name('backend').gem_dir | Name | Default value | Configurable values | Whitelist -| `find_by_sql` +| `find_by_sql`, `find_by_token_for` | Array | AllowedMethods -| `find_by_sql` +| `find_by_sql`, `find_by_token_for` | Array | AllowedReceivers -| `Gem::Specification` +| `Gem::Specification`, `page` | Array |=== @@ -2227,7 +2230,7 @@ User.all.find_each User.order(:foo).each ---- -==== AllowedPattern: [/order/] +==== AllowedPattern: ['order'] [source,ruby] ---- @@ -2524,6 +2527,7 @@ render json: { foo: 'bar' }, status: 200 render plain: 'foo/bar', status: 304 redirect_to root_url, status: 301 head 200 +get '/foobar', to: redirect('/foobar/baz', status: 301) # good render :foo, status: :ok @@ -2531,6 +2535,7 @@ render json: { foo: 'bar' }, status: :ok render plain: 'foo/bar', status: :not_modified redirect_to root_url, status: :moved_permanently head :ok +get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently) ---- ==== EnforcedStyle: numeric @@ -2543,6 +2548,7 @@ render json: { foo: 'bar' }, status: :not_found render plain: 'foo/bar', status: :not_modified redirect_to root_url, status: :moved_permanently head :ok +get '/foobar', to: redirect('/foobar/baz', status: :moved_permanently) # good render :foo, status: 200 @@ -2550,6 +2556,7 @@ render json: { foo: 'bar' }, status: 404 render plain: 'foo/bar', status: 304 redirect_to root_url, status: 301 head 200 +get '/foobar', to: redirect('/foobar/baz', status: 301) ---- === Configurable attributes @@ -2742,6 +2749,55 @@ end * https://rails.rubystyle.guide/#locale-texts +== Rails/IgnoredColumnsAssignment + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Yes (Unsafe) +| 2.17 +| - +|=== + +Looks for assignments of `ignored_columns` that may override previous +assignments. + +Overwriting previous assignments is usually a mistake, since it will +un-ignore the first set of columns. Since duplicate column names is not +a problem, it is better to simply append to the list. + +=== Examples + +[source,ruby] +---- +# bad +class User < ActiveRecord::Base + self.ignored_columns = [:one] +end + +# bad +class User < ActiveRecord::Base + self.ignored_columns = [:one, :two] +end + +# good +class User < ActiveRecord::Base + self.ignored_columns += [:one, :two] +end + +# good +class User < ActiveRecord::Base + self.ignored_columns += [:one] + self.ignored_columns += [:two] +end +---- + +=== References + +* https://rails.rubystyle.guide/#append-ignored-columns + == Rails/IgnoredSkipActionFilterOption |=== @@ -6444,3 +6500,39 @@ User.where.not(users: { name: 'Gabe' }) === References * https://rails.rubystyle.guide/#hash-conditions + +== Rails/WhereNotWithMultipleConditions + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| No +| 2.17 +| - +|=== + +Identifies calls to `where.not` with multiple hash arguments. + +The behavior of `where.not` changed in Rails 6.1. Prior to the change, +`.where.not(trashed: true, role: 'admin')` evaluated to +`WHERE trashed != TRUE AND role != 'admin'`. +From Rails 6.1 onwards, this executes the query +`WHERE NOT (trashed == TRUE AND roles == 'admin')`. + +=== Examples + +[source,ruby] +---- +# bad +User.where.not(trashed: true, role: 'admin') +User.where.not(trashed: true, role: ['moderator', 'admin']) +User.joins(:posts).where.not(posts: { trashed: true, title: 'Rails' }) + +# good +User.where.not(trashed: true) +User.where.not(role: ['moderator', 'admin']) +User.where.not(trashed: true).where.not(role: ['moderator', 'admin']) +User.where.not('trashed = ? OR role = ?', true, 'admin') +---- diff --git a/lib/rubocop/rails/version.rb b/lib/rubocop/rails/version.rb index 910c915687..9434e22808 100644 --- a/lib/rubocop/rails/version.rb +++ b/lib/rubocop/rails/version.rb @@ -4,7 +4,7 @@ module RuboCop module Rails # This module holds the RuboCop Rails version information. module Version - STRING = '2.16.1' + STRING = '2.17.0' def self.document_version STRING.match('\d+\.\d+').to_s diff --git a/relnotes/v2.17.0.md b/relnotes/v2.17.0.md new file mode 100644 index 0000000000..b6021a8f66 --- /dev/null +++ b/relnotes/v2.17.0.md @@ -0,0 +1,40 @@ +### New features + +* [#547](https://github.com/rubocop/rubocop-rails/pull/547): Add new `Rails/ActionOrder` cop. ([@mollerhoj][]) +* [#565](https://github.com/rubocop/rubocop-rails/issues/565): Add cop Rails/WhereNotWithMultipleConditions. ([@niklas-hasselmeyer][]) +* [#771](https://github.com/rubocop/rubocop-rails/pull/771): Add new `Rails/IgnoredColumnsAssignment` cop. ([@fsateler][], [@kkitadate][]) +* [#790](https://github.com/rubocop/rubocop-rails/issues/790): Make `Style/HashExcept` aware of TargetRubyVersion: 2.x because Rails has `Hash#except`. ([@koic][]) + +### Bug fixes + +* [#786](https://github.com/rubocop/rubocop-rails/issues/786): Fix a false negative for `Rails/ActionControllerTestCase` when extending `ActionController::TestCase` and having a method definition. ([@koic][]) +* [#792](https://github.com/rubocop/rubocop-rails/issues/792): Fix a false negative for `Rails/RedundantPresenceValidationOnBelongsTo` when belongs_to at least one block and one hash like `belongs_to :company, -> { where(foo: true) }, inverse_of: :employee`. ([@PedroAugustoRamalhoDuarte][]) +* [#781](https://github.com/rubocop/rubocop-rails/issues/781): Make `Rails/DynamicFindBy` aware of `find_by_token_for`. ([@koic][]) +* [#809](https://github.com/rubocop/rubocop-rails/issues/809): Fix an error for `Rails/FreezeTime` when using `travel_to` without argument. ([@koic][]) +* [#794](https://github.com/rubocop/rubocop-rails/issues/794): Fix an error for `Rails/RedundantReceiverInWithOptions` when calling a method with a receiver in `with_options` without block arguments. ([@koic][]) +* [#782](https://github.com/rubocop/rubocop-rails/issues/782): Fix an incorrect autocorrect for `Rails/EagerEvaluationLogMessage` when using `Style/MethodCallWithArgsParentheses`'s autocorrection together. ([@koic][]) +* [#776](https://github.com/rubocop/rubocop-rails/issues/776): Fix an incorrect autocorrect for `Rails/Presence` when using arithmetic operation in `else` branch. ([@koic][]) +* [#813](https://github.com/rubocop/rubocop-rails/pull/813): Fix errors that occur when unrelated `tag` is investigated by `Rails/ContentTag`. ([@r7kamura][]) +* [#808](https://github.com/rubocop/rubocop-rails/issues/808): Fix false positive for `Rails/ActionControllerFlashBeforeRender` when `render` call precedes `flash` call. ([@americodls][]) +* [#778](https://github.com/rubocop/rubocop-rails/issues/778): Fix a false positive for `Rails/DynamicFindBy` when using `page.find_by_id` as a Capybara testing API. ([@koic][]) +* [#816](https://github.com/rubocop/rubocop-rails/pull/816): Fix an incorrect autocorrect for `Rails/Presence` when a right-hand side of the relational operator. ([@ydah][]) + +### Changes + +* [#779](https://github.com/rubocop/rubocop-rails/issues/779): Add `mail` to `AllowedMethods` of `Style/SymbolProc`. ([@koic][]) +* [#796](https://github.com/rubocop/rubocop-rails/issues/796): Add several directories to `Exclude` to prevent slow investigation. ([@koic][]) +* [#822](https://github.com/rubocop/rubocop-rails/issues/822): Extends `Rails/HttpStatus` cop to check `routes.rb`. ([@anthony-robin][]) +* [#787](https://github.com/rubocop/rubocop-rails/issues/787): Make `Rails/Pluck` aware of all keys. ([@koic][]) +* [#800](https://github.com/rubocop/rubocop-rails/issues/800): Make `Rails/TimeZone` aware of timezone UTF offset. ([@inkstak][]) + +[@mollerhoj]: https://github.com/mollerhoj +[@niklas-hasselmeyer]: https://github.com/niklas-hasselmeyer +[@fsateler]: https://github.com/fsateler +[@kkitadate]: https://github.com/kkitadate +[@koic]: https://github.com/koic +[@PedroAugustoRamalhoDuarte]: https://github.com/PedroAugustoRamalhoDuarte +[@r7kamura]: https://github.com/r7kamura +[@americodls]: https://github.com/americodls +[@ydah]: https://github.com/ydah +[@anthony-robin]: https://github.com/anthony-robin +[@inkstak]: https://github.com/inkstak