diff --git a/.circleci/config.yml b/.circleci/config.yml deleted file mode 100644 index d0339eec53..0000000000 --- a/.circleci/config.yml +++ /dev/null @@ -1,52 +0,0 @@ -version: 2.1 - -jobs: - rake_default: - parameters: - image: - description: "Name of the Docker image." - type: string - default: "cimg/ruby:3.2" - docker: - - image: << parameters.image >> - environment: - # CircleCI container has two cores, but Ruby can see 32 cores. So we - # configure test-queue. - # See https://github.com/tmm1/test-queue#environment-variables - TEST_QUEUE_WORKERS: 2 - JRUBY_OPTS: "--dev -J-Xmx1000M" - steps: - - checkout - - run: bundle install - - run: bundle exec rake - - # Miscellaneous tasks - documentation-checks: - docker: - - image: cimg/ruby:3.2 - steps: - - checkout - - run: bundle install - - run: - name: Check documentation syntax - command: bundle exec rake documentation_syntax_check - -workflows: - build: - jobs: - - documentation-checks - - rake_default: - name: Ruby 2.7 - image: cimg/ruby:2.7 - - rake_default: - name: Ruby 3.0 - image: cimg/ruby:3.0 - - rake_default: - name: Ruby 3.1 - image: cimg/ruby:3.1 - - rake_default: - name: Ruby 3.2 - image: cimg/ruby:3.2 - - rake_default: - name: Ruby HEAD - image: rubocophq/circleci-ruby-snapshot:latest # Nightly snapshot build diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000000..6b6894882c --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,88 @@ +name: CI + +on: + push: + branches: + - master + pull_request: + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + main: + name: Ruby ${{ matrix.ruby }} + runs-on: ubuntu-latest + env: + # See https://github.com/tmm1/test-queue#environment-variables + TEST_QUEUE_WORKERS: 2 + strategy: + fail-fast: false + matrix: + os: [ubuntu] + ruby: ['2.7', '3.0', '3.1', '3.2', 'head'] + + steps: + - name: checkout + uses: actions/checkout@v4 + - name: set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - name: spec + run: bundle exec rake spec + - name: internal_investigation + run: bundle exec rake internal_investigation + + jruby: + name: JRuby 9.4 + runs-on: ubuntu-latest + steps: + - name: checkout + uses: actions/checkout@v4 + - name: set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: jruby-9.4 + bundler-cache: true + - name: spec + run: bundle exec rake spec + - name: internal_investigation + run: bundle exec rake internal_investigation + + documentation_checks: + runs-on: ubuntu-latest + name: Check documentation syntax + steps: + - uses: actions/checkout@v4 + - name: set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.2 + bundler-cache: true + - name: test + run: bundle exec rake documentation_syntax_check + + oldest_supported_rubocop: + runs-on: ubuntu-latest + name: Check the oldest supported RuboCop version + steps: + - uses: actions/checkout@v4 + - name: Use the oldest supported RuboCop + run: | + sed -e "/gem 'rubocop', github: 'rubocop\/rubocop'/d" \ + -e "/gem 'rubocop-performance',/d" \ + -e "/gem 'rubocop-rspec',/d" -i Gemfile + cat << EOF > Gemfile.local + gem 'rubocop', '1.33.0' # Specify the oldest supported RuboCop version + EOF + - name: set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 2.7 + bundler-cache: true + - name: spec + run: bundle exec rake spec diff --git a/CHANGELOG.md b/CHANGELOG.md index 290d916bb5..c4f4035f95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,66 @@ ## master (unreleased) +## 2.23.0 (2023-12-16) + +### New features + +* [#1183](https://github.com/rubocop/rubocop-rails/pull/1183): Support PostGIS adapter for PostgreSQL. ([@Dania02525][]) + +### Bug fixes + +* [#1206](https://github.com/rubocop/rubocop-rails/issues/1206): Fix an error for `Rails/WhereMissing` where join method is called without arguments. ([@fatkodima][]) +* [#1189](https://github.com/rubocop/rubocop-rails/issues/1189): Fix false negatives for `Rails/Pluck` when using safe navigation method calls. ([@koic][]) +* [#1204](https://github.com/rubocop/rubocop-rails/pull/1204): Make `Rails/ActiveSupportAliases`, `Rails/FindBy`, `Rails/FindById`, `Rails/Inquiry`, `Rails/Pick` `Rails/PluckId`, `Rails/PluckInWhere`, `Rails/WhereEquals`, `Rails/WhereExists`, and `Rails/WhereNot` cops aware of safe navigation operator. ([@koic][]) + +### Changes + +* [#1213](https://github.com/rubocop/rubocop-rails/issues/1213): Update `Rails/PluckInWhere` to check for `.ids` call. ([@fatkodima][]) +* [#1181](https://github.com/rubocop/rubocop-rails/pull/1181): Support `Nokogiri::HTML.parse` and `Nokogiri::HTML5.parse` on `Rails/ResponseParsedBody`. ([@r7kamura][]) +* [#1198](https://github.com/rubocop/rubocop-rails/issues/1198): Support `where.not` for `Rails/PluckInWhere`. ([@fatkodima][]) + +## 2.22.2 (2023-11-19) + +### Bug fixes + +* [#1172](https://github.com/rubocop/rubocop-rails/issues/1172): Fix an error for `Rails/UnknownEnv` when using Rails 7.1. ([@koic][]) +* [#1173](https://github.com/rubocop/rubocop-rails/issues/1173): Fix an error for `Rails/RedundantActiveRecordAllMethod` cop when used with RuboCop 1.51 or lower. ([@koic][]) + +### Changes + +* [#1171](https://github.com/rubocop/rubocop-rails/pull/1171): Change `Rails/RedundantActiveRecordAllMethod` to ignore `delete_all` and `destroy_all` when receiver is an association. ([@masato-bkn][]) +* [#1178](https://github.com/rubocop/rubocop-rails/pull/1178): Require RuboCop AST 1.30.0+. ([@koic][]) + +## 2.22.1 (2023-10-28) + +### Bug fixes + +* [#1145](https://github.com/rubocop/rubocop-rails/issues/1145): Fix a false positive for `Rails/DuplicateAssociation` when using duplicate `belongs_to` associations of same class without other arguments. ([@koic][]) + +## 2.22.0 (2023-10-27) + +### New features + +* [#906](https://github.com/rubocop/rubocop-rails/pull/906): Add `Rails/EnvLocal` cop. ([@sambostock][]) +* [#1128](https://github.com/rubocop/rubocop-rails/issues/1128): Make `Rails/DuplicateAssociation` aware of duplicate `class_name`. ([@koic][]) +* [#1157](https://github.com/rubocop/rubocop-rails/pull/1157): Support some Rails 7.1's new querying methods for `Rails/RedundantActiveRecordAllMethod`. ([@koic][]) +* [#1147](https://github.com/rubocop/rubocop-rails/issues/1147): Support the Trilogy adapter for MySQL. ([@koic][]) + +### Bug fixes + +* [#952](https://github.com/rubocop/rubocop-rails/issues/952): Fix a false positive for `Rails/NotNullColumn` when using `null: false` for MySQL's TEXT type. ([@koic][]) +* [#1041](https://github.com/rubocop/rubocop-rails/issues/1041): Fix a false positive for `Rails/Output` when output method is called with block argument. ([@koic][]) +* [#1143](https://github.com/rubocop/rubocop-rails/issues/1143): Fix an error for `Rails/RedundantActiveRecordAllMethod` when using RuboCop 1.51 or lower. ([@koic][]) +* [#1105](https://github.com/rubocop/rubocop-rails/issues/1105): Fix false positives for `Rails/RedundantPresenceValidationOnBelongsTo` when using `validates` with `:if` or `:unless` options. ([@koic][]) +* [#1158](https://github.com/rubocop/rubocop-rails/issues/1158): `Rails/HasManyOrHasOneDependent` does not add offence when has_many or has_one is called on an explicit receiver. ([@samrjenkins][]) +* [#1160](https://github.com/rubocop/rubocop-rails/issues/1160): Fix `Rails/SaveBang` to ignore parenthesis. ([@fatkodima][]) + +### Changes + +* [#1152](https://github.com/rubocop/rubocop-rails/pull/1152): Add more dangerous column names to `Rails/DangerousColumnNames`. ([@r7kamura][]) +* [#1039](https://github.com/rubocop/rubocop-rails/issues/1039): Deprecate `Rails/ActionFilter` cop; it will be disabled by default. ([@koic][]) +* [#893](https://github.com/rubocop/rubocop-rails/issues/893): Support `local` as an environment for `Rails/UnknownEnv` from Rails 7.1 onward. ([@ghiculescu][]) + ## 2.21.2 (2023-09-30) ### Bug fixes @@ -952,3 +1012,5 @@ [@nipe0324]: https://github.com/nipe0324 [@marocchino]: https://github.com/marocchino [@jamiemccarthy]: https://github.com/jamiemccarthy +[@sambostock]: https://github.com/sambostock +[@Dania02525]: https://github.com/Dania02525 diff --git a/Gemfile b/Gemfile index 8cf6699950..7e9a9c0b6e 100644 --- a/Gemfile +++ b/Gemfile @@ -11,7 +11,7 @@ gem 'rake' gem 'rspec' gem 'rubocop', github: 'rubocop/rubocop' gem 'rubocop-performance', '~> 1.18.0' -gem 'rubocop-rspec', '~> 2.22.0' +gem 'rubocop-rspec', '~> 2.25.0' gem 'simplecov' gem 'test-queue' gem 'yard', '~> 0.9' diff --git a/README.md b/README.md index 56d7291576..ca42e975a5 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,8 @@ Note: `--rails` option is required while `rubocop` command supports `--rails` op ### Rake task ```ruby +require 'rubocop/rake_task' + RuboCop::RakeTask.new do |task| task.requires << 'rubocop-rails' end @@ -80,7 +82,7 @@ module YourCoolApp end ``` -It uses `rubocop -A` to apply `Style/FrozenStringLiteralComment` and other unsafe autocorretion cops. +It uses `rubocop -A` to apply `Style/FrozenStringLiteralComment` and other unsafe autocorrection cops. `rubocop -A` is unsafe autocorrection, but code generated by default is simple and less likely to be incompatible with `rubocop -A`. If you have problems you can replace it with `rubocop -a` instead. diff --git a/config/default.yml b/config/default.yml index bd61a764b2..33f4a9f284 100644 --- a/config/default.yml +++ b/config/default.yml @@ -95,8 +95,9 @@ Rails/ActionControllerTestCase: Rails/ActionFilter: Description: 'Enforces consistent use of action filter methods.' - Enabled: true + Enabled: false VersionAdded: '0.19' + VersionChanged: '2.22' EnforcedStyle: action SupportedStyles: - action @@ -338,7 +339,6 @@ Rails/Date: Rails/DefaultScope: Description: 'Avoid use of `default_scope`.' - StyleGuide: 'https://rails.rubystyle.guide#avoid-default-scope' Enabled: false VersionAdded: '2.7' @@ -430,6 +430,11 @@ Rails/EnumUniqueness: Include: - app/models/**/*.rb +Rails/EnvLocal: + Description: 'Use `Rails.env.local?` instead of `Rails.env.development? || Rails.env.test?`.' + Enabled: pending + VersionAdded: '2.22' + Rails/EnvironmentComparison: Description: "Favor `Rails.env.production?` over `Rails.env == 'production'`." Enabled: true @@ -690,6 +695,9 @@ Rails/NotNullColumn: Enabled: true VersionAdded: '0.43' VersionChanged: '2.20' + Database: null + SupportedDatabases: + - mysql Include: - db/**/*.rb @@ -894,7 +902,7 @@ Rails/RequireDependency: VersionAdded: '2.10' Rails/ResponseParsedBody: - Description: Prefer `response.parsed_body` to `JSON.parse(response.body)`. + Description: Prefer `response.parsed_body` to custom parsing logic for `response.body`. Enabled: pending Safe: false VersionAdded: '2.18' diff --git a/docs/antora.yml b/docs/antora.yml index a5c58943de..ebab17b65b 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: '2.21' +version: '2.23' nav: - modules/ROOT/nav.adoc diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 569afede1e..56dfc04e5b 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -54,6 +54,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide]. * xref:cops_rails.adoc#railseagerevaluationlogmessage[Rails/EagerEvaluationLogMessage] * xref:cops_rails.adoc#railsenumhash[Rails/EnumHash] * xref:cops_rails.adoc#railsenumuniqueness[Rails/EnumUniqueness] +* xref:cops_rails.adoc#railsenvlocal[Rails/EnvLocal] * xref:cops_rails.adoc#railsenvironmentcomparison[Rails/EnvironmentComparison] * xref:cops_rails.adoc#railsenvironmentvariableaccess[Rails/EnvironmentVariableAccess] * xref:cops_rails.adoc#railsexit[Rails/Exit] diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index c328af841f..156dc48ba9 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -95,11 +95,11 @@ end |=== | Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed -| Enabled +| Disabled | Yes | Yes | 0.19 -| - +| 2.22 |=== Enforces the consistent use of action filter methods. @@ -107,6 +107,9 @@ Enforces the consistent use of action filter methods. The cop is configurable and can enforce the use of the older something_filter methods or the newer something_action methods. +IMPORTANT: This cop is deprecated. Because the `*_filter` methods were removed in Rails 4.2, +and that Rails version is no longer supported by RuboCop Rails. This cop will be removed in RuboCop Rails 3.0. + === Examples ==== EnforcedStyle: action (default) @@ -237,10 +240,10 @@ This cop is unsafe because custom `update_attributes` method call was changed to [source,ruby] ---- -#bad +# bad book.update_attributes!(author: 'Alice') -#good +# good book.update!(author: 'Alice') ---- @@ -974,7 +977,7 @@ the PostgreSQL (5.2 later) adapter; thus it will automatically detect an adapter from `development` environment in `config/database.yml` or the environment variable `DATABASE_URL` when the `Database` option is not set. -If the adapter is not `mysql2` or `postgresql`, +If the adapter is not `mysql2`, `trilogy`, `postgresql`, or `postgis`, this Cop ignores offenses. === Examples @@ -1395,10 +1398,6 @@ def self.published end ---- -=== References - -* https://rails.rubystyle.guide#avoid-default-scope - == Rails/Delegate |=== @@ -1637,6 +1636,15 @@ has_one :foo # good belongs_to :bar has_one :foo + +# bad +has_many :foo, class_name: 'Foo' +has_many :bar, class_name: 'Foo' +has_one :baz + +# good +has_many :bar, class_name: 'Foo' +has_one :foo ---- === Configurable attributes @@ -1833,10 +1841,10 @@ when no output would be produced anyway. [source,ruby] ---- -#bad +# bad Rails.logger.debug "The time is #{Time.zone.now}." -#good +# good Rails.logger.debug { "The time is #{Time.zone.now}." } ---- @@ -1929,6 +1937,32 @@ enum status: [:active, :archived] | Array |=== +== Rails/EnvLocal + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Yes +| 2.22 +| - +|=== + +Checks for usage of `Rails.env.development? || Rails.env.test?` which +can be replaced with `Rails.env.local?`, introduced in Rails 7.1. + +=== Examples + +[source,ruby] +---- +# bad +Rails.env.development? || Rails.env.test? + +# good +Rails.env.local? +---- + == Rails/EnvironmentComparison |=== @@ -2510,7 +2544,7 @@ end Checks for use of the helper methods which reference instance variables. -Relying on instance variables makes it difficult to re-use helper +Relying on instance variables makes it difficult to reuse helper methods. If it seems awkward to explicitly pass in each dependent @@ -3619,8 +3653,13 @@ hash.exclude?(:key) | 2.20 |=== -Checks for add_column call with NOT NULL constraint -in migration file. +Checks for add_column call with NOT NULL constraint in migration file. + +`TEXT` can have default values in PostgreSQL, but not in MySQL. +It will automatically detect an adapter from `development` environment +in `config/database.yml` or the environment variable `DATABASE_URL` +when the `Database` option is not set. If the database is MySQL, +this cop ignores offenses for the `TEXT`. === Examples @@ -3642,6 +3681,10 @@ add_reference :products, :category, null: false, default: 1 |=== | Name | Default value | Configurable values +| Database +| `` +| `mysql` + | Include | `+db/**/*.rb+` | Array @@ -3971,10 +4014,13 @@ as the cop cannot replace to `select` between calls to `pluck` on an ---- # bad Post.where(user_id: User.active.pluck(:id)) +Post.where(user_id: User.active.ids) +Post.where.not(user_id: User.active.pluck(:id)) # good Post.where(user_id: User.active.select(:id)) Post.where(user_id: active_users.select(:id)) +Post.where.not(user_id: active_users.select(:id)) ---- ==== EnforcedStyle: conservative (default) @@ -4303,6 +4349,12 @@ end Detect redundant `all` used as a receiver for Active Record query methods. +For the methods `delete_all` and `destroy_all`, this cop will only check cases where the receiver is a model. +It will ignore cases where the receiver is an association (e.g., `user.articles.all.delete_all`). +This is because omitting `all` from an association changes the methods +from `ActiveRecord::Relation` to `ActiveRecord::Associations::CollectionProxy`, +which can affect their behavior. + === Safety This cop is unsafe for autocorrection if the receiver for `all` is not an Active Record object. @@ -4934,13 +4986,14 @@ require_dependency 'some_lib' | 2.19 |=== -Prefer `response.parsed_body` to `JSON.parse(response.body)`. +Prefer `response.parsed_body` to custom parsing logic for `response.body`. === Safety -This cop is unsafe because Content-Type may not be `application/json`. For example, the proprietary -Content-Type provided by corporate entities such as `application/vnd.github+json` is not supported at -`response.parsed_body` by default, so you still have to use `JSON.parse(response.body)` there. +This cop is unsafe because Content-Type may not be `application/json` or `text/html`. +For example, the proprietary Content-Type provided by corporate entities such as +`application/vnd.github+json` is not supported at `response.parsed_body` by default, +so you still have to use `JSON.parse(response.body)` there. === Examples @@ -4949,6 +5002,12 @@ Content-Type provided by corporate entities such as `application/vnd.github+json # bad JSON.parse(response.body) +# bad +Nokogiri::HTML.parse(response.body) + +# bad +Nokogiri::HTML5.parse(response.body) + # good response.parsed_body ---- diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 4153623164..7d9f47b47b 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -34,7 +34,7 @@ end == Rails configuration tip If you are using Rails 6.1 or newer, add the following `config.generators.after_generate` setting to -your config/application.rb to apply RuboCop autocorretion to code generated by `bin/rails g`. +your config/application.rb to apply RuboCop autocorrection to code generated by `bin/rails g`. [source,ruby] ---- @@ -50,6 +50,6 @@ module YourCoolApp end ---- -It uses `rubocop -A` to apply `Style/FrozenStringLiteralComment` and other unsafe autocorretion cops. +It uses `rubocop -A` to apply `Style/FrozenStringLiteralComment` and other unsafe autocorrection cops. `rubocop -A` is unsafe autocorrection, but code generated by default is simple and less likely to be incompatible with `rubocop -A`. If you have problems you can replace it with `rubocop -a` instead. diff --git a/lib/rubocop/cop/mixin/active_record_helper.rb b/lib/rubocop/cop/mixin/active_record_helper.rb index ca5cfd6ab4..e5b5b5903c 100644 --- a/lib/rubocop/cop/mixin/active_record_helper.rb +++ b/lib/rubocop/cop/mixin/active_record_helper.rb @@ -98,8 +98,15 @@ def polymorphic?(belongs_to) end def in_where?(node) - send_node = node.each_ancestor(:send).first - send_node && WHERE_METHODS.include?(send_node.method_name) + send_node = node.each_ancestor(:send, :csend).first + return false unless send_node + + return true if WHERE_METHODS.include?(send_node.method_name) + + receiver = send_node.receiver + return false unless receiver&.send_type? + + send_node.method?(:not) && WHERE_METHODS.include?(receiver.method_name) end end end diff --git a/lib/rubocop/cop/mixin/database_type_resolvable.rb b/lib/rubocop/cop/mixin/database_type_resolvable.rb new file mode 100644 index 0000000000..3743514492 --- /dev/null +++ b/lib/rubocop/cop/mixin/database_type_resolvable.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # A mixin to extend cops in order to determine the database type. + # + # This module automatically detect an adapter from `development` environment + # in `config/database.yml` or the environment variable `DATABASE_URL` + # when the `Database` option is not set. + module DatabaseTypeResolvable + MYSQL = 'mysql' + POSTGRESQL = 'postgresql' + + def database + cop_config['Database'] || database_from_yaml || database_from_env + end + + private + + def database_from_yaml + return unless database_yaml + + case database_adapter + when 'mysql2', 'trilogy' + MYSQL + when 'postgresql', 'postgis' + POSTGRESQL + end + end + + def database_from_env + url = ENV['DATABASE_URL'].presence + return unless url + + case url + when %r{\A(mysql2|trilogy)://} + MYSQL + when %r{\Apostgres(ql)?://} + POSTGRESQL + end + end + + def database_yaml + return unless File.exist?('config/database.yml') + + yaml = if YAML.respond_to?(:unsafe_load_file) + YAML.unsafe_load_file('config/database.yml') + else + YAML.load_file('config/database.yml') + end + return unless yaml.is_a? Hash + + config = yaml['development'] + return unless config.is_a?(Hash) + + config + rescue Psych::SyntaxError + # noop + end + + def database_adapter + database_yaml['adapter'] || database_yaml.first.last['adapter'] + end + end + end +end diff --git a/lib/rubocop/cop/rails/action_filter.rb b/lib/rubocop/cop/rails/action_filter.rb index e144568e8a..e85830dadd 100644 --- a/lib/rubocop/cop/rails/action_filter.rb +++ b/lib/rubocop/cop/rails/action_filter.rb @@ -8,6 +8,9 @@ module Rails # The cop is configurable and can enforce the use of the older # something_filter methods or the newer something_action methods. # + # IMPORTANT: This cop is deprecated. Because the `*_filter` methods were removed in Rails 4.2, + # and that Rails version is no longer supported by RuboCop Rails. This cop will be removed in RuboCop Rails 3.0. + # # @example EnforcedStyle: action (default) # # bad # after_filter :do_stuff diff --git a/lib/rubocop/cop/rails/active_record_aliases.rb b/lib/rubocop/cop/rails/active_record_aliases.rb index 8d0e802fa3..7f7c854078 100644 --- a/lib/rubocop/cop/rails/active_record_aliases.rb +++ b/lib/rubocop/cop/rails/active_record_aliases.rb @@ -11,10 +11,10 @@ module Rails # `update` but the method name remained same in the method definition. # # @example - # #bad + # # bad # book.update_attributes!(author: 'Alice') # - # #good + # # good # book.update!(author: 'Alice') class ActiveRecordAliases < Base extend AutoCorrector diff --git a/lib/rubocop/cop/rails/active_support_aliases.rb b/lib/rubocop/cop/rails/active_support_aliases.rb index 312490da4b..50bf431ce4 100644 --- a/lib/rubocop/cop/rails/active_support_aliases.rb +++ b/lib/rubocop/cop/rails/active_support_aliases.rb @@ -27,13 +27,13 @@ class ActiveSupportAliases < Base ALIASES = { starts_with?: { - original: :start_with?, matcher: '(send str :starts_with? _)' + original: :start_with?, matcher: '(call str :starts_with? _)' }, ends_with?: { - original: :end_with?, matcher: '(send str :ends_with? _)' + original: :end_with?, matcher: '(call str :ends_with? _)' }, - append: { original: :<<, matcher: '(send array :append _)' }, - prepend: { original: :unshift, matcher: '(send array :prepend _)' } + append: { original: :<<, matcher: '(call array :append _)' }, + prepend: { original: :unshift, matcher: '(call array :prepend _)' } }.freeze ALIASES.each do |aliased_method, options| @@ -47,13 +47,14 @@ def on_send(node) preferred_method = ALIASES[aliased_method][:original] message = format(MSG, prefer: preferred_method, current: aliased_method) - add_offense(node, message: message) do |corrector| + add_offense(node.loc.selector.join(node.source_range.end), message: message) do |corrector| next if append(node) corrector.replace(node.loc.selector, preferred_method) end end end + alias on_csend on_send end end end diff --git a/lib/rubocop/cop/rails/after_commit_override.rb b/lib/rubocop/cop/rails/after_commit_override.rb index 070941de77..7fb1fa0fa2 100644 --- a/lib/rubocop/cop/rails/after_commit_override.rb +++ b/lib/rubocop/cop/rails/after_commit_override.rb @@ -48,7 +48,7 @@ def on_class(class_node) seen_callback_names = {} each_after_commit_callback(class_node) do |node| - callback_name = node.arguments[0].value + callback_name = node.first_argument.value if seen_callback_names.key?(callback_name) add_offense(node, message: format(MSG, name: callback_name)) else diff --git a/lib/rubocop/cop/rails/bulk_change_table.rb b/lib/rubocop/cop/rails/bulk_change_table.rb index bb2bfce3d3..2d5a503d6f 100644 --- a/lib/rubocop/cop/rails/bulk_change_table.rb +++ b/lib/rubocop/cop/rails/bulk_change_table.rb @@ -14,7 +14,7 @@ module Rails # automatically detect an adapter from `development` environment # in `config/database.yml` or the environment variable `DATABASE_URL` # when the `Database` option is not set. - # If the adapter is not `mysql2` or `postgresql`, + # If the adapter is not `mysql2`, `trilogy`, `postgresql`, or `postgis`, # this Cop ignores offenses. # # @example @@ -64,6 +64,8 @@ module Rails # end # end class BulkChangeTable < Base + include DatabaseTypeResolvable + MSG_FOR_CHANGE_TABLE = <<~MSG.chomp You can combine alter queries using `bulk: true` options. MSG @@ -71,9 +73,6 @@ class BulkChangeTable < Base You can use `change_table :%s, bulk: true` to combine alter queries. MSG - MYSQL = 'mysql' - POSTGRESQL = 'postgresql' - MIGRATION_METHODS = %i[change up down].freeze COMBINABLE_TRANSFORMATIONS = %i[ @@ -175,55 +174,6 @@ def include_bulk_options?(node) options.hash_type? && options.keys.any? { |key| key.sym_type? && key.value == :bulk } end - def database - cop_config['Database'] || database_from_yaml || database_from_env - end - - def database_from_yaml - return nil unless database_yaml - - case database_adapter - when 'mysql2' - MYSQL - when 'postgresql' - POSTGRESQL - end - end - - def database_adapter - database_yaml['adapter'] || database_yaml.first.last['adapter'] - end - - def database_yaml - return nil unless File.exist?('config/database.yml') - - yaml = if YAML.respond_to?(:unsafe_load_file) - YAML.unsafe_load_file('config/database.yml') - else - YAML.load_file('config/database.yml') - end - return nil unless yaml.is_a? Hash - - config = yaml['development'] - return nil unless config.is_a?(Hash) - - config - rescue Psych::SyntaxError - nil - end - - def database_from_env - url = ENV['DATABASE_URL'].presence - return nil unless url - - case url - when %r{\Amysql2://} - MYSQL - when %r{\Apostgres(ql)?://} - POSTGRESQL - end - end - def support_bulk_alter? case database when MYSQL @@ -262,7 +212,7 @@ def combinable_transformations # @param node [RuboCop::AST::SendNode] def add_offense_for_alter_methods(node) # arguments: [{(sym :table)(str "table")} ...] - table_node = node.arguments[0] + table_node = node.first_argument return unless table_node.is_a? RuboCop::AST::BasicLiteralNode message = format(MSG_FOR_ALTER_METHODS, table: table_node.value) @@ -284,10 +234,10 @@ def initialize # @param new_node [RuboCop::AST::SendNode] def process(new_node) # arguments: [{(sym :table)(str "table")} ...] - table_node = new_node.arguments[0] + table_node = new_node.first_argument if table_node.is_a? RuboCop::AST::BasicLiteralNode flush unless @nodes.all? do |node| - node.arguments[0].value.to_s == table_node.value.to_s + node.first_argument.value.to_s == table_node.value.to_s end @nodes << new_node else diff --git a/lib/rubocop/cop/rails/dangerous_column_names.rb b/lib/rubocop/cop/rails/dangerous_column_names.rb index d50a3a006b..a1c1bcc1bb 100644 --- a/lib/rubocop/cop/rails/dangerous_column_names.rb +++ b/lib/rubocop/cop/rails/dangerous_column_names.rb @@ -31,10 +31,11 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength time ].to_set.freeze - # Generated from `ActiveRecord::AttributeMethods.dangerous_attribute_methods` on activerecord 7.0.5. + # Generated from `ActiveRecord::AttributeMethods.dangerous_attribute_methods` on activerecord 7.1.0. # rubocop:disable Metrics/CollectionLiteralLength DANGEROUS_COLUMN_NAMES = %w[ __callbacks + __id__ _assign_attribute _assign_attributes _before_commit_callbacks @@ -195,11 +196,13 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength changes_to_save check_record_limit ciphertext_for + class clear_attribute_change clear_attribute_changes clear_changes_information clear_timestamp_attributes clear_transaction_record_state + clone collection_cache_versioning column_for_attribute committed @@ -227,6 +230,7 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength destroyed destroyed_by_association destroyed_by_association= + dup each_counter_cached_associations encode_with encrypt @@ -243,7 +247,9 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength find_parameter_position forget_attribute_assignments format_for_inspect + freeze from_json + frozen? halted_callback_hook has_attribute has_changes_to_save @@ -252,6 +258,7 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength has_encrypted_attributes has_encrypted_rich_texts has_transactional_callbacks + hash id id_before_type_cast id_for_database @@ -283,6 +290,7 @@ class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength new_record no_touching normalize_reflection_attribute + object_id partial_inserts partial_updates perform_validations @@ -420,7 +428,7 @@ def column_name_node_from(node) when :rename_column node.arguments[2] when *COLUMN_TYPE_METHOD_NAMES - node.arguments[0] + node.first_argument end end diff --git a/lib/rubocop/cop/rails/duplicate_association.rb b/lib/rubocop/cop/rails/duplicate_association.rb index 968585fa7a..0373fa4530 100644 --- a/lib/rubocop/cop/rails/duplicate_association.rb +++ b/lib/rubocop/cop/rails/duplicate_association.rb @@ -20,6 +20,15 @@ module Rails # belongs_to :bar # has_one :foo # + # # bad + # has_many :foo, class_name: 'Foo' + # has_many :bar, class_name: 'Foo' + # has_one :baz + # + # # good + # has_many :bar, class_name: 'Foo' + # has_one :foo + # class DuplicateAssociation < Base include RangeHelp extend AutoCorrector @@ -27,31 +36,76 @@ class DuplicateAssociation < Base include ActiveRecordHelper MSG = "Association `%s` is defined multiple times. Don't repeat associations." + MSG_CLASS_NAME = "Association `class_name: %s` is defined multiple times. Don't repeat associations." def_node_matcher :association, <<~PATTERN - (send nil? {:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) ...) + (send nil? {:belongs_to :has_one :has_many :has_and_belongs_to_many} ({sym str} $_) $...) + PATTERN + + def_node_matcher :class_name, <<~PATTERN + (hash (pair (sym :class_name) $_)) PATTERN def on_class(class_node) return unless active_record?(class_node.parent_class) - offenses(class_node).each do |name, nodes| - nodes.each do |node| - add_offense(node, message: format(MSG, name: name)) do |corrector| - next if same_line?(nodes.last, node) + association_nodes = association_nodes(class_node) - corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true)) - end - end + duplicated_association_name_nodes(association_nodes).each do |name, nodes| + register_offense(name, nodes, MSG) + end + + duplicated_class_name_nodes(association_nodes).each do |class_name, nodes| + register_offense(class_name, nodes, MSG_CLASS_NAME) end end private - def offenses(class_node) - class_send_nodes(class_node).select { |node| association(node) } - .group_by { |node| association(node).to_sym } - .select { |_, nodes| nodes.length > 1 } + def register_offense(name, nodes, message_template) + nodes.each do |node| + add_offense(node, message: format(message_template, name: name)) do |corrector| + next if same_line?(nodes.last, node) + + corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true)) + end + end + end + + def association_nodes(class_node) + class_send_nodes(class_node).select do |node| + association(node)&.first + end + end + + def duplicated_association_name_nodes(association_nodes) + grouped_associations = association_nodes.group_by do |node| + association(node).first.to_sym + end + + leave_duplicated_association(grouped_associations) + end + + def duplicated_class_name_nodes(association_nodes) + filtered_nodes = association_nodes.reject { |node| node.method?(:belongs_to) } + grouped_associations = filtered_nodes.group_by do |node| + arguments = association(node).last + next unless arguments.count == 1 + + if (class_name = class_name(arguments.first)) + class_name.source + end + end + + grouped_associations.delete(nil) + + leave_duplicated_association(grouped_associations) + end + + def leave_duplicated_association(grouped_associations) + grouped_associations.select do |_, nodes| + nodes.length > 1 + end end end end diff --git a/lib/rubocop/cop/rails/eager_evaluation_log_message.rb b/lib/rubocop/cop/rails/eager_evaluation_log_message.rb index 8fc230121f..45b3513c93 100644 --- a/lib/rubocop/cop/rails/eager_evaluation_log_message.rb +++ b/lib/rubocop/cop/rails/eager_evaluation_log_message.rb @@ -14,10 +14,10 @@ module Rails # when no output would be produced anyway. # # @example - # #bad + # # bad # Rails.logger.debug "The time is #{Time.zone.now}." # - # #good + # # good # Rails.logger.debug { "The time is #{Time.zone.now}." } # class EagerEvaluationLogMessage < Base diff --git a/lib/rubocop/cop/rails/env_local.rb b/lib/rubocop/cop/rails/env_local.rb new file mode 100644 index 0000000000..b76a354647 --- /dev/null +++ b/lib/rubocop/cop/rails/env_local.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Checks for usage of `Rails.env.development? || Rails.env.test?` which + # can be replaced with `Rails.env.local?`, introduced in Rails 7.1. + # + # @example + # + # # bad + # Rails.env.development? || Rails.env.test? + # + # # good + # Rails.env.local? + # + class EnvLocal < Base + extend AutoCorrector + extend TargetRailsVersion + + MSG = 'Use `Rails.env.local?` instead.' + LOCAL_ENVIRONMENTS = %i[development? test?].to_set.freeze + + minimum_target_rails_version 7.1 + + # @!method rails_env_local_candidate?(node) + def_node_matcher :rails_env_local_candidate?, <<~PATTERN + (or + (send (send (const {cbase nil? } :Rails) :env) $%LOCAL_ENVIRONMENTS) + (send (send (const {cbase nil? } :Rails) :env) $%LOCAL_ENVIRONMENTS) + ) + PATTERN + + def on_or(node) + rails_env_local_candidate?(node) do |*environments| + next unless environments.to_set == LOCAL_ENVIRONMENTS + + add_offense(node) do |corrector| + corrector.replace(node, 'Rails.env.local?') + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails/file_path.rb b/lib/rubocop/cop/rails/file_path.rb index ee9ef97f71..f98f37076f 100644 --- a/lib/rubocop/cop/rails/file_path.rb +++ b/lib/rubocop/cop/rails/file_path.rb @@ -163,9 +163,9 @@ def autocorrect_slash_after_rails_root_in_dstr(corrector, node, rails_root_index def autocorrect_extension_after_rails_root_join_in_dstr(corrector, node, rails_root_index, extension_node) rails_root_node = node.children[rails_root_index].children.first - return unless rails_root_node.arguments.last.str_type? + return unless rails_root_node.last_argument.str_type? - corrector.insert_before(rails_root_node.arguments.last.location.end, extension_node.source) + corrector.insert_before(rails_root_node.last_argument.location.end, extension_node.source) corrector.remove(extension_node) end @@ -174,7 +174,7 @@ def autocorrect_file_join(corrector, node) corrector.remove( range_with_surrounding_space( range_with_surrounding_comma( - node.arguments.first.source_range, + node.first_argument.source_range, :right ), side: :right @@ -187,7 +187,7 @@ def autocorrect_file_join(corrector, node) end def autocorrect_rails_root_join_with_string_arguments(corrector, node) - corrector.replace(node.arguments.first, %("#{node.arguments.map(&:value).join('/')}")) + corrector.replace(node.first_argument, %("#{node.arguments.map(&:value).join('/')}")) node.arguments[1..].each do |argument| corrector.remove( range_with_surrounding_comma( @@ -221,7 +221,7 @@ def find_rails_root_index(node) end def append_argument(corrector, node, argument_source) - corrector.insert_after(node.arguments.last, %(, "#{argument_source}")) + corrector.insert_after(node.last_argument, %(, "#{argument_source}")) end def replace_with_rails_root_join(corrector, node, argument_source) diff --git a/lib/rubocop/cop/rails/find_by.rb b/lib/rubocop/cop/rails/find_by.rb index 74da0840f3..28ad3200ef 100644 --- a/lib/rubocop/cop/rails/find_by.rb +++ b/lib/rubocop/cop/rails/find_by.rb @@ -28,7 +28,7 @@ class FindBy < Base include RangeHelp extend AutoCorrector - MSG = 'Use `find_by` instead of `where.%s`.' + MSG = 'Use `find_by` instead of `where%s%s`.' RESTRICT_ON_SEND = %i[first take].freeze def on_send(node) @@ -37,7 +37,7 @@ def on_send(node) range = offense_range(node) - add_offense(range, message: format(MSG, method: node.method_name)) do |corrector| + add_offense(range, message: format(MSG, dot: node.loc.dot.source, method: node.method_name)) do |corrector| autocorrect(corrector, node) end end diff --git a/lib/rubocop/cop/rails/find_by_id.rb b/lib/rubocop/cop/rails/find_by_id.rb index 9e6dc1c274..10bd10b2b4 100644 --- a/lib/rubocop/cop/rails/find_by_id.rb +++ b/lib/rubocop/cop/rails/find_by_id.rb @@ -24,40 +24,39 @@ class FindById < Base RESTRICT_ON_SEND = %i[take! find_by_id! find_by!].freeze def_node_matcher :where_take?, <<~PATTERN - (send - $(send _ :where + (call + $(call _ :where (hash (pair (sym :id) $_))) :take!) PATTERN def_node_matcher :find_by?, <<~PATTERN { - (send _ :find_by_id! $_) - (send _ :find_by! (hash (pair (sym :id) $_))) + (call _ :find_by_id! $_) + (call _ :find_by! (hash (pair (sym :id) $_))) } PATTERN def on_send(node) where_take?(node) do |where, id_value| range = where_take_offense_range(node, where) - bad_method = build_where_take_bad_method(id_value) - register_offense(range, id_value, bad_method) + register_offense(range, id_value) end find_by?(node) do |id_value| range = find_by_offense_range(node) - bad_method = build_find_by_bad_method(node, id_value) - register_offense(range, id_value, bad_method) + register_offense(range, id_value) end end + alias on_csend on_send private - def register_offense(range, id_value, bad_method) + def register_offense(range, id_value) good_method = build_good_method(id_value) - message = format(MSG, good_method: good_method, bad_method: bad_method) + message = format(MSG, good_method: good_method, bad_method: range.source) add_offense(range, message: message) do |corrector| corrector.replace(range, good_method) @@ -75,19 +74,6 @@ def find_by_offense_range(node) def build_good_method(id_value) "find(#{id_value.source})" end - - def build_where_take_bad_method(id_value) - "where(id: #{id_value.source}).take!" - end - - def build_find_by_bad_method(node, id_value) - case node.method_name - when :find_by_id! - "find_by_id!(#{id_value.source})" - when :find_by! - "find_by!(id: #{id_value.source})" - end - end end end end diff --git a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb index a00f93c52c..a5ec9c70ec 100644 --- a/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb +++ b/lib/rubocop/cop/rails/has_many_or_has_one_dependent.rb @@ -60,7 +60,7 @@ class HasManyOrHasOneDependent < Base (block (send nil? :with_options (hash $...)) - (args) ...) + (args _?) ...) PATTERN def_node_matcher :association_extension_block?, <<~PATTERN diff --git a/lib/rubocop/cop/rails/helper_instance_variable.rb b/lib/rubocop/cop/rails/helper_instance_variable.rb index 92b327c308..60e8db050d 100644 --- a/lib/rubocop/cop/rails/helper_instance_variable.rb +++ b/lib/rubocop/cop/rails/helper_instance_variable.rb @@ -6,7 +6,7 @@ module Rails # Checks for use of the helper methods which reference # instance variables. # - # Relying on instance variables makes it difficult to re-use helper + # Relying on instance variables makes it difficult to reuse helper # methods. # # If it seems awkward to explicitly pass in each dependent diff --git a/lib/rubocop/cop/rails/inquiry.rb b/lib/rubocop/cop/rails/inquiry.rb index a7933a9347..45ad7ecd65 100644 --- a/lib/rubocop/cop/rails/inquiry.rb +++ b/lib/rubocop/cop/rails/inquiry.rb @@ -33,6 +33,7 @@ def on_send(node) add_offense(node.loc.selector) end + alias on_csend on_send end end end diff --git a/lib/rubocop/cop/rails/inverse_of.rb b/lib/rubocop/cop/rails/inverse_of.rb index c4dbd0e8eb..7b04522714 100644 --- a/lib/rubocop/cop/rails/inverse_of.rb +++ b/lib/rubocop/cop/rails/inverse_of.rb @@ -222,7 +222,7 @@ def options_contain_inverse_of?(options) def with_options_arguments(recv, node) blocks = node.each_ancestor(:block).select do |block| - block.send_node.command?(:with_options) && same_context_in_with_options?(block.arguments.first, recv) + block.send_node.command?(:with_options) && same_context_in_with_options?(block.first_argument, recv) end blocks.flat_map { |n| n.send_node.arguments } end diff --git a/lib/rubocop/cop/rails/not_null_column.rb b/lib/rubocop/cop/rails/not_null_column.rb index fde967c3ae..2abd0927c8 100644 --- a/lib/rubocop/cop/rails/not_null_column.rb +++ b/lib/rubocop/cop/rails/not_null_column.rb @@ -3,8 +3,13 @@ module RuboCop module Cop module Rails - # Checks for add_column call with NOT NULL constraint - # in migration file. + # Checks for add_column call with NOT NULL constraint in migration file. + # + # `TEXT` can have default values in PostgreSQL, but not in MySQL. + # It will automatically detect an adapter from `development` environment + # in `config/database.yml` or the environment variable `DATABASE_URL` + # when the `Database` option is not set. If the database is MySQL, + # this cop ignores offenses for the `TEXT`. # # @example # # bad @@ -17,6 +22,8 @@ module Rails # add_reference :products, :category # add_reference :products, :category, null: false, default: 1 class NotNullColumn < Base + include DatabaseTypeResolvable + MSG = 'Do not add a NOT NULL column without a default value.' RESTRICT_ON_SEND = %i[add_column add_reference].freeze @@ -45,7 +52,10 @@ def on_send(node) def check_add_column(node) add_not_null_column?(node) do |type, pairs| - return if type.respond_to?(:value) && (type.value == :virtual || type.value == 'virtual') + if type.respond_to?(:value) + return if type.value == :virtual || type.value == 'virtual' + return if (type.value == :text || type.value == 'text') && database == MYSQL + end check_pairs(pairs) end diff --git a/lib/rubocop/cop/rails/output.rb b/lib/rubocop/cop/rails/output.rb index 43b104c8f2..067fdb4be8 100644 --- a/lib/rubocop/cop/rails/output.rb +++ b/lib/rubocop/cop/rails/output.rb @@ -23,6 +23,7 @@ class Output < Base MSG = "Do not write to stdout. Use Rails's logger if you want to log." RESTRICT_ON_SEND = %i[ap p pp pretty_print print puts binwrite syswrite write write_nonblock].freeze + ALLOWED_TYPES = %i[send csend block numblock].freeze def_node_matcher :output?, <<~PATTERN (send nil? {:ap :p :pp :pretty_print :print :puts} ...) @@ -39,8 +40,8 @@ class Output < Base PATTERN def on_send(node) - return if node.parent&.call_type? - return unless output?(node) || io_output?(node) + return if ALLOWED_TYPES.include?(node.parent&.type) + return if !output?(node) && !io_output?(node) range = offense_range(node) diff --git a/lib/rubocop/cop/rails/pick.rb b/lib/rubocop/cop/rails/pick.rb index a13ba846f0..f4f344ff9c 100644 --- a/lib/rubocop/cop/rails/pick.rb +++ b/lib/rubocop/cop/rails/pick.rb @@ -28,13 +28,13 @@ class Pick < Base extend AutoCorrector extend TargetRailsVersion - MSG = 'Prefer `pick(%s)` over `pluck(%s).first`.' + MSG = 'Prefer `pick(%s)` over `%s`.' RESTRICT_ON_SEND = %i[first].freeze minimum_target_rails_version 6.0 def_node_matcher :pick_candidate?, <<~PATTERN - (send (send _ :pluck ...) :first) + (call (call _ :pluck ...) :first) PATTERN def on_send(node) @@ -44,7 +44,7 @@ def on_send(node) node_selector = node.loc.selector range = receiver_selector.join(node_selector) - add_offense(range, message: message(receiver)) do |corrector| + add_offense(range, message: message(receiver, range)) do |corrector| first_range = receiver.source_range.end.join(node_selector) corrector.remove(first_range) @@ -52,11 +52,12 @@ def on_send(node) end end end + alias on_csend on_send private - def message(receiver) - format(MSG, args: receiver.arguments.map(&:source).join(', ')) + def message(receiver, current) + format(MSG, args: receiver.arguments.map(&:source).join(', '), current: current.source) end end end diff --git a/lib/rubocop/cop/rails/pluck.rb b/lib/rubocop/cop/rails/pluck.rb index ee10214853..a6094ad648 100644 --- a/lib/rubocop/cop/rails/pluck.rb +++ b/lib/rubocop/cop/rails/pluck.rb @@ -38,7 +38,7 @@ class Pluck < Base minimum_target_rails_version 5.0 def_node_matcher :pluck_candidate?, <<~PATTERN - ({block numblock} (send _ {:map :collect}) $_argument (send lvar :[] $_key)) + ({block numblock} (call _ {:map :collect}) $_argument (send lvar :[] $_key)) PATTERN def on_block(node) diff --git a/lib/rubocop/cop/rails/pluck_id.rb b/lib/rubocop/cop/rails/pluck_id.rb index 4ba2e109b2..8ef983c280 100644 --- a/lib/rubocop/cop/rails/pluck_id.rb +++ b/lib/rubocop/cop/rails/pluck_id.rb @@ -34,7 +34,7 @@ class PluckId < Base RESTRICT_ON_SEND = %i[pluck].freeze def_node_matcher :pluck_id_call?, <<~PATTERN - (send _ :pluck {(sym :id) (send nil? :primary_key)}) + (call _ :pluck {(sym :id) (send nil? :primary_key)}) PATTERN def on_send(node) @@ -47,6 +47,7 @@ def on_send(node) corrector.replace(offense_range(node), 'ids') end end + alias on_csend on_send private diff --git a/lib/rubocop/cop/rails/pluck_in_where.rb b/lib/rubocop/cop/rails/pluck_in_where.rb index 1da5967386..05d0fb9715 100644 --- a/lib/rubocop/cop/rails/pluck_in_where.rb +++ b/lib/rubocop/cop/rails/pluck_in_where.rb @@ -22,10 +22,13 @@ module Rails # @example # # bad # Post.where(user_id: User.active.pluck(:id)) + # Post.where(user_id: User.active.ids) + # Post.where.not(user_id: User.active.pluck(:id)) # # # good # Post.where(user_id: User.active.select(:id)) # Post.where(user_id: active_users.select(:id)) + # Post.where.not(user_id: active_users.select(:id)) # # @example EnforcedStyle: conservative (default) # # good @@ -40,8 +43,9 @@ class PluckInWhere < Base include ConfigurableEnforcedStyle extend AutoCorrector - MSG = 'Use `select` instead of `pluck` within `where` query method.' - RESTRICT_ON_SEND = %i[pluck].freeze + MSG_SELECT = 'Use `select` instead of `pluck` within `where` query method.' + MSG_IDS = 'Use `select(:id)` instead of `ids` within `where` query method.' + RESTRICT_ON_SEND = %i[pluck ids].freeze def on_send(node) return unless in_where?(node) @@ -49,17 +53,26 @@ def on_send(node) range = node.loc.selector - add_offense(range) do |corrector| - corrector.replace(range, 'select') + if node.method?(:ids) + replacement = 'select(:id)' + message = MSG_IDS + else + replacement = 'select' + message = MSG_SELECT + end + + add_offense(range, message: message) do |corrector| + corrector.replace(range, replacement) end end + alias on_csend on_send private def root_receiver(node) receiver = node.receiver - if receiver&.send_type? + if receiver&.call_type? root_receiver(receiver) else receiver diff --git a/lib/rubocop/cop/rails/rake_environment.rb b/lib/rubocop/cop/rails/rake_environment.rb index 78274ebf1c..bf8a285d4e 100644 --- a/lib/rubocop/cop/rails/rake_environment.rb +++ b/lib/rubocop/cop/rails/rake_environment.rb @@ -72,7 +72,7 @@ def correct_task_dependency(task_name) end def task_name(node) - first_arg = node.arguments[0] + first_arg = node.first_argument case first_arg&.type when :sym, :str first_arg.value.to_sym @@ -97,7 +97,7 @@ def with_arguments?(node) end def with_dependencies?(node) - first_arg = node.arguments[0] + first_arg = node.first_argument return false unless first_arg if first_arg.hash_type? diff --git a/lib/rubocop/cop/rails/redundant_active_record_all_method.rb b/lib/rubocop/cop/rails/redundant_active_record_all_method.rb index a2a7f645b0..ee5fa6afd6 100644 --- a/lib/rubocop/cop/rails/redundant_active_record_all_method.rb +++ b/lib/rubocop/cop/rails/redundant_active_record_all_method.rb @@ -3,8 +3,43 @@ module RuboCop module Cop module Rails + # TODO: In the future, please support only RuboCop 1.52+ and use `RuboCop::Cop::AllowedReceivers`: + # https://github.com/rubocop/rubocop/blob/v1.52.0/lib/rubocop/cop/mixin/allowed_receivers.rb + # At that time, this duplicated module implementation can be removed. + module AllowedReceivers + def allowed_receiver?(receiver) + receiver_name = receiver_name(receiver) + + allowed_receivers.include?(receiver_name) + end + + def receiver_name(receiver) + return receiver_name(receiver.receiver) if receiver.receiver && !receiver.receiver.const_type? + + if receiver.send_type? + if receiver.receiver + "#{receiver_name(receiver.receiver)}.#{receiver.method_name}" + else + receiver.method_name.to_s + end + else + receiver.source + end + end + + def allowed_receivers + cop_config.fetch('AllowedReceivers', []) + end + end + # Detect redundant `all` used as a receiver for Active Record query methods. # + # For the methods `delete_all` and `destroy_all`, this cop will only check cases where the receiver is a model. + # It will ignore cases where the receiver is an association (e.g., `user.articles.all.delete_all`). + # This is because omitting `all` from an association changes the methods + # from `ActiveRecord::Relation` to `ActiveRecord::Associations::CollectionProxy`, + # which can affect their behavior. + # # @safety # This cop is unsafe for autocorrection if the receiver for `all` is not an Active Record object. # @@ -35,11 +70,19 @@ class RedundantActiveRecordAllMethod < Base RESTRICT_ON_SEND = [:all].freeze - # Defined methods in `ActiveRecord::Querying::QUERYING_METHODS` on activerecord 7.0.5. + # Defined methods in `ActiveRecord::Querying::QUERYING_METHODS` on activerecord 7.1.0. QUERYING_METHODS = %i[ and annotate any? + async_average + async_count + async_ids + async_maximum + async_minimum + async_pick + async_pluck + async_sum average calculate count @@ -109,6 +152,7 @@ class RedundantActiveRecordAllMethod < Base preload readonly references + regroup reorder reselect rewhere @@ -130,17 +174,20 @@ class RedundantActiveRecordAllMethod < Base unscope update_all where + with without ].to_set.freeze POSSIBLE_ENUMERABLE_BLOCK_METHODS = %i[any? count find none? one? select sum].freeze + SENSITIVE_METHODS_ON_ASSOCIATION = %i[delete_all destroy_all].freeze def_node_matcher :followed_by_query_method?, <<~PATTERN (send (send _ :all) QUERYING_METHODS ...) PATTERN def on_send(node) - return if !followed_by_query_method?(node.parent) || possible_enumerable_block_method?(node) + return unless followed_by_query_method?(node.parent) + return if possible_enumerable_block_method?(node) || sensitive_association_method?(node) return if node.receiver ? allowed_receiver?(node.receiver) : !inherit_active_record_base?(node) range_of_all_method = offense_range(node) @@ -159,6 +206,10 @@ def possible_enumerable_block_method?(node) parent.parent&.block_type? || parent.parent&.numblock_type? || parent.first_argument&.block_pass_type? end + def sensitive_association_method?(node) + !node.receiver&.const_type? && SENSITIVE_METHODS_ON_ASSOCIATION.include?(node.parent.method_name) + end + def offense_range(node) range_between(node.loc.selector.begin_pos, node.source_range.end_pos) end diff --git a/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb b/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb index d8ee2670e7..c6a656eea7 100644 --- a/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb +++ b/lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb @@ -53,6 +53,12 @@ class RedundantPresenceValidationOnBelongsTo < Base # @example source that matches - by a foreign key # validates :user_id, presence: true # + # @example source that DOES NOT match - if condition + # validates :user_id, presence: true, if: condition + # + # @example source that DOES NOT match - unless condition + # validates :user_id, presence: true, unless: condition + # # @example source that DOES NOT match - strict validation # validates :user_id, presence: true, strict: true # @@ -65,6 +71,7 @@ class RedundantPresenceValidationOnBelongsTo < Base $[ (hash <$(pair (sym :presence) true) ...>) # presence: true !(hash <$(pair (sym :strict) {true const}) ...>) # strict: true + !(hash <$(pair (sym {:if :unless}) _) ...>) # if: some_condition or unless: some_condition ] ) PATTERN diff --git a/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb b/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb index ec6699e96d..62d52786cf 100644 --- a/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb +++ b/lib/rubocop/cop/rails/redundant_receiver_in_with_options.rb @@ -100,7 +100,7 @@ def redundant_receiver?(send_nodes, node) else return false if node.arguments.empty? - arg = node.arguments.first + arg = node.first_argument ->(n) { same_value?(arg, n.receiver) } end diff --git a/lib/rubocop/cop/rails/response_parsed_body.rb b/lib/rubocop/cop/rails/response_parsed_body.rb index d80b2ec06b..184ddcb3a7 100644 --- a/lib/rubocop/cop/rails/response_parsed_body.rb +++ b/lib/rubocop/cop/rails/response_parsed_body.rb @@ -3,25 +3,30 @@ module RuboCop module Cop module Rails - # Prefer `response.parsed_body` to `JSON.parse(response.body)`. + # Prefer `response.parsed_body` to custom parsing logic for `response.body`. # # @safety - # This cop is unsafe because Content-Type may not be `application/json`. For example, the proprietary - # Content-Type provided by corporate entities such as `application/vnd.github+json` is not supported at - # `response.parsed_body` by default, so you still have to use `JSON.parse(response.body)` there. + # This cop is unsafe because Content-Type may not be `application/json` or `text/html`. + # For example, the proprietary Content-Type provided by corporate entities such as + # `application/vnd.github+json` is not supported at `response.parsed_body` by default, + # so you still have to use `JSON.parse(response.body)` there. # # @example # # bad # JSON.parse(response.body) # + # # bad + # Nokogiri::HTML.parse(response.body) + # + # # bad + # Nokogiri::HTML5.parse(response.body) + # # # good # response.parsed_body class ResponseParsedBody < Base extend AutoCorrector extend TargetRailsVersion - MSG = 'Prefer `response.parsed_body` to `JSON.parse(response.body)`.' - RESTRICT_ON_SEND = %i[parse].freeze minimum_target_rails_version 5.0 @@ -38,12 +43,27 @@ class ResponseParsedBody < Base ) PATTERN + # @!method nokogiri_html_parse_response_body(node) + def_node_matcher :nokogiri_html_parse_response_body, <<~PATTERN + (send + (const + (const {nil? cbase} :Nokogiri) + ${:HTML :HTML5} + ) + :parse + (send + (send nil? :response) + :body + ) + ) + PATTERN + def on_send(node) - return unless json_parse_response_body?(node) + check_json_parse_response_body(node) - add_offense(node) do |corrector| - autocorrect(corrector, node) - end + return unless target_rails_version >= 7.1 + + check_nokogiri_html_parse_response_body(node) end private @@ -51,6 +71,28 @@ def on_send(node) def autocorrect(corrector, node) corrector.replace(node, 'response.parsed_body') end + + def check_json_parse_response_body(node) + return unless json_parse_response_body?(node) + + add_offense( + node, + message: 'Prefer `response.parsed_body` to `JSON.parse(response.body)`.' + ) do |corrector| + autocorrect(corrector, node) + end + end + + def check_nokogiri_html_parse_response_body(node) + return unless (const = nokogiri_html_parse_response_body(node)) + + add_offense( + node, + message: "Prefer `response.parsed_body` to `Nokogiri::#{const}.parse(response.body)`." + ) do |corrector| + autocorrect(corrector, node) + end + end end end end diff --git a/lib/rubocop/cop/rails/reversible_migration.rb b/lib/rubocop/cop/rails/reversible_migration.rb index edafc95201..c881797eae 100644 --- a/lib/rubocop/cop/rails/reversible_migration.rb +++ b/lib/rubocop/cop/rails/reversible_migration.rb @@ -290,10 +290,10 @@ def reversible_change_table_call?(node) when :change false when :remove - target_rails_version >= 6.1 && all_hash_key?(node.arguments.last, :type) + target_rails_version >= 6.1 && all_hash_key?(node.last_argument, :type) when :change_default, :change_column_default, :change_table_comment, :change_column_comment - all_hash_key?(node.arguments.last, :from, :to) + all_hash_key?(node.last_argument, :from, :to) else true end diff --git a/lib/rubocop/cop/rails/save_bang.rb b/lib/rubocop/cop/rails/save_bang.rb index 0366837214..133dcc97ee 100644 --- a/lib/rubocop/cop/rails/save_bang.rb +++ b/lib/rubocop/cop/rails/save_bang.rb @@ -235,10 +235,10 @@ def check_used_in_condition_or_compound_boolean(node) def in_condition_or_compound_boolean?(node) node = node.block_node || node - parent = node.parent + parent = node.each_ancestor.find { |ancestor| !ancestor.begin_type? } return false unless parent - operator_or_single_negative?(parent) || (conditional?(parent) && node == parent.condition) + operator_or_single_negative?(parent) || (conditional?(parent) && node == deparenthesize(parent.condition)) end def operator_or_single_negative?(node) @@ -249,6 +249,11 @@ def conditional?(parent) parent.if_type? || parent.case_type? end + def deparenthesize(node) + node = node.children.last while node.begin_type? + node + end + def checked_immediately?(node) node.parent && call_to_persisted?(node.parent) end @@ -331,10 +336,10 @@ def persist_method?(node, methods = RESTRICT_ON_SEND) # Check argument signature as no arguments or one hash def expected_signature?(node) - !node.arguments? || - (node.arguments.one? && - node.method_name != :destroy && - (node.first_argument.hash_type? || !node.first_argument.literal?)) + return true unless node.arguments? + return false if !node.arguments.one? || node.method?(:destroy) + + node.first_argument.hash_type? || !node.first_argument.literal? end end end diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index 15aaa7758b..f355c2c6e8 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -124,7 +124,7 @@ def class_node(node) end def uniqueness_part(node) - pairs = node.arguments.last + pairs = node.last_argument return unless pairs&.hash_type? pairs.each_pair.find do |pair| diff --git a/lib/rubocop/cop/rails/unknown_env.rb b/lib/rubocop/cop/rails/unknown_env.rb index c3d8d467eb..ebb62ce908 100644 --- a/lib/rubocop/cop/rails/unknown_env.rb +++ b/lib/rubocop/cop/rails/unknown_env.rb @@ -86,7 +86,11 @@ def unknown_env_name?(name) end def environments - cop_config['Environments'] + @environments ||= begin + environments = cop_config['Environments'] || [] + environments << 'local' if target_rails_version >= 7.1 + environments + end end end end diff --git a/lib/rubocop/cop/rails/validation.rb b/lib/rubocop/cop/rails/validation.rb index f250d5d8a4..534af1b260 100644 --- a/lib/rubocop/cop/rails/validation.rb +++ b/lib/rubocop/cop/rails/validation.rb @@ -51,7 +51,7 @@ class Validation < Base uniqueness ].freeze - RESTRICT_ON_SEND = TYPES.map { |p| "validates_#{p}_of".to_sym }.freeze + RESTRICT_ON_SEND = TYPES.map { |p| :"validates_#{p}_of" }.freeze ALLOWLIST = TYPES.map { |p| "validates :column, #{p}: value" }.freeze def on_send(node) @@ -60,7 +60,7 @@ def on_send(node) range = node.loc.selector add_offense(range, message: message(node)) do |corrector| - last_argument = node.arguments.last + last_argument = node.last_argument return if !last_argument.literal? && !last_argument.splat_type? && !frozen_array_argument?(last_argument) corrector.replace(range, 'validates') diff --git a/lib/rubocop/cop/rails/where_equals.rb b/lib/rubocop/cop/rails/where_equals.rb index db9072d867..d7a568e8e4 100644 --- a/lib/rubocop/cop/rails/where_equals.rb +++ b/lib/rubocop/cop/rails/where_equals.rb @@ -33,8 +33,8 @@ class WhereEquals < Base def_node_matcher :where_method_call?, <<~PATTERN { - (send _ :where (array $str_type? $_ ?)) - (send _ :where $str_type? $_ ?) + (call _ :where (array $str_type? $_ ?)) + (call _ :where $str_type? $_ ?) } PATTERN @@ -55,6 +55,7 @@ def on_send(node) end end end + alias on_csend on_send EQ_ANONYMOUS_RE = /\A([\w.]+)\s+=\s+\?\z/.freeze # column = ? IN_ANONYMOUS_RE = /\A([\w.]+)\s+IN\s+\(\?\)\z/i.freeze # column IN (?) diff --git a/lib/rubocop/cop/rails/where_exists.rb b/lib/rubocop/cop/rails/where_exists.rb index 8893213416..ff3b064b0c 100644 --- a/lib/rubocop/cop/rails/where_exists.rb +++ b/lib/rubocop/cop/rails/where_exists.rb @@ -55,11 +55,11 @@ class WhereExists < Base RESTRICT_ON_SEND = %i[exists?].freeze def_node_matcher :where_exists_call?, <<~PATTERN - (send (send _ :where $...) :exists?) + (call (call _ :where $...) :exists?) PATTERN def_node_matcher :exists_with_args?, <<~PATTERN - (send _ :exists? $...) + (call _ :exists? $...) PATTERN def on_send(node) @@ -67,7 +67,7 @@ def on_send(node) return unless convertable_args?(args) range = correction_range(node) - good_method = build_good_method(args) + good_method = build_good_method(args, dot_source: node.loc.dot.source) message = format(MSG, good_method: good_method, bad_method: range.source) add_offense(range, message: message) do |corrector| @@ -75,6 +75,7 @@ def on_send(node) end end end + alias on_csend on_send private @@ -108,11 +109,11 @@ def correction_range(node) end end - def build_good_method(args) + def build_good_method(args, dot_source: '.') if exists_style? build_good_method_exists(args) elsif where_style? - build_good_method_where(args) + build_good_method_where(args, dot_source) end end @@ -124,11 +125,11 @@ def build_good_method_exists(args) end end - def build_good_method_where(args) + def build_good_method_where(args, dot_source) if args.size > 1 - "where(#{args.map(&:source).join(', ')}).exists?" + "where(#{args.map(&:source).join(', ')})#{dot_source}exists?" else - "where(#{args[0].source}).exists?" + "where(#{args[0].source})#{dot_source}exists?" end end end diff --git a/lib/rubocop/cop/rails/where_missing.rb b/lib/rubocop/cop/rails/where_missing.rb index 168d2516d1..915fd53301 100644 --- a/lib/rubocop/cop/rails/where_missing.rb +++ b/lib/rubocop/cop/rails/where_missing.rb @@ -36,7 +36,7 @@ class WhereMissing < Base PATTERN def on_send(node) - return unless node.first_argument.sym_type? + return unless node.first_argument&.sym_type? root_receiver = root_receiver(node) where_node_and_argument(root_receiver) do |where_node, where_argument| diff --git a/lib/rubocop/cop/rails/where_not.rb b/lib/rubocop/cop/rails/where_not.rb index 1e2aab2b65..fd6ad6d65e 100644 --- a/lib/rubocop/cop/rails/where_not.rb +++ b/lib/rubocop/cop/rails/where_not.rb @@ -32,8 +32,8 @@ class WhereNot < Base def_node_matcher :where_method_call?, <<~PATTERN { - (send _ :where (array $str_type? $_ ?)) - (send _ :where $str_type? $_ ?) + (call _ :where (array $str_type? $_ ?)) + (call _ :where $str_type? $_ ?) } PATTERN @@ -46,7 +46,7 @@ def on_send(node) column_and_value = extract_column_and_value(template_node, value_node) return unless column_and_value - good_method = build_good_method(*column_and_value) + good_method = build_good_method(node.loc.dot.source, *column_and_value) message = format(MSG, good_method: good_method) add_offense(range, message: message) do |corrector| @@ -54,6 +54,7 @@ def on_send(node) end end end + alias on_csend on_send NOT_EQ_ANONYMOUS_RE = /\A([\w.]+)\s+(?:!=|<>)\s+\?\z/.freeze # column != ?, column <> ? NOT_IN_ANONYMOUS_RE = /\A([\w.]+)\s+NOT\s+IN\s+\(\?\)\z/i.freeze # column NOT IN (?) @@ -86,13 +87,13 @@ def extract_column_and_value(template_node, value_node) [Regexp.last_match(1), value] end - def build_good_method(column, value) + def build_good_method(dot, column, value) if column.include?('.') table, column = column.split('.') - "where.not(#{table}: { #{column}: #{value} })" + "where#{dot}not(#{table}: { #{column}: #{value} })" else - "where.not(#{column}: #{value})" + "where#{dot}not(#{column}: #{value})" end end end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index c10f028f4d..0d92ff4ca5 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -3,6 +3,7 @@ require_relative 'mixin/active_record_helper' require_relative 'mixin/active_record_migrations_helper' require_relative 'mixin/class_send_node_helper' +require_relative 'mixin/database_type_resolvable' require_relative 'mixin/enforce_superclass' require_relative 'mixin/index_method' require_relative 'mixin/migrations_helper' @@ -46,6 +47,7 @@ require_relative 'rails/eager_evaluation_log_message' require_relative 'rails/enum_hash' require_relative 'rails/enum_uniqueness' +require_relative 'rails/env_local' require_relative 'rails/environment_comparison' require_relative 'rails/environment_variable_access' require_relative 'rails/exit' diff --git a/lib/rubocop/rails/schema_loader/schema.rb b/lib/rubocop/rails/schema_loader/schema.rb index 424441d975..7bb511d5a7 100644 --- a/lib/rubocop/rails/schema_loader/schema.rb +++ b/lib/rubocop/rails/schema_loader/schema.rb @@ -127,7 +127,7 @@ def initialize(node) private def analyze_keywords!(node) - pairs = node.arguments.last + pairs = node.last_argument return unless pairs.hash_type? pairs.each_pair do |k, v| @@ -158,7 +158,7 @@ def build_columns_or_expr(columns) end def analyze_keywords!(node) - pairs = node.arguments.last + pairs = node.last_argument return unless pairs.hash_type? pairs.each_pair do |k, v| diff --git a/lib/rubocop/rails/version.rb b/lib/rubocop/rails/version.rb index 9e9f054da0..4b20fde50f 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.21.2' + STRING = '2.23.0' def self.document_version STRING.match('\d+\.\d+').to_s diff --git a/relnotes/v2.22.0.md b/relnotes/v2.22.0.md new file mode 100644 index 0000000000..966a44811a --- /dev/null +++ b/relnotes/v2.22.0.md @@ -0,0 +1,28 @@ +### New features + +* [#906](https://github.com/rubocop/rubocop-rails/pull/906): Add `Rails/EnvLocal` cop. ([@sambostock][]) +* [#1128](https://github.com/rubocop/rubocop-rails/issues/1128): Make `Rails/DuplicateAssociation` aware of duplicate `class_name`. ([@koic][]) +* [#1157](https://github.com/rubocop/rubocop-rails/pull/1157): Support some Rails 7.1's new querying methods for `Rails/RedundantActiveRecordAllMethod`. ([@koic][]) +* [#1147](https://github.com/rubocop/rubocop-rails/issues/1147): Support the Trilogy adapter for MySQL. ([@koic][]) + +### Bug fixes + +* [#952](https://github.com/rubocop/rubocop-rails/issues/952): Fix a false positive for `Rails/NotNullColumn` when using `null: false` for MySQL's TEXT type. ([@koic][]) +* [#1041](https://github.com/rubocop/rubocop-rails/issues/1041): Fix a false positive for `Rails/Output` when output method is called with block argument. ([@koic][]) +* [#1143](https://github.com/rubocop/rubocop-rails/issues/1143): Fix an error for `Rails/RedundantActiveRecordAllMethod` when using RuboCop 1.51 or lower. ([@koic][]) +* [#1105](https://github.com/rubocop/rubocop-rails/issues/1105): Fix false positives for `Rails/RedundantPresenceValidationOnBelongsTo` when using `validates` with `:if` or `:unless` options. ([@koic][]) +* [#1158](https://github.com/rubocop/rubocop-rails/issues/1158): `Rails/HasManyOrHasOneDependent` does not add offence when has_many or has_one is called on an explicit receiver. ([@samrjenkins][]) +* [#1160](https://github.com/rubocop/rubocop-rails/issues/1160): Fix `Rails/SaveBang` to ignore parenthesis. ([@fatkodima][]) + +### Changes + +* [#1152](https://github.com/rubocop/rubocop-rails/pull/1152): Add more dangerous column names to `Rails/DangerousColumnNames`. ([@r7kamura][]) +* [#1039](https://github.com/rubocop/rubocop-rails/issues/1039): Deprecate `Rails/ActionFilter` cop; it will be disabled by default. ([@koic][]) +* [#893](https://github.com/rubocop/rubocop-rails/issues/893): Support `local` as an environment for `Rails/UnknownEnv` from Rails 7.1 onward. ([@ghiculescu][]) + +[@sambostock]: https://github.com/sambostock +[@koic]: https://github.com/koic +[@samrjenkins]: https://github.com/samrjenkins +[@fatkodima]: https://github.com/fatkodima +[@r7kamura]: https://github.com/r7kamura +[@ghiculescu]: https://github.com/ghiculescu diff --git a/relnotes/v2.22.1.md b/relnotes/v2.22.1.md new file mode 100644 index 0000000000..ec964ad248 --- /dev/null +++ b/relnotes/v2.22.1.md @@ -0,0 +1,5 @@ +### Bug fixes + +* [#1145](https://github.com/rubocop/rubocop-rails/issues/1145): Fix a false positive for `Rails/DuplicateAssociation` when using duplicate `belongs_to` associations of same class without other arguments. ([@koic][]) + +[@koic]: https://github.com/koic diff --git a/relnotes/v2.22.2.md b/relnotes/v2.22.2.md new file mode 100644 index 0000000000..c222a9c07a --- /dev/null +++ b/relnotes/v2.22.2.md @@ -0,0 +1,12 @@ +### Bug fixes + +* [#1172](https://github.com/rubocop/rubocop-rails/issues/1172): Fix an error for `Rails/UnknownEnv` when using Rails 7.1. ([@koic][]) +* [#1173](https://github.com/rubocop/rubocop-rails/issues/1173): Fix an error for `Rails/RedundantActiveRecordAllMethod` cop when used with RuboCop 1.51 or lower. ([@koic][]) + +### Changes + +* [#1171](https://github.com/rubocop/rubocop-rails/pull/1171): Change `Rails/RedundantActiveRecordAllMethod` to ignore `delete_all` and `destroy_all` when receiver is an association. ([@masato-bkn][]) +* [#1178](https://github.com/rubocop/rubocop-rails/pull/1178): Require RuboCop AST 1.30.0+. ([@koic][]) + +[@koic]: https://github.com/koic +[@masato-bkn]: https://github.com/masato-bkn diff --git a/relnotes/v2.23.0.md b/relnotes/v2.23.0.md new file mode 100644 index 0000000000..24c676efe0 --- /dev/null +++ b/relnotes/v2.23.0.md @@ -0,0 +1,20 @@ +### New features + +* [#1183](https://github.com/rubocop/rubocop-rails/pull/1183): Support PostGIS adapter for PostgreSQL. ([@Dania02525][]) + +### Bug fixes + +* [#1206](https://github.com/rubocop/rubocop-rails/issues/1206): Fix an error for `Rails/WhereMissing` where join method is called without arguments. ([@fatkodima][]) +* [#1189](https://github.com/rubocop/rubocop-rails/issues/1189): Fix false negatives for `Rails/Pluck` when using safe navigation method calls. ([@koic][]) +* [#1204](https://github.com/rubocop/rubocop-rails/pull/1204): Make `Rails/ActiveSupportAliases`, `Rails/FindBy`, `Rails/FindById`, `Rails/Inquiry`, `Rails/Pick` `Rails/PluckId`, `Rails/PluckInWhere`, `Rails/WhereEquals`, `Rails/WhereExists`, and `Rails/WhereNot` cops aware of safe navigation operator. ([@koic][]) + +### Changes + +* [#1213](https://github.com/rubocop/rubocop-rails/issues/1213): Update `Rails/PluckInWhere` to check for `.ids` call. ([@fatkodima][]) +* [#1181](https://github.com/rubocop/rubocop-rails/pull/1181): Support `Nokogiri::HTML.parse` and `Nokogiri::HTML5.parse` on `Rails/ResponseParsedBody`. ([@r7kamura][]) +* [#1198](https://github.com/rubocop/rubocop-rails/issues/1198): Support `where.not` for `Rails/PluckInWhere`. ([@fatkodima][]) + +[@Dania02525]: https://github.com/Dania02525 +[@fatkodima]: https://github.com/fatkodima +[@koic]: https://github.com/koic +[@r7kamura]: https://github.com/r7kamura diff --git a/rubocop-rails.gemspec b/rubocop-rails.gemspec index 5467c0db16..2bccd94c71 100644 --- a/rubocop-rails.gemspec +++ b/rubocop-rails.gemspec @@ -36,4 +36,5 @@ Gem::Specification.new do |s| # introduced in rack 1.1 s.add_runtime_dependency 'rack', '>= 1.1' s.add_runtime_dependency 'rubocop', '>= 1.33.0', '< 2.0' + s.add_runtime_dependency 'rubocop-ast', '>= 1.30.0', '< 2.0' end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 5fa4f79213..8bb9c60751 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -260,6 +260,10 @@ dir = File.expand_path('../changelog', __dir__) + it 'will not have a directory' do + expect(Dir["#{dir}/*"].none? { |path| File.directory?(path) }).to be(true) + end + Dir["#{dir}/*.md"].each do |path| context "For #{path}" do let(:path) { path } diff --git a/spec/rubocop/cop/rails/active_support_aliases_spec.rb b/spec/rubocop/cop/rails/active_support_aliases_spec.rb index 6aec9ed046..1644e0808b 100644 --- a/spec/rubocop/cop/rails/active_support_aliases_spec.rb +++ b/spec/rubocop/cop/rails/active_support_aliases_spec.rb @@ -6,7 +6,7 @@ it 'registers as an offense and corrects' do expect_offense(<<~RUBY) 'some_string'.starts_with?('prefix') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `start_with?` instead of `starts_with?`. + ^^^^^^^^^^^^^^^^^^^^^^ Use `start_with?` instead of `starts_with?`. RUBY expect_correction(<<~RUBY) @@ -15,6 +15,19 @@ end end + describe '&.starts_with?' do + it 'registers as an offense and corrects' do + expect_offense(<<~RUBY) + 'some_string'&.starts_with?('prefix') + ^^^^^^^^^^^^^^^^^^^^^^ Use `start_with?` instead of `starts_with?`. + RUBY + + expect_correction(<<~RUBY) + 'some_string'&.start_with?('prefix') + RUBY + end + end + describe '#start_with?' do it 'is not registered as an offense' do expect_no_offenses("'some_string'.start_with?('prefix')") @@ -25,7 +38,7 @@ it 'registers as an offense and corrects' do expect_offense(<<~RUBY) 'some_string'.ends_with?('prefix') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `end_with?` instead of `ends_with?`. + ^^^^^^^^^^^^^^^^^^^^ Use `end_with?` instead of `ends_with?`. RUBY expect_correction(<<~RUBY) @@ -34,6 +47,19 @@ end end + describe '&ends_with?' do + it 'registers as an offense and corrects' do + expect_offense(<<~RUBY) + 'some_string'&.ends_with?('prefix') + ^^^^^^^^^^^^^^^^^^^^ Use `end_with?` instead of `ends_with?`. + RUBY + + expect_correction(<<~RUBY) + 'some_string'&.end_with?('prefix') + RUBY + end + end + describe '#end_with?' do it 'is not registered as an offense' do expect_no_offenses("'some_string'.end_with?('prefix')") @@ -46,7 +72,18 @@ it 'registers as an offense and does not correct' do expect_offense(<<~RUBY) [1, 'a', 3].append('element') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `<<` instead of `append`. + ^^^^^^^^^^^^^^^^^ Use `<<` instead of `append`. + RUBY + + expect_no_corrections + end + end + + describe '&.append' do + it 'registers as an offense and does not correct' do + expect_offense(<<~RUBY) + [1, 'a', 3]&.append('element') + ^^^^^^^^^^^^^^^^^ Use `<<` instead of `append`. RUBY expect_no_corrections @@ -63,7 +100,7 @@ it 'registers as an offense and corrects' do expect_offense(<<~RUBY) [1, 'a', 3].prepend('element') - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `unshift` instead of `prepend`. + ^^^^^^^^^^^^^^^^^^ Use `unshift` instead of `prepend`. RUBY expect_correction(<<~RUBY) @@ -72,6 +109,19 @@ end end + describe '&.prepend' do + it 'registers as an offense and corrects' do + expect_offense(<<~RUBY) + [1, 'a', 3]&.prepend('element') + ^^^^^^^^^^^^^^^^^^ Use `unshift` instead of `prepend`. + RUBY + + expect_correction(<<~RUBY) + [1, 'a', 3]&.unshift('element') + RUBY + end + end + describe '#unshift' do it 'is not registered as an offense' do expect_no_offenses("[1, 'a', 3].unshift('element')") diff --git a/spec/rubocop/cop/rails/bulk_change_table_spec.rb b/spec/rubocop/cop/rails/bulk_change_table_spec.rb index 9420e32fe1..8beeec92c0 100644 --- a/spec/rubocop/cop/rails/bulk_change_table_spec.rb +++ b/spec/rubocop/cop/rails/bulk_change_table_spec.rb @@ -479,6 +479,34 @@ def change end end + context 'trilogy' do + context 'with top-level adapter configuration' do + let(:yaml) do + { + 'development' => { + 'adapter' => 'trilogy' + } + } + end + + it_behaves_like 'offense for mysql' + end + + context 'with nested adapter configuration' do + let(:yaml) do + { + 'development' => { + 'primary' => { + 'adapter' => 'trilogy' + } + } + } + end + + it_behaves_like 'offense for mysql' + end + end + context 'postgresql' do context 'with top-level adapter configuration' do let(:yaml) do @@ -519,6 +547,46 @@ def change end end + context 'postgis' do + context 'with top-level adapter configuration' do + let(:yaml) do + { + 'development' => { + 'adapter' => 'postgis' + } + } + end + + context 'with Rails 5.2', :rails52 do + it_behaves_like 'offense for postgresql' + end + + context 'with Rails 5.1', :rails51 do + it_behaves_like 'no offense for postgresql' + end + end + + context 'with nested adapter configuration' do + let(:yaml) do + { + 'development' => { + 'primary' => { + 'adapter' => 'postgis' + } + } + } + end + + context 'with Rails 5.2', :rails52 do + it_behaves_like 'offense for postgresql' + end + + context 'with Rails 5.1', :rails51 do + it_behaves_like 'no offense for postgresql' + end + end + end + context 'invalid (e.g. ERB)' do before do allow(YAML).to receive(:load_file).with('config/database.yml') do @@ -541,6 +609,12 @@ def change it_behaves_like 'offense for mysql' end + context 'trilogy' do + let(:database_url) { 'trilogy://localhost/my_database' } + + it_behaves_like 'offense for mysql' + end + context 'postgres' do let(:database_url) { 'postgres://localhost/my_database' } diff --git a/spec/rubocop/cop/rails/duplicate_association_spec.rb b/spec/rubocop/cop/rails/duplicate_association_spec.rb index 91c77a344b..3fbe151ce2 100644 --- a/spec/rubocop/cop/rails/duplicate_association_spec.rb +++ b/spec/rubocop/cop/rails/duplicate_association_spec.rb @@ -210,6 +210,61 @@ class Post < ApplicationRecord RUBY end + it 'registers offenses when using duplicate `has_*` associations of same class without other arguments' do + expect_offense(<<~RUBY) + class Post < ApplicationRecord + has_many :foos, class_name: 'Foo' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Foo'` is defined multiple times. Don't repeat associations. + has_many :bars, class_name: 'Foo' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Foo'` is defined multiple times. Don't repeat associations. + + has_one :baz, class_name: 'Bar' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Bar'` is defined multiple times. Don't repeat associations. + has_one :qux, class_name: 'Bar' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Association `class_name: 'Bar'` is defined multiple times. Don't repeat associations. + end + RUBY + + expect_correction(<<~RUBY) + class Post < ApplicationRecord + has_many :bars, class_name: 'Foo' + + has_one :qux, class_name: 'Bar' + end + RUBY + end + + it 'does not register an offenses when using duplicate `belongs_to` assocs of same class without other args' do + expect_no_offenses(<<~RUBY) + class Post < ApplicationRecord + belongs_to :foos, class_name: 'Foo' + belongs_to :bars, class_name: 'Foo' + end + RUBY + end + + it 'does not register an offense when using duplicate associations of same class with other arguments' do + expect_no_offenses(<<~RUBY) + class Post < ApplicationRecord + has_many :foos, if: condition, class_name: 'Foo' + has_many :bars, if: some_condition, class_name: 'Foo' + + has_one :baz, -> { condition }, class_name: 'Bar' + has_one :qux, -> { some_condition }, class_name: 'Bar' + + belongs_to :group, class_name: 'IndustryGroup', foreign_key: :industry_group_id + end + RUBY + end + + it 'does not register an offense when not using association method' do + expect_no_offenses(<<~RUBY) + class Post < ApplicationRecord + validates_presence_of :name + end + RUBY + end + it 'does not flag non-duplicate associations' do expect_no_offenses(<<-RUBY) class Post < ApplicationRecord diff --git a/spec/rubocop/cop/rails/env_local_spec.rb b/spec/rubocop/cop/rails/env_local_spec.rb new file mode 100644 index 0000000000..62cef8b1f3 --- /dev/null +++ b/spec/rubocop/cop/rails/env_local_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::EnvLocal, :config do + shared_examples 'non-local candidates' do + it 'registers no offenses for non-local `Rails.env._? || Rails.env._?`' do + expect_no_offenses(<<~RUBY) + Rails.env.development? || Rails.env.production? + Rails.env.test? || Rails.env.production? + Rails.env.production? || Rails.env.other? + RUBY + end + + it 'registers no offenses for single `Rails.env._?`' do + expect_no_offenses(<<~RUBY) + Rails.env.development? + Rails.env.test? + Rails.env.production? + Rails.env.other? + RUBY + end + end + + context 'In Rails >= 7.1', :rails71 do + it 'registers an offense for `Rails.env.development? || Rails.env.test?`' do + expect_offense(<<~RUBY) + Rails.env.development? || Rails.env.test? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.env.local?` instead. + Rails.env.test? || Rails.env.development? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Rails.env.local?` instead. + RUBY + + expect_correction(<<~RUBY) + Rails.env.local? + Rails.env.local? + RUBY + end + + it 'registers no offenses for `Rails.env.local?`' do + expect_no_offenses(<<~RUBY) + Rails.env.local? + RUBY + end + + include_examples 'non-local candidates' + end + + context 'In Rails < 7.1', :rails70 do + it 'registers no offenses for `Rails.env.development? || Rails.env.test?`' do + expect_no_offenses(<<~RUBY) + Rails.env.development? || Rails.env.test? + Rails.env.test? || Rails.env.development? + RUBY + end + + it 'registers no offenses for `Rails.env.local?`' do + expect_no_offenses(<<~RUBY) + Rails.env.local? + RUBY + end + + include_examples 'non-local candidates' + end +end diff --git a/spec/rubocop/cop/rails/find_by_id_spec.rb b/spec/rubocop/cop/rails/find_by_id_spec.rb index c04baaef7f..6e3212649b 100644 --- a/spec/rubocop/cop/rails/find_by_id_spec.rb +++ b/spec/rubocop/cop/rails/find_by_id_spec.rb @@ -12,6 +12,28 @@ RUBY end + it 'registers an offense and corrects when using `where(id: ...)&.take!`' do + expect_offense(<<~RUBY) + User.where(id: 1)&.take! + ^^^^^^^^^^^^^^^^^^^ Use `find(1)` instead of `where(id: 1)&.take!`. + RUBY + + expect_correction(<<~RUBY) + User.find(1) + RUBY + end + + it 'registers an offense and corrects when using `where(id: ...)&.take!` with safe navigation' do + expect_offense(<<~RUBY) + User&.where(id: 1)&.take! + ^^^^^^^^^^^^^^^^^^^ Use `find(1)` instead of `where(id: 1)&.take!`. + RUBY + + expect_correction(<<~RUBY) + User&.find(1) + RUBY + end + it 'registers an offense and corrects when using `find_by_id!`' do expect_offense(<<~RUBY) User.find_by_id!(1) @@ -23,6 +45,17 @@ RUBY end + it 'registers an offense and corrects when using `find_by_id!` with safe navigation' do + expect_offense(<<~RUBY) + User&.find_by_id!(1) + ^^^^^^^^^^^^^^ Use `find(1)` instead of `find_by_id!(1)`. + RUBY + + expect_correction(<<~RUBY) + User&.find(1) + RUBY + end + it 'registers an offense and corrects when using `find_by!(id: ...)`' do expect_offense(<<~RUBY) User.find_by!(id: 1) @@ -34,6 +67,17 @@ RUBY end + it 'registers an offense and corrects when using `find_by!(id: ...)` with safe navigation' do + expect_offense(<<~RUBY) + User&.find_by!(id: 1) + ^^^^^^^^^^^^^^^ Use `find(1)` instead of `find_by!(id: 1)`. + RUBY + + expect_correction(<<~RUBY) + User&.find(1) + RUBY + end + it 'does not register an offense when using `find`' do expect_no_offenses(<<~RUBY) User.find(1) diff --git a/spec/rubocop/cop/rails/find_by_spec.rb b/spec/rubocop/cop/rails/find_by_spec.rb index ec6f9ff25c..ecf64ffb3b 100644 --- a/spec/rubocop/cop/rails/find_by_spec.rb +++ b/spec/rubocop/cop/rails/find_by_spec.rb @@ -12,6 +12,28 @@ RUBY end + it 'registers and corrects an offense when using `&.take`' do + expect_offense(<<~RUBY) + User.where(id: x)&.take + ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where&.take`. + RUBY + + expect_correction(<<~RUBY) + User.find_by(id: x) + RUBY + end + + it 'registers and corrects an offense when using `&.take` with safe navigation' do + expect_offense(<<~RUBY) + User&.where(id: x)&.take + ^^^^^^^^^^^^^^^^^^ Use `find_by` instead of `where&.take`. + RUBY + + expect_correction(<<~RUBY) + User&.find_by(id: x) + RUBY + end + context 'when using safe navigation operator' do it 'registers an offense when using `#take`' do expect_offense(<<~RUBY) diff --git a/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb b/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb index 35053454a9..1890b34b55 100644 --- a/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb +++ b/spec/rubocop/cop/rails/has_many_or_has_one_dependent_spec.rb @@ -92,6 +92,16 @@ class Person < ApplicationRecord end RUBY end + + it 'does not register an offense when has_one called on explicit receiver' do + expect_no_offenses(<<~RUBY) + class Person < ApplicationRecord + with_options dependent: :destroy do |model| + model.has_one :foo + end + end + RUBY + end end end @@ -188,6 +198,16 @@ class Person < ApplicationRecord RUBY end + it 'does not register an offense when has_many called on explicit receiver' do + expect_no_offenses(<<~RUBY) + class Person < ApplicationRecord + with_options dependent: :destroy do |model| + model.has_many :foo + end + end + RUBY + end + it "doesn't register an offense for `with_options dependent: :destroy` and for using association extension" do expect_no_offenses(<<~RUBY) class Person < ApplicationRecord diff --git a/spec/rubocop/cop/rails/inquiry_spec.rb b/spec/rubocop/cop/rails/inquiry_spec.rb index 8ed0c736e7..be2caddfe9 100644 --- a/spec/rubocop/cop/rails/inquiry_spec.rb +++ b/spec/rubocop/cop/rails/inquiry_spec.rb @@ -8,6 +8,13 @@ RUBY end + it 'registers an offense when using `String&.inquiry`' do + expect_offense(<<~RUBY) + 'two'&.inquiry + ^^^^^^^ Prefer Ruby's comparison operators over Active Support's `inquiry`. + RUBY + end + it 'registers an offense when using `Array#inquiry`' do expect_offense(<<~RUBY) [foo, bar].inquiry diff --git a/spec/rubocop/cop/rails/not_null_column_spec.rb b/spec/rubocop/cop/rails/not_null_column_spec.rb index 7446e562ac..15d8a310c0 100644 --- a/spec/rubocop/cop/rails/not_null_column_spec.rb +++ b/spec/rubocop/cop/rails/not_null_column_spec.rb @@ -113,4 +113,41 @@ def change end end end + + context 'when database is MySQL' do + let(:cop_config) do + { 'Database' => 'mysql' } + end + + it 'does not register an offense when using `null: false` for `:text` type' do + expect_no_offenses(<<~RUBY) + def change + add_column :articles, :content, :text, null: false + end + RUBY + end + + it "does not register an offense when using `null: false` for `'text'` type" do + expect_no_offenses(<<~RUBY) + def change + add_column :articles, :content, 'text', null: false + end + RUBY + end + end + + context 'when database is PostgreSQL' do + let(:cop_config) do + { 'Database' => 'postgresql' } + end + + it 'registers an offense when using `null: false` for `:text` type' do + expect_offense(<<~RUBY) + def change + add_column :articles, :content, :text, null: false + ^^^^^^^^^^^ Do not add a NOT NULL column without a default value. + end + RUBY + end + end end diff --git a/spec/rubocop/cop/rails/output_spec.rb b/spec/rubocop/cop/rails/output_spec.rb index 4165d0ddeb..9dac2f84e0 100644 --- a/spec/rubocop/cop/rails/output_spec.rb +++ b/spec/rubocop/cop/rails/output_spec.rb @@ -140,6 +140,27 @@ RUBY end + it 'does not register an offense when the `p` method is called with block argument' do + expect_no_offenses(<<~RUBY) + # phlex-rails gem. + div do + p { 'Some text' } + end + RUBY + end + + it 'does not register an offense when io method is called with block argument' do + expect_no_offenses(<<~RUBY) + obj.write { do_somethig } + RUBY + end + + it 'does not register an offense when io method is called with numbered block argument' do + expect_no_offenses(<<~RUBY) + obj.write { do_something(_1) } + RUBY + end + it 'does not register an offense when a method is ' \ 'safe navigation called to a local variable with the same name as a print method' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/rails/pick_spec.rb b/spec/rubocop/cop/rails/pick_spec.rb index 3d17acde7e..c2bae0107e 100644 --- a/spec/rubocop/cop/rails/pick_spec.rb +++ b/spec/rubocop/cop/rails/pick_spec.rb @@ -3,7 +3,7 @@ RSpec.describe RuboCop::Cop::Rails::Pick, :config do context 'when using Rails 6.0 or newer', :rails60 do context 'with one argument' do - it 'registers an offense for `pluck(...).first' do + it 'registers an offense for `pluck(...).first`' do expect_offense(<<~RUBY) x.pluck(:a).first ^^^^^^^^^^^^^^^ Prefer `pick(:a)` over `pluck(:a).first`. @@ -13,10 +13,32 @@ x.pick(:a) RUBY end + + it 'registers an offense for `pluck(...)&.first`' do + expect_offense(<<~RUBY) + x.pluck(:a)&.first + ^^^^^^^^^^^^^^^^ Prefer `pick(:a)` over `pluck(:a)&.first`. + RUBY + + expect_correction(<<~RUBY) + x.pick(:a) + RUBY + end + + it 'registers an offense for `pluck(...)&.first` with safe navigation' do + expect_offense(<<~RUBY) + x&.pluck(:a)&.first + ^^^^^^^^^^^^^^^^ Prefer `pick(:a)` over `pluck(:a)&.first`. + RUBY + + expect_correction(<<~RUBY) + x&.pick(:a) + RUBY + end end context 'with multiple arguments' do - it 'registers an offense for `pluck(...).first' do + it 'registers an offense for `pluck(...).first`' do expect_offense(<<~RUBY) x.pluck(:a, :b).first ^^^^^^^^^^^^^^^^^^^ Prefer `pick(:a, :b)` over `pluck(:a, :b).first`. @@ -31,7 +53,7 @@ context 'when using Rails 5.2 or older', :rails52 do context 'with one argument' do - it 'does not register an offense for `pluck(...).first' do + it 'does not register an offense for `pluck(...).first`' do expect_no_offenses(<<~RUBY) x.pluck(:a).first RUBY @@ -39,7 +61,7 @@ end context 'with multiple arguments' do - it 'does not register an offense for `pluck(...).first' do + it 'does not register an offense for `pluck(...).first`' do expect_no_offenses(<<~RUBY) x.pluck(:a, :b).first RUBY diff --git a/spec/rubocop/cop/rails/pluck_id_spec.rb b/spec/rubocop/cop/rails/pluck_id_spec.rb index 58358f7c28..edeae8d234 100644 --- a/spec/rubocop/cop/rails/pluck_id_spec.rb +++ b/spec/rubocop/cop/rails/pluck_id_spec.rb @@ -12,6 +12,17 @@ RUBY end + it 'registers an offense and corrects when using `pluck(:id)` with safe navigation' do + expect_offense(<<~RUBY) + User&.pluck(:id) + ^^^^^^^^^^ Use `ids` instead of `pluck(:id)`. + RUBY + + expect_correction(<<~RUBY) + User&.ids + RUBY + end + it 'registers an offense and corrects when using `pluck(primary_key)`' do expect_offense(<<~RUBY) def self.user_ids diff --git a/spec/rubocop/cop/rails/pluck_in_where_spec.rb b/spec/rubocop/cop/rails/pluck_in_where_spec.rb index 030c73a00a..bc7b0daa06 100644 --- a/spec/rubocop/cop/rails/pluck_in_where_spec.rb +++ b/spec/rubocop/cop/rails/pluck_in_where_spec.rb @@ -17,6 +17,50 @@ RUBY end + it 'registers an offense and corrects when using `ids` in `where` for constant' do + expect_offense(<<~RUBY) + Post.where(user_id: User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where(user_id: User.active.select(:id)) + RUBY + end + + it 'registers an offense and corrects when using `pluck` in `where.not` for constant' do + expect_offense(<<~RUBY) + Post.where.not(user_id: User.active.pluck(:id)) + ^^^^^ Use `select` instead of `pluck` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where.not(user_id: User.active.select(:id)) + RUBY + end + + it 'registers an offense and corrects when using `ids` in `where.not` for constant' do + expect_offense(<<~RUBY) + Post.where.not(user_id: User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where.not(user_id: User.active.select(:id)) + RUBY + end + + it 'registers an offense and corrects when using `pluck` in `where` for constant with safe navigation' do + expect_offense(<<~RUBY) + Post&.where(user_id: User&.active&.pluck(:id)) + ^^^^^ Use `select` instead of `pluck` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post&.where(user_id: User&.active&.select(:id)) + RUBY + end + it 'registers an offense and corrects when using `pluck` in `rewhere` for constant' do expect_offense(<<~RUBY) Post.rewhere('user_id IN (?)', User.active.pluck(:id)) @@ -28,6 +72,17 @@ RUBY end + it 'registers an offense and corrects when using `ids` in `rewhere` for constant' do + expect_offense(<<~RUBY) + Post.rewhere('user_id IN (?)', User.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.rewhere('user_id IN (?)', User.active.select(:id)) + RUBY + end + it 'does not register an offense when using `select` in `where`' do expect_no_offenses(<<~RUBY) Post.where(user_id: User.active.select(:id)) @@ -40,11 +95,23 @@ RUBY end + it 'does not register an offense when using `ids` chained with other method calls in `where`' do + expect_no_offenses(<<~RUBY) + Post.where(user_id: User.ids.map(&:to_i)) + RUBY + end + it 'does not register an offense when using `select` in query methods other than `where`' do expect_no_offenses(<<~RUBY) Post.order(columns.pluck(:name)) RUBY end + + it 'does not register an offense when using `ids` in query methods other than `where`' do + expect_no_offenses(<<~RUBY) + Post.order(columns.ids) + RUBY + end end context 'EnforcedStyle: conservative' do @@ -58,6 +125,12 @@ Post.where(user_id: users.active.pluck(:id)) RUBY end + + it 'does not register an offense when using `ids` in `where`' do + expect_no_offenses(<<~RUBY) + Post.where(user_id: users.active.ids) + RUBY + end end end @@ -77,6 +150,17 @@ Post.where(user_id: users.active.select(:id)) RUBY end + + it 'registers and corrects an offense when using `ids` in `where`' do + expect_offense(<<~RUBY) + Post.where(user_id: users.active.ids) + ^^^ Use `select(:id)` instead of `ids` within `where` query method. + RUBY + + expect_correction(<<~RUBY) + Post.where(user_id: users.active.select(:id)) + RUBY + end end end end diff --git a/spec/rubocop/cop/rails/pluck_spec.rb b/spec/rubocop/cop/rails/pluck_spec.rb index 01ba5192d9..bf425980a1 100644 --- a/spec/rubocop/cop/rails/pluck_spec.rb +++ b/spec/rubocop/cop/rails/pluck_spec.rb @@ -16,6 +16,19 @@ end end + context "when safe navigation `#{method}` with symbol literal key can be replaced with `pluck`" do + it 'registers an offense' do + expect_offense(<<~RUBY, method: method) + x&.%{method} { |a| a[:foo] } + ^{method}^^^^^^^^^^^^^^^^ Prefer `pluck(:foo)` over `%{method} { |a| a[:foo] }`. + RUBY + + expect_correction(<<~RUBY) + x&.pluck(:foo) + RUBY + end + end + context "when `#{method}` with string literal key can be replaced with `pluck`" do it 'registers an offense' do expect_offense(<<~RUBY, method: method) diff --git a/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb b/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb index 1442a4bf83..9dd1581464 100644 --- a/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb +++ b/spec/rubocop/cop/rails/redundant_active_record_all_method_spec.rb @@ -8,6 +8,14 @@ and annotate any? + async_average + async_count + async_ids + async_maximum + async_minimum + async_pick + async_pluck + async_sum average calculate count @@ -77,6 +85,7 @@ preload readonly references + regroup reorder reselect rewhere @@ -98,6 +107,7 @@ unscope update_all where + with without ].to_set ) @@ -388,26 +398,6 @@ class User < ::ActiveRecord::Base RUBY end - context 'using `any?`' do - it 'does not register an offense when using `any?` with block' do - expect_no_offenses(<<~RUBY) - User.all.any? { |item| item.do_something } - RUBY - end - - it 'does not register an offense when using `any?` with numbered block' do - expect_no_offenses(<<~RUBY) - User.all.any? { _1.do_something } - RUBY - end - - it 'does not register an offense when using `any?` with symbol block' do - expect_no_offenses(<<~RUBY) - User.all.any?(&:do_something) - RUBY - end - end - described_class::POSSIBLE_ENUMERABLE_BLOCK_METHODS.each do |method| context "using `#{method}`" do it "does not register an offense when using `#{method}` with block" do @@ -431,6 +421,36 @@ class User < ::ActiveRecord::Base end end + described_class::SENSITIVE_METHODS_ON_ASSOCIATION.each do |method| + context "using `#{method}`" do + it "registers an offense when using `#{method}` and the receiver for `all` is a model" do + expect_offense(<<~RUBY) + User.all.#{method} + ^^^ Redundant `all` detected. + RUBY + end + + it "does not register an offense when using `#{method}` and the receiver for `all` is an association" do + expect_no_offenses(<<~RUBY) + user.articles.all.#{method} + RUBY + end + + it "does not register an offense when using `#{method}` and the receiver for `all` is a relation" do + expect_no_offenses(<<~RUBY) + users = User.all + users.all.#{method} + RUBY + end + + it "does not register an offense when using `#{method}` and `all` has no receiver" do + expect_no_offenses(<<~RUBY) + all.#{method} + RUBY + end + end + end + context 'with `AllowedReceivers` config' do let(:cop_config) do { 'AllowedReceivers' => %w[ActionMailer::Preview ActiveSupport::TimeZone] } diff --git a/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb b/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb index 0d8831a63d..e7d8d66e7c 100644 --- a/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb +++ b/spec/rubocop/cop/rails/redundant_presence_validation_on_belongs_to_spec.rb @@ -271,6 +271,20 @@ class Profile RUBY end + it 'does not register an offense with `if` option' do + expect_no_offenses(<<~RUBY) + belongs_to :user + validates :user, presence: true, if: -> { condition } + RUBY + end + + it 'does not register an offense with `unless` option' do + expect_no_offenses(<<~RUBY) + belongs_to :user + validates :user, presence: true, unless: -> { condition } + RUBY + end + it 'does not register an offense with strict validation' do expect_no_offenses(<<~RUBY) belongs_to :user diff --git a/spec/rubocop/cop/rails/response_parsed_body_spec.rb b/spec/rubocop/cop/rails/response_parsed_body_spec.rb index bf36455b6c..fac0c61c19 100644 --- a/spec/rubocop/cop/rails/response_parsed_body_spec.rb +++ b/spec/rubocop/cop/rails/response_parsed_body_spec.rb @@ -42,4 +42,38 @@ RUBY end end + + context 'when `Nokogiri::HTML.parse(response.body)` is used on Rails 7.0', :rails70 do + it 'registers no offense' do + expect_no_offenses(<<~RUBY) + Nokogiri::HTML.parse(response.body) + RUBY + end + end + + context 'when `Nokogiri::HTML.parse(response.body)` is used on Rails 7.1', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `Nokogiri::HTML.parse(response.body)`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end + + context 'when `Nokogiri::HTML5.parse(response.body)` is used on Rails 7.1', :rails71 do + it 'registers offense' do + expect_offense(<<~RUBY) + Nokogiri::HTML5.parse(response.body) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `response.parsed_body` to `Nokogiri::HTML5.parse(response.body)`. + RUBY + + expect_correction(<<~RUBY) + response.parsed_body + RUBY + end + end end diff --git a/spec/rubocop/cop/rails/save_bang_spec.rb b/spec/rubocop/cop/rails/save_bang_spec.rb index 2eba06497f..ce3a4a25eb 100644 --- a/spec/rubocop/cop/rails/save_bang_spec.rb +++ b/spec/rubocop/cop/rails/save_bang_spec.rb @@ -170,6 +170,19 @@ end end + it "when using #{method} wrapped within parenthesis with if" do + if update + expect_no_offenses(<<~RUBY, method: method) + if (object.#{method}); something; end + RUBY + else + expect_offense(<<~RUBY, method: method) + if (object.#{method}); something; end + ^{method} `#{method}` returns a model which is always truthy. + RUBY + end + end + it "when using #{method} with if with block" do if update expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/rails/unknown_env_spec.rb b/spec/rubocop/cop/rails/unknown_env_spec.rb index 8819e9fd1e..1965000916 100644 --- a/spec/rubocop/cop/rails/unknown_env_spec.rb +++ b/spec/rubocop/cop/rails/unknown_env_spec.rb @@ -24,6 +24,8 @@ ^^^^^^^^^^^^^ Unknown environment `developpment`. Did you mean `development`? Rails.env.something? ^^^^^^^^^^ Unknown environment `something`. + Rails.env.local? + ^^^^^^ Unknown environment `local`. RUBY end @@ -36,6 +38,9 @@ 'something' === Rails.env ^^^^^^^^^^^ Unknown environment `something`. + + 'local' === Rails.env + ^^^^^^^ Unknown environment `local`. RUBY end end @@ -49,6 +54,9 @@ ^^^^^^^^^^^^^ Unknown environment `developpment`. Rails.env.something? ^^^^^^^^^^ Unknown environment `something`. + + Rails.env.local? + ^^^^^^ Unknown environment `local`. RUBY end @@ -61,6 +69,9 @@ 'something' === Rails.env ^^^^^^^^^^^ Unknown environment `something`. + + 'local' === Rails.env + ^^^^^^^ Unknown environment `local`. RUBY end end @@ -72,4 +83,28 @@ Rails.env == 'production' RUBY end + + context 'when Rails 7.1 or newer', :rails71 do + it 'accepts local as an environment name' do + expect_no_offenses(<<~RUBY) + Rails.env.local? + Rails.env == 'local' + RUBY + end + + context 'when `Environments` is nil' do + let(:cop_config) do + { + 'Environments' => nil + } + end + + it 'accepts local as an environment name' do + expect_no_offenses(<<~RUBY) + Rails.env.local? + Rails.env == 'local' + RUBY + end + end + end end diff --git a/spec/rubocop/cop/rails/where_equals_spec.rb b/spec/rubocop/cop/rails/where_equals_spec.rb index 8bbea48233..69c718d85c 100644 --- a/spec/rubocop/cop/rails/where_equals_spec.rb +++ b/spec/rubocop/cop/rails/where_equals_spec.rb @@ -12,6 +12,17 @@ RUBY end + it 'registers an offense and corrects when using `=` and anonymous placeholder with safe navigation' do + expect_offense(<<~RUBY) + User&.where('name = ?', 'Gabe') + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(name: 'Gabe')` instead of manually constructing SQL. + RUBY + + expect_correction(<<~RUBY) + User&.where(name: 'Gabe') + RUBY + end + it 'registers an offense and corrects when using `=` and named placeholder' do expect_offense(<<~RUBY) User.where('name = :name', name: 'Gabe') @@ -79,6 +90,17 @@ RUBY end + it 'registers an offense and corrects when using `=` and anonymous placeholder with safe navigation' do + expect_offense(<<~RUBY) + User&.where(['name = ?', 'Gabe']) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where(name: 'Gabe')` instead of manually constructing SQL. + RUBY + + expect_correction(<<~RUBY) + User&.where(name: 'Gabe') + RUBY + end + it 'registers an offense and corrects when using `=` and named placeholder' do expect_offense(<<~RUBY) User.where(['name = :name', { name: 'Gabe' }]) diff --git a/spec/rubocop/cop/rails/where_exists_spec.rb b/spec/rubocop/cop/rails/where_exists_spec.rb index e98332c27f..b16a3e023d 100644 --- a/spec/rubocop/cop/rails/where_exists_spec.rb +++ b/spec/rubocop/cop/rails/where_exists_spec.rb @@ -15,6 +15,28 @@ RUBY end + it 'registers an offense and corrects when using `where(...)&.exists?` with hash argument' do + expect_offense(<<~RUBY) + User.where(name: 'john')&.exists? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `exists?(name: 'john')` over `where(name: 'john')&.exists?`. + RUBY + + expect_correction(<<~RUBY) + User.exists?(name: 'john') + RUBY + end + + it 'registers an offense and corrects when using `where(...)&.exists?` with hash argument with safe navigation' do + expect_offense(<<~RUBY) + User&.where(name: 'john')&.exists? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `exists?(name: 'john')` over `where(name: 'john')&.exists?`. + RUBY + + expect_correction(<<~RUBY) + User&.exists?(name: 'john') + RUBY + end + it 'registers an offense and corrects when using `where(...).exists?` with array argument' do expect_offense(<<~RUBY) User.where(['name = ?', 'john']).exists? @@ -87,6 +109,17 @@ RUBY end + it 'registers an offense and corrects when using `exists?` with a hash with safe navigation' do + expect_offense(<<~RUBY) + User&.exists?(name: 'john') + ^^^^^^^^^^^^^^^^^^^^^ Prefer `where(name: 'john')&.exists?` over `exists?(name: 'john')`. + RUBY + + expect_correction(<<~RUBY) + User&.where(name: 'john')&.exists? + RUBY + end + it 'registers an offense and corrects when using `exists?` with an array' do expect_offense(<<~RUBY) User.exists?(['name = ?', 'john']) diff --git a/spec/rubocop/cop/rails/where_missing_spec.rb b/spec/rubocop/cop/rails/where_missing_spec.rb index fde3476564..532d09104b 100644 --- a/spec/rubocop/cop/rails/where_missing_spec.rb +++ b/spec/rubocop/cop/rails/where_missing_spec.rb @@ -315,6 +315,12 @@ def test Foo.left_joins(bar: :foo).where(bars: { id: nil }) RUBY end + + it 'does not register an offense when using `left_joins` without arguments' do + expect_no_offenses(<<~RUBY) + Foo.left_joins(left_joins).where(bars: { id: nil }) + RUBY + end end context 'Rails 6.0', :rails60 do diff --git a/spec/rubocop/cop/rails/where_not_spec.rb b/spec/rubocop/cop/rails/where_not_spec.rb index 4ce3d88fdb..aa257d7246 100644 --- a/spec/rubocop/cop/rails/where_not_spec.rb +++ b/spec/rubocop/cop/rails/where_not_spec.rb @@ -12,6 +12,17 @@ RUBY end + it 'registers an offense and corrects when using `!=` and anonymous placeholder with safe navigation' do + expect_offense(<<~RUBY) + User&.where('name != ?', 'Gabe') + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where&.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`. + RUBY + + expect_correction(<<~RUBY) + User&.where&.not(name: 'Gabe') + RUBY + end + it 'registers an offense and corrects when using `!=` and named placeholder' do expect_offense(<<~RUBY) User.where('name != :name', name: 'Gabe') @@ -112,6 +123,17 @@ RUBY end + it 'registers an offense and corrects when using `!=` and anonymous placeholder with safe navigation' do + expect_offense(<<~RUBY) + User&.where(['name != ?', 'Gabe']) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `where&.not(name: 'Gabe')` instead of manually constructing negated SQL in `where`. + RUBY + + expect_correction(<<~RUBY) + User&.where&.not(name: 'Gabe') + RUBY + end + it 'registers an offense and corrects when using `!=` and named placeholder' do expect_offense(<<~RUBY) User.where(['name != :name', { name: 'Gabe' }]) diff --git a/spec/support/shared_contexts.rb b/spec/support/shared_contexts.rb index f6eddb00ee..bd659b6a0b 100644 --- a/spec/support/shared_contexts.rb +++ b/spec/support/shared_contexts.rb @@ -27,3 +27,7 @@ RSpec.shared_context 'with Rails 7.0', :rails70 do let(:rails_version) { 7.0 } end + +RSpec.shared_context 'with Rails 7.1', :rails71 do + let(:rails_version) { 7.1 } +end