diff --git a/.rubocop_todo/gettext/static_identifier.yml b/.rubocop_todo/gettext/static_identifier.yml new file mode 100644 index 0000000000000000000000000000000000000000..c330ffe148222cbaa3254419d79238981e4da828 --- /dev/null +++ b/.rubocop_todo/gettext/static_identifier.yml @@ -0,0 +1,28 @@ +--- +Gettext/StaticIdentifier: + Details: grace period + Exclude: + - 'app/graphql/types/project_type.rb' + - 'app/models/integrations/apple_app_store.rb' + - 'app/models/integrations/confluence.rb' + - 'app/models/integrations/google_play.rb' + - 'app/services/import/fogbugz_service.rb' + - 'app/services/issuable_links/create_service.rb' + - 'app/services/issues/set_crm_contacts_service.rb' + - 'app/services/projects/create_from_template_service.rb' + - 'app/services/security/ci_configuration/base_create_service.rb' + - 'app/services/users/banned_user_base_service.rb' + - 'app/services/work_items/widgets/hierarchy_service/base_service.rb' + - 'ee/app/controllers/admin/licenses_controller.rb' + - 'ee/app/controllers/subscriptions/groups_controller.rb' + - 'ee/app/mailers/ee/emails/admin_notification.rb' + - 'ee/app/mailers/emails/namespace_storage_usage_mailer.rb' + - 'ee/app/models/ee/member.rb' + - 'ee/app/models/integrations/github.rb' + - 'ee/app/services/ee/projects/create_from_template_service.rb' + - 'ee/app/services/security/security_orchestration_policies/policy_configuration_validation_service.rb' + - 'ee/app/services/timebox/rollup_report_service.rb' + - 'ee/app/services/timebox_report_service.rb' + - 'ee/spec/controllers/groups/security/policies_controller_spec.rb' + - 'ee/spec/features/registrations/identity_verification_spec.rb' + - 'lib/gitlab/github_import/settings.rb' diff --git a/rubocop/cop/gettext/static_identifier.rb b/rubocop/cop/gettext/static_identifier.rb new file mode 100644 index 0000000000000000000000000000000000000000..9ca1c88f4b6b5fabca8c015a4dd804accfaf7100 --- /dev/null +++ b/rubocop/cop/gettext/static_identifier.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gettext + # Ensure that gettext identifiers are statically defined and not + # interpolated, formatted, or concatenated. + # + # @example + # + # # bad + # _('Hi #{name}') + # _('Hi %{name}' % { name: 'Luki' }) + # _(format('Hi %{name}', name: 'Luki')) + # + # # good + # _('Hi %{name}') % { name: 'Luki' } + # format(_('Hi %{name}', name: 'Luki')) + # + # # also good + # var = "Hi" + # _(var) + # _(some_method_call) + # _(CONST) + class StaticIdentifier < RuboCop::Cop::Base + MSG = 'Ensure to pass static strings to translation method `%{method_name}(...)`.' + + # Number of parameters to check for translation methods. + PARAMETERS_TO_CHECK = { + _: 1, + s_: 1, + N_: 1, + n_: 2 + }.freeze + + # RuboCop-specific optimization for `on_send`. + RESTRICT_ON_SEND = PARAMETERS_TO_CHECK.keys.freeze + + DENIED_METHOD_CALLS = %i[% format + concat].freeze + + def on_send(node) + method_name = node.method_name + arguments = node.arguments + + each_invalid_argument(method_name, arguments) do |argument_node| + message = format(MSG, method_name: method_name) + + add_offense(argument_node || node, message: message) + end + end + + private + + def each_invalid_argument(method_name, argument_nodes) + number = PARAMETERS_TO_CHECK.fetch(method_name) + + argument_nodes.take(number).each do |argument_node| + yield argument_node unless valid_argument?(argument_node) + end + end + + def valid_argument?(node) + return false unless node + + basic_type?(node) || multiline_string?(node) || allowed_method_call?(node) + end + + def basic_type?(node) + node.str_type? || node.lvar_type? || node.const_type? + end + + def multiline_string?(node) + node.dstr_type? && node.children.all?(&:str_type?) + end + + def allowed_method_call?(node) + return false unless node.send_type? + + !DENIED_METHOD_CALLS.include?(node.method_name) # rubocop:disable Rails/NegateInclude + end + end + end + end +end diff --git a/spec/rubocop/cop/gettext/static_identifier_spec.rb b/spec/rubocop/cop/gettext/static_identifier_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a0c15d8ad484f6d9b16042f75146e2f63114bd07 --- /dev/null +++ b/spec/rubocop/cop/gettext/static_identifier_spec.rb @@ -0,0 +1,174 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' +require 'rspec-parameterized' + +require_relative '../../../../rubocop/cop/gettext/static_identifier' + +RSpec.describe RuboCop::Cop::Gettext::StaticIdentifier, feature_category: :internationalization do + describe '#_()' do + it 'does not flag correct use' do + expect_no_offenses(<<~'RUBY') + _('Hello') + _('Hello #{name}') + + _('Hello %{name}') % { name: name } + format(_('Hello %{name}') % { name: name }) + + _('Hello' \ + 'Multiline') + _('Hello' \ + 'Multiline %{name}') % { name: name } + + var = "Hello" + _(var) + _(method_name) + list.each { |item| _(item) } + _(CONST) + RUBY + end + + it 'flags incorrect use' do + expect_offense(<<~'RUBY') + _('Hello' + ' concat') + ^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `_(...)`. + _('Hello'.concat(' concat')) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `_(...)`. + _("Hello #{name}") + ^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `_(...)`. + _('Hello %{name}' % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `_(...)`. + _(format('Hello %{name}') % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `_(...)`. + RUBY + end + end + + describe '#N_()' do + it 'does not flag correct use' do + expect_no_offenses(<<~'RUBY') + N_('Hello') + N_('Hello #{name}') + N_('Hello %{name}') % { name: name } + format(_('Hello %{name}') % { name: name }) + + N_('Hello' \ + 'Multiline') + + var = "Hello" + N_(var) + N_(method_name) + list.each { |item| N_(item) } + N_(CONST) + RUBY + end + + it 'flags incorrect use' do + expect_offense(<<~'RUBY') + N_('Hello' + ' concat') + ^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `N_(...)`. + N_("Hello #{name}") + ^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `N_(...)`. + N_('Hello %{name}' % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `N_(...)`. + N_('Hello' \ + ^^^^^^^^^ Ensure to pass static strings to translation method `N_(...)`. + 'Multiline %{name}' % { name: name }) + N_(format('Hello %{name}') % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `N_(...)`. + RUBY + end + end + + describe '#s_()' do + it 'does not flag correct use' do + expect_no_offenses(<<~'RUBY') + s_('World|Hello') + s_('World|Hello #{name}') + s_('World|Hello %{name}') % { name: name } + format(s_('World|Hello %{name}') % { name: name }) + + s_('World|Hello' \ + 'Multiline') + + var = "Hello" + s_(var) + s_(method_name) + list.each { |item| s_(item) } + s_(CONST) + RUBY + end + + it 'flags incorrect use' do + expect_offense(<<~'RUBY') + s_("World|Hello #{name}") + ^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `s_(...)`. + s_('World|Hello' + ' concat') + ^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `s_(...)`. + s_('World|Hello %{name}' % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `s_(...)`. + s_('World|Hello' \ + ^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `s_(...)`. + 'Multiline %{name}' % { name: name }) + s_(format('World|Hello %{name}') % { name: name }) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `s_(...)`. + RUBY + end + end + + describe '#n_()' do + it 'does not flag correct use' do + expect_no_offenses(<<~'RUBY') + n_('Hello', 'Hellos', 2) + n_('Hello', 'Hellos', count) + + n_('Hello' ' concat', 'Hellos', 2) + n_('Hello', 'Hello' 's', 2) + + n_('Hello %{name}', 'Hellos %{name}', 2) % { name: name } + format(n_('Hello %{name}', 'Hellos %{name}', count) % { name: name }) + + n_('Hello', 'Hellos' \ + 'Multiline', 2) + + n_('Hello' \ + 'Multiline', 'Hellos', 2) + + n_('Hello' \ + 'Multiline %{name}', 'Hellos %{name}', 2) % { name: name } + + var = "Hello" + n_(var, var, 1) + n_(method_name, method_name, count) + list.each { |item| n_(item, item, 2) } + n_(CONST, CONST, 2) + RUBY + end + + it 'flags incorrect use' do + expect_offense(<<~'RUBY') + n_('Hello' + ' concat', 'Hellos', 2) + ^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + n_('Hello', 'Hello' + 's', 2) + ^^^^^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + n_("Hello #{name}", "Hellos", 2) + ^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + n_('Hello %{name}' % { name: name }, 'Hellos', 2) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + n_('Hello' \ + ^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + 'Multiline %{name}' % { name: name }, 'Hellos %{name}', 2) + n_('Hello', format('Hellos %{name}') % { name: name }, count) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Ensure to pass static strings to translation method `n_(...)`. + RUBY + end + end + + describe 'edge cases' do + it 'does not flag' do + expect_no_offenses(<<~RUBY) + n_(s_('World|Hello'), s_('World|Hellos'), 2) + RUBY + end + end +end