Skip to content

Commit

Permalink
[Fix rubocop#495] Add new Rails/I18nLocaleAssignment cop
Browse files Browse the repository at this point in the history
Fixes rubocop#495

This PR adds new `Rails/I18nLocaleAssignment` cop.
It has a similar purpose to `Rails/TimeZoneAssignment` cop.
rubocop#458
  • Loading branch information
koic committed Jun 8, 2021
1 parent fe8c4ac commit 7f9c3a5
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* [#494](https://github.com/rubocop/rubocop-rails/pull/494): Add new `Rails/UnusedIgnoredColumns` cop. ([@pocke][])
* [#490](https://github.com/rubocop/rubocop-rails/issues/490): Make `Rails/HttpStatus` aware of `head` method. ([@koic][])
* [#483](https://github.com/rubocop/rubocop-rails/pull/483): Add new `Rails/EagerEvaluationLogMessage` cop. ([@aesthetikx][])
* [#495](https://github.com/rubocop/rubocop-rails/issues/495): Add new `Rails/I18nLocaleAssignment` cop. ([@koic][])

### Bug fixes

Expand Down
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,14 @@ Rails/HttpStatus:
- numeric
- symbolic

Rails/I18nLocaleAssignment:
Description: 'Prefer the usage of `I18n.with_locale` instead of manually updating `I18n.locale` value.'
Enabled: 'pending'
VersionAdded: '2.11'
Include:
- spec/**/*.rb
- test/**/*.rb

Rails/IgnoredSkipActionFilterOption:
Description: 'Checks that `if` and `only` (or `except`) are not used together as options of `skip_*` action filter.'
Reference: 'https://api.rubyonrails.org/classes/AbstractController/Callbacks/ClassMethods.html#method-i-_normalize_callback_options'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
* xref:cops_rails.adoc#railshelperinstancevariable[Rails/HelperInstanceVariable]
* xref:cops_rails.adoc#railshttppositionalarguments[Rails/HttpPositionalArguments]
* xref:cops_rails.adoc#railshttpstatus[Rails/HttpStatus]
* xref:cops_rails.adoc#railsi18nlocaleassignment[Rails/I18nLocaleAssignment]
* xref:cops_rails.adoc#railsignoredskipactionfilteroption[Rails/IgnoredSkipActionFilterOption]
* xref:cops_rails.adoc#railsindexby[Rails/IndexBy]
* xref:cops_rails.adoc#railsindexwith[Rails/IndexWith]
Expand Down
41 changes: 41 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,47 @@ head 200
| `numeric`, `symbolic`
|===

== Rails/I18nLocaleAssignment

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 2.11
| -
|===

This cop checks for the use of `I18n.locale=` method.

The `locale` attribute persists for the rest of the Ruby runtime, potentially causing
unexpected behavior at a later time.
Using `I18n.with_locale` ensures the code passed in the block is the only place `I18n.locale` is affected.
It eliminates the possibility of a `locale` sticking around longer than intended.

=== Examples

[source,ruby]
----
# bad
I18n.locale = :fr
# good
I18n.with_locale(:fr) do
end
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| Include
| `spec/**/*.rb`, `test/**/*.rb`
| Array
|===

== Rails/IgnoredSkipActionFilterOption

|===
Expand Down
37 changes: 37 additions & 0 deletions lib/rubocop/cop/rails/i18n_locale_assignment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop checks for the use of `I18n.locale=` method.
#
# The `locale` attribute persists for the rest of the Ruby runtime, potentially causing
# unexpected behavior at a later time.
# Using `I18n.with_locale` ensures the code passed in the block is the only place `I18n.locale` is affected.
# It eliminates the possibility of a `locale` sticking around longer than intended.
#
# @example
# # bad
# I18n.locale = :fr
#
# # good
# I18n.with_locale(:fr) do
# end
#
class I18nLocaleAssignment < Base
MSG = 'Use `I18n.with_locale` with block instead of `I18n.locale=`.'
RESTRICT_ON_SEND = %i[locale=].freeze

def_node_matcher :i18n_locale_assignment?, <<~PATTERN
(send (const {nil? cbase} :I18n) :locale= ...)
PATTERN

def on_send(node)
return unless i18n_locale_assignment?(node)

add_offense(node)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
require_relative 'rails/helper_instance_variable'
require_relative 'rails/http_positional_arguments'
require_relative 'rails/http_status'
require_relative 'rails/i18n_locale_assignment'
require_relative 'rails/ignored_skip_action_filter_option'
require_relative 'rails/index_by'
require_relative 'rails/index_with'
Expand Down
25 changes: 25 additions & 0 deletions spec/rubocop/cop/rails/i18n_locale_assignment_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::I18nLocaleAssignment, :config do
it 'registers an offense for `I18n.locale=`' do
expect_offense(<<~RUBY)
I18n.locale = :fr
^^^^^^^^^^^^^^^^^ Use `I18n.with_locale` with block instead of `I18n.locale=`.
RUBY
end

it 'registers an offense for `::I18n.locale=`' do
expect_offense(<<~RUBY)
::I18n.locale = :fr
^^^^^^^^^^^^^^^^^^^ Use `I18n.with_locale` with block instead of `I18n.locale=`.
RUBY
end

it 'accepts `I18n.with_locale`' do
expect_no_offenses(<<~RUBY)
I18n.with_locale(:fr) do
do_something
end
RUBY
end
end

0 comments on commit 7f9c3a5

Please sign in to comment.