From c0b9a107a0b2daf280ac7981c08bf0b2c83086cf Mon Sep 17 00:00:00 2001 From: fatkodima Date: Wed, 12 Aug 2020 15:05:58 +0300 Subject: [PATCH] Add new `Rails/I18nLazyLookup` cop --- CHANGELOG.md | 1 + config/default.yml | 8 ++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 55 +++++++++ lib/rubocop/cop/rails/i18n_lazy_lookup.rb | 95 +++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/i18n_lazy_lookup_spec.rb | 114 ++++++++++++++++++ 7 files changed, 275 insertions(+) create mode 100644 lib/rubocop/cop/rails/i18n_lazy_lookup.rb create mode 100644 spec/rubocop/cop/rails/i18n_lazy_lookup_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 052943ee6f..ed3d28d82f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#52](https://github.com/rubocop-hq/rubocop-rails/issues/52): Add new `Rails/AfterCommitOverride` cop. ([@fatkodima][]) * [#274](https://github.com/rubocop-hq/rubocop-rails/pull/274): Add new `Rails/WhereNot` cop. ([@fatkodima][]) +* [#326](https://github.com/rubocop-hq/rubocop-rails/pull/326): Add new `Rails/I18nLazyLookup` cop. ([@fatkodima][]) * [#311](https://github.com/rubocop-hq/rubocop-rails/issues/311): Make `Rails/HelperInstanceVariable` aware of memoization. ([@koic][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 171b5824ba..7ca2e0e087 100644 --- a/config/default.yml +++ b/config/default.yml @@ -312,6 +312,14 @@ Rails/HttpStatus: - numeric - symbolic +Rails/I18nLazyLookup: + Description: 'Checks for places where I18n "lazy" lookup can be used.' + StyleGuide: 'https://rails.rubystyle.guide/#lazy-lookup' + Enabled: pending + VersionAdded: '2.8' + Include: + - 'controllers/**/*' + 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' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 3a028a53d6..a0c251ce42 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -50,6 +50,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#railsi18nlazylookup[Rails/I18nLazyLookup] * xref:cops_rails.adoc#railsignoredskipactionfilteroption[Rails/IgnoredSkipActionFilterOption] * xref:cops_rails.adoc#railsindexby[Rails/IndexBy] * xref:cops_rails.adoc#railsindexwith[Rails/IndexWith] diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index a42ee1c1fe..959e5a15a1 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -1622,6 +1622,61 @@ redirect_to root_url, status: 301 | `numeric`, `symbolic` |=== +== Rails/I18nLazyLookup + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 2.8 +| - +|=== + +This cop checks for places where I18n "lazy" lookup can be used. + +=== Examples + +[source,ruby] +---- +# en.yml +# en: +# books: +# create: +# success: Book created! + +# bad +class BooksController < ApplicationController + def create + # ... + redirect_to books_url, notice: t('books.create.success') + end +end + +# good +class BooksController < ApplicationController + def create + # ... + redirect_to books_url, notice: t('.success') + end +end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| Include +| `controllers/**/*` +| Array +|=== + +=== References + +* https://rails.rubystyle.guide/#lazy-lookup + == Rails/IgnoredSkipActionFilterOption |=== diff --git a/lib/rubocop/cop/rails/i18n_lazy_lookup.rb b/lib/rubocop/cop/rails/i18n_lazy_lookup.rb new file mode 100644 index 0000000000..f7e0e460e9 --- /dev/null +++ b/lib/rubocop/cop/rails/i18n_lazy_lookup.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # This cop checks for places where I18n "lazy" lookup can be used. + # + # @example + # # en.yml + # # en: + # # books: + # # create: + # # success: Book created! + # + # # bad + # class BooksController < ApplicationController + # def create + # # ... + # redirect_to books_url, notice: t('books.create.success') + # end + # end + # + # # good + # class BooksController < ApplicationController + # def create + # # ... + # redirect_to books_url, notice: t('.success') + # end + # end + # + class I18nLazyLookup < Base + include VisibilityHelp + extend AutoCorrector + + MSG = 'Use "lazy" lookup for the texts used in controllers.' + + def_node_matcher :translate_call?, <<~PATTERN + (send nil? {:translate :t} ${sym_type? str_type?} ...) + PATTERN + + def on_send(node) + translate_call?(node) do |key_node| + controller, action = controller_and_action(node) + return unless controller && action + + key = key_node.value + scoped_key = get_scoped_key(key_node, controller, action) + return unless key == scoped_key + + add_offense(key_node) do |corrector| + unscoped_key = key_node.value.to_s.split('.').last + corrector.replace(key_node, "'.#{unscoped_key}'") + end + end + end + + private + + def controller_and_action(node) + controller = nil + action = nil + + def_node = node.each_ancestor(:def).first + action = def_node if def_node && node_visibility(def_node) == :public + + class_node = node.each_ancestor(:class).first + controller = class_node if class_node && class_node.identifier.source.end_with?('Controller') + + [controller, action] + end + + def get_scoped_key(key_node, controller, action) + path = controller_path(controller).tr('/', '.') + action_name = action.method_name + key = key_node.value.to_s.split('.').last + + "#{path}.#{action_name}.#{key}" + end + + def controller_path(controller) + module_name = controller.parent_module_name + controller_name = controller.identifier.source + + path = if module_name == 'Object' + controller_name + else + "#{module_name}::#{controller_name}" + end + + path.delete_suffix('Controller').underscore + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 50957f1c77..c543ee0eec 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -38,6 +38,7 @@ require_relative 'rails/helper_instance_variable' require_relative 'rails/http_positional_arguments' require_relative 'rails/http_status' +require_relative 'rails/i18n_lazy_lookup' require_relative 'rails/ignored_skip_action_filter_option' require_relative 'rails/index_by' require_relative 'rails/index_with' diff --git a/spec/rubocop/cop/rails/i18n_lazy_lookup_spec.rb b/spec/rubocop/cop/rails/i18n_lazy_lookup_spec.rb new file mode 100644 index 0000000000..1d07ec3f33 --- /dev/null +++ b/spec/rubocop/cop/rails/i18n_lazy_lookup_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::I18nLazyLookup do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when using translation helpers with the key scoped to controller and action' do + expect_offense(<<~RUBY) + class FooController + def action + t 'foo.action.key' + ^^^^^^^^^^^^^^^^ Use "lazy" lookup for the texts used in controllers. + translate 'foo.action.key' + ^^^^^^^^^^^^^^^^ Use "lazy" lookup for the texts used in controllers. + end + end + RUBY + + expect_correction(<<~RUBY) + class FooController + def action + t '.key' + translate '.key' + end + end + RUBY + end + + it 'does not register an offense when translation methods scoped to `I18n`' do + expect_no_offenses(<<~RUBY) + class FooController + def action + I18n.t 'foo.action.key' + I18n.translate 'foo.action.key' + end + end + RUBY + end + + it 'does not register an offense when not inside controller' do + expect_no_offenses(<<~RUBY) + class FooService + def do_something + t 'foo_service.do_something.key' + end + end + RUBY + end + + it 'does not register an offense when not inside controller action' do + expect_no_offenses(<<~RUBY) + class FooController + private + + def action + t 'foo.action.key' + end + end + RUBY + end + + it 'does not register an offense when translating key not scoped to controller and action' do + expect_no_offenses(<<~RUBY) + class FooController + def action + t 'one.two.key' + end + end + RUBY + end + + it 'does not register an offense when using "lazy" translation' do + expect_no_offenses(<<~RUBY) + class FooController + def action + t '.key' + end + end + RUBY + end + + it 'does not register an offense when translation key is not a string nor a symbol' do + expect_no_offenses(<<~RUBY) + class FooController + def action + t ['foo.action.key'] + t key + end + end + RUBY + end + + it 'handles scoped controllers' do + expect_offense(<<~RUBY) + module Bar + class FooController + def action + t 'bar.foo.action.key' + ^^^^^^^^^^^^^^^^^^^^ Use "lazy" lookup for the texts used in controllers. + end + end + end + RUBY + + expect_correction(<<~RUBY) + module Bar + class FooController + def action + t '.key' + end + end + end + RUBY + end +end