From 50eff016925765f6cefeea4550f6c6487d6f42e8 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 16:41:56 -0400 Subject: [PATCH 01/12] Helpers for custom cops --- .rubocop.yml | 1 + rubocop/rubocop.rb | 5 +++++ spec/rubocop_spec_helper.rb | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 rubocop/rubocop.rb create mode 100644 spec/rubocop_spec_helper.rb diff --git a/.rubocop.yml b/.rubocop.yml index d6fdc3dd71b..3039beed887 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,6 +6,7 @@ require: - rubocop-rails - rubocop-rspec + - ./rubocop/rubocop inherit_mode: merge: diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb new file mode 100644 index 00000000000..63c2b902fa8 --- /dev/null +++ b/rubocop/rubocop.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +# This comes from GitLab's Rubocop setup +# Auto-require all cops under `rubocop/cop/**/*.rb` +Dir[File.join(__dir__, "cop", "**", "*.rb")].each { |file| require file } diff --git a/spec/rubocop_spec_helper.rb b/spec/rubocop_spec_helper.rb new file mode 100644 index 00000000000..e5997cafba6 --- /dev/null +++ b/spec/rubocop_spec_helper.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require "spec_helper" +require "rubocop" +require "rubocop/rspec/cop_helper" +require "rubocop/rspec/expect_offense" +require "rubocop/rspec/host_environment_simulation_helper" +require "rubocop/rspec/shared_contexts" + +RSpec.configure do |config| + config.define_derived_metadata(file_path: %r{spec/rubocop}) do |metadata| + metadata[:type] = :rubocop + end + + config.include CopHelper, type: :rubocop + config.include HostEnvironmentSimulatorHelper, type: :rubocop + config.include RuboCop::RSpec::ExpectOffense, type: :rubocop + + config.include_context "config", type: :rubocop +end From e8e0b1b751ac4d3e7b212ff5795611035f5196a6 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 16:43:36 -0400 Subject: [PATCH 02/12] Cop: large table migrations --- .../migration/large_table_schema_update.rb | 99 +++++++++++++++++++ .../large_table_schema_update_spec.rb | 49 +++++++++ 2 files changed, 148 insertions(+) create mode 100644 rubocop/cop/migration/large_table_schema_update.rb create mode 100644 spec/rubocop/cop/migration/large_table_schema_update_spec.rb diff --git a/rubocop/cop/migration/large_table_schema_update.rb b/rubocop/cop/migration/large_table_schema_update.rb new file mode 100644 index 00000000000..36334579cf9 --- /dev/null +++ b/rubocop/cop/migration/large_table_schema_update.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Migration + # Checks that migrations updating the schema of large tables, + # as defined in the configuration, do so safely. As of writing, + # this involves utilizing the `uses_departure!` helper. + # + # @example + # # good + # class ExampleMigration < ActiveRecord::Migration[6.1] + # uses_departure! if Rails.env.staging? || Rails.env.production? + # + # add_column :users, :new_field, :integer, nullable: true + # end + # + # @example + # # bad + # class ExampleMigration < ActiveRecord::Migration[6.1] + # add_column :users, :new_field, :integer, nullable: true + # end + class LargeTableSchemaUpdate < RuboCop::Cop::Base + MSG = "Use departure to change the schema of large table `%s`" + + LARGE_TABLES = %i[ + abuse_reports + admin_activities + audits + bookmarks + comments + common_taggings + collection_items + collection_participants + collection_profiles + collections + creatorships + external_works + favorite_tags + feedbacks + filter_counts + filter_taggings + gifts + inbox_comments + invitations + kudos + log_items + mutes + preferences + profiles + prompts + pseuds + readings + set_taggings + serial_works + series + skins + stat_counters + subscriptions + tag_nominations + tag_set_associations + tags + taggings + users + works + ].freeze + + # @!method uses_departure?(node) + def_node_search :uses_departure?, <<~PATTERN + (send nil? :uses_departure!) + PATTERN + + # @!method schema_changes(node) + def_node_search :schema_changes, <<~PATTERN + $(send nil? _ (sym $_) ...) + PATTERN + + def on_class(node) + return unless in_migration?(node) + return if uses_departure?(node) + + schema_changes(node).each do |change_node, table_name| + next unless LARGE_TABLES.include?(table_name) + + add_offense(change_node.loc.expression, message: format(MSG, table_name)) + end + end + + private + + def in_migration?(node) + filename = node.location.expression.source_buffer.name + directory = File.dirname(filename) + directory.end_with?("db/migrate") + end + end + end + end +end diff --git a/spec/rubocop/cop/migration/large_table_schema_update_spec.rb b/spec/rubocop/cop/migration/large_table_schema_update_spec.rb new file mode 100644 index 00000000000..7afe41545a1 --- /dev/null +++ b/spec/rubocop/cop/migration/large_table_schema_update_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "rubocop_spec_helper" +require_relative "../../../../rubocop/cop/migration/large_table_schema_update" + +describe RuboCop::Cop::Migration::LargeTableSchemaUpdate do + context "when running on a migration file" do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + context "when modifying a large table" do + it "registers an offense if Departure is not used" do + expect_offense(<<~RUBY) + class FakeMigration < ActiveRecord::Migration[6.1] + def change + add_column :users, :foo, :boolean, default: false, null: false + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use departure to change the schema of large table `users` + end + end + RUBY + end + + it "does not register an offense if Departure is used" do + expect_no_offenses(<<~RUBY) + class FakeMigration < ActiveRecord::Migration[6.1] + uses_departure! if Rails.env.staging? || Rails.env.production? + + def change + add_column :users, :foo, :boolean, default: false, null: false + end + end + RUBY + end + end + + context "when modifying a small table" do + it "does not register an offense if Departure is not used" do + expect_no_offenses(<<~RUBY) + class FakeMigration < ActiveRecord::Migration[6.1] + def change + add_column :small, :foo, :boolean, default: false, null: false + end + end + RUBY + end + end + end +end From 05ceea4e2b5187e1cbbb1a745875676a2cc9fba8 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 19:44:44 -0400 Subject: [PATCH 03/12] Cop: regex cucumber steps --- rubocop/cop/cucumber/regex_step_name.rb | 51 ++++++++++++++ .../cop/cucumber/regex_step_name_spec.rb | 69 +++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 rubocop/cop/cucumber/regex_step_name.rb create mode 100644 spec/rubocop/cop/cucumber/regex_step_name_spec.rb diff --git a/rubocop/cop/cucumber/regex_step_name.rb b/rubocop/cop/cucumber/regex_step_name.rb new file mode 100644 index 00000000000..56998bf81c0 --- /dev/null +++ b/rubocop/cop/cucumber/regex_step_name.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Cucumber + # Checks that Cucumber step definitions use Cucumber expressions + # instead of Regex. Note: this may not always be possible, and this + # cop is not smart enough to detect those cases. + # + # @example + # # bad + # Given /foobar/ do + # ... + # end + # When /baz/ do + # ... + # end + # Then /oops(\w+)/ do |it| + # ... + # end + # + # @example + # # good + # Given "foobar(s)" do + # ... + # end + # When "baz" do + # ... + # end + # Then "oops{str}" do |it| + # ... + # end + class RegexStepName < RuboCop::Cop::Base + MSG = "Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names" + + RESTRICT_ON_SEND = %i[Given When Then].freeze + + # @!method regex_name(node) + def_node_matcher :regex_name, <<~PATTERN + (send nil? _ $(:regexp ...) ...) + PATTERN + + def on_send(node) + regex_name(node) do |regex_node| + add_offense(regex_node) + end + end + end + end + end +end diff --git a/spec/rubocop/cop/cucumber/regex_step_name_spec.rb b/spec/rubocop/cop/cucumber/regex_step_name_spec.rb new file mode 100644 index 00000000000..0ee79c8404f --- /dev/null +++ b/spec/rubocop/cop/cucumber/regex_step_name_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require "rubocop_spec_helper" +require_relative "../../../../rubocop/cop/cucumber/regex_step_name" + +describe RuboCop::Cop::Cucumber::RegexStepName do + context "with a `Given` block" do + it "records a violation when named via regex" do + expect_offense(<<~INVALID) + Given /^I have no users$/ do + ^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + User.delete_all + end + INVALID + end + + it "does not record a violation when named via a Cucumber expression" do + expect_no_offenses(<<~RUBY) + Given "I have no users" do + User.delete_all + end + RUBY + end + end + + context "with a `When` block" do + it "records a violation when named via regex" do + expect_offense(<<~INVALID) + When /^I visit the change username page for (.*)$/ do |login| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + user = User.find_by(login: login) + visit change_username_user_path(user) + end + INVALID + end + + it "does not record a violation when named via a Cucumber expression" do + expect_no_offenses(<<~RUBY) + When "I request a password reset for {string}" do |login| + step(%{I am on the login page}) + step(%{I follow "Reset password"}) + step(%{I fill in "Email address or user name" with "\#{login}"}) + step(%{I press "Reset Password"}) + end + RUBY + end + end + + context "with a `Then` block" do + it "records a violation when named via regex" do + expect_offense(<<~INVALID) + Then /^the user "([^"]*)" should be activated$/ do |login| + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + user = User.find_by(login: login) + expect(user).to be_active + end + INVALID + end + + it "does not record a violation when named via a Cucumber expression" do + expect_no_offenses(<<~RUBY) + Then "I should see the invitation id for the user {string}" do |login| + invitation_id = User.find_by(login: login).invitation.id + step %{I should see "Invitation: \#{invitation_id}"} + end + RUBY + end + end +end From 2b5e48566fc918345ab351fdb6f86bf77b4a71ee Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 20:52:49 -0400 Subject: [PATCH 04/12] Cop: guard against `ts` --- rubocop/cop/i18n/deprecated_helper.rb | 31 +++++++++++++++++ .../cop/i18n/deprecated_helper_spec.rb | 33 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 rubocop/cop/i18n/deprecated_helper.rb create mode 100644 spec/rubocop/cop/i18n/deprecated_helper_spec.rb diff --git a/rubocop/cop/i18n/deprecated_helper.rb b/rubocop/cop/i18n/deprecated_helper.rb new file mode 100644 index 00000000000..26c72e52e77 --- /dev/null +++ b/rubocop/cop/i18n/deprecated_helper.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module I18n + # Checks for uses of the deprecated helper function, `ts`. + # Strings passed to it cannot be translated, and all calls + # will need to be replaced with I18n.t to enable UI translations + # in the future. + # + # @example + # # bad + # ts("This will only be in English :(") + # ts("Hello %{name}", name: "world") + # + # @example + # # good + # t(".relative.path.to.translation") + # t(".greeting", name: "world") + class DeprecatedHelper < RuboCop::Cop::Base + MSG = "Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable" + + RESTRICT_ON_SEND = %i[ts].freeze + + def on_send(node) + add_offense(node) + end + end + end + end +end diff --git a/spec/rubocop/cop/i18n/deprecated_helper_spec.rb b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb new file mode 100644 index 00000000000..ee2f7fb5cba --- /dev/null +++ b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "rubocop_spec_helper" +require_relative "../../../../rubocop/cop/i18n/deprecated_helper" + +describe RuboCop::Cop::I18n::DeprecatedHelper do + it "registers an offense when `ts` is used" do + expect_offense(<<~INVALID) + ts("Some String") + ^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable + INVALID + end + + it "registers an offense when `ts` is used without parentheses" do + expect_offense(<<~INVALID) + ts "Another string" + ^^^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable + INVALID + end + + it "does not register an offense when `I18n.t` is used" do + expect_no_offenses(<<~RUBY) + I18n.t(".hello") + t(".goodbye") + RUBY + end + + it "does not register an offense for functions containing the letter `ts`" do + expect_no_offenses(<<~RUBY) + cats("meow") + RUBY + end +end From 475d42b3c641f46211bb520edf5ea37abb7c1295 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 21:13:08 -0400 Subject: [PATCH 05/12] Cop: guard against name_with_colon --- .../cop/i18n/deprecated_translation_key.rb | 45 +++++++++++++++++++ .../i18n/deprecated_translation_key_spec.rb | 24 ++++++++++ 2 files changed, 69 insertions(+) create mode 100644 rubocop/cop/i18n/deprecated_translation_key.rb create mode 100644 spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb diff --git a/rubocop/cop/i18n/deprecated_translation_key.rb b/rubocop/cop/i18n/deprecated_translation_key.rb new file mode 100644 index 00000000000..b29694ed162 --- /dev/null +++ b/rubocop/cop/i18n/deprecated_translation_key.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module I18n + # Checks for uses of translation keys that have been superseded + # by others or different methods of translation. + # + # @example + # # bad + # Category.human_attribute_name("name_with_colon", count: 1) + # t(".relative.path.name_with_colon", count: 2) + # + # @example + # # good + # Category.human_attribute_name("name", count: 1) + I18n.t("mailer.general.metadata_label_indicator") + # metadata_property(t(".relative.path.name", count: 2)) # views only + class DeprecatedTranslationKey < RuboCop::Cop::Base + DENIED_TRANSLATION_KEYS = { + name_with_colon: "Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon`" + }.freeze + + # Rubocop optimization: the check here is a little bit inefficient, + # and we know which functions/methods to check, so only run it on those. + RESTRICT_ON_SEND = %i[t translate human_attribute_name].freeze + + # @!method translation_keys(node) + def_node_search :translation_keys, <<~PATTERN + $(str $_) + PATTERN + + def on_send(node) + translation_keys(node).each do |key_node, key_text| + denied_key = DENIED_TRANSLATION_KEYS.find do |key, _| + key_text.include?(key.to_s) + end + next unless denied_key + + add_offense(key_node, message: denied_key[1]) + end + end + end + end + end +end diff --git a/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb b/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb new file mode 100644 index 00000000000..22ee1c01dd1 --- /dev/null +++ b/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require "rubocop_spec_helper" +require_relative "../../../../rubocop/cop/i18n/deprecated_translation_key" + +describe RuboCop::Cop::I18n::DeprecatedTranslationKey do + context "when using I18n.translate" do + it "records a violation for `name_with_colon`" do + expect_offense(<<~INVALID) + I18n.translate("name_with_colon") + ^^^^^^^^^^^^^^^^^ Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon` + INVALID + end + end + + context "when using human_attribute_name" do + it "records a violation for `name_with_colon`" do + expect_offense(<<~INVALID) + Fandom.human_attribute_name("name_with_colon", count: prompt.any_fandom ? 1 : tag_groups["Fandom"].count) + ^^^^^^^^^^^^^^^^^ Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon` + INVALID + end + end +end From 74e8056e3d3f68b3e709d9fceb72fd00cc6b5fd1 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 18 Sep 2023 21:51:41 -0400 Subject: [PATCH 06/12] Cop: prevent default scope --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 3039beed887..ff436c616ab 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -73,6 +73,9 @@ Metrics/ParameterLists: Metrics/PerceivedComplexity: Enabled: false +Rails/DefaultScope: + Enabled: true + Rails/DynamicFindBy: AllowedMethods: # Exception for Tag.find_by_name From a70ebc1c42b816cc7ca5f9103cf2399b3b8ba846 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Tue, 5 Mar 2024 20:52:59 -0500 Subject: [PATCH 07/12] Link to i18n wiki --- rubocop/cop/i18n/deprecated_helper.rb | 2 +- spec/rubocop/cop/i18n/deprecated_helper_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rubocop/cop/i18n/deprecated_helper.rb b/rubocop/cop/i18n/deprecated_helper.rb index 26c72e52e77..f12dc2e54b2 100644 --- a/rubocop/cop/i18n/deprecated_helper.rb +++ b/rubocop/cop/i18n/deprecated_helper.rb @@ -18,7 +18,7 @@ module I18n # t(".relative.path.to.translation") # t(".greeting", name: "world") class DeprecatedHelper < RuboCop::Cop::Base - MSG = "Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable" + MSG = "Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards." RESTRICT_ON_SEND = %i[ts].freeze diff --git a/spec/rubocop/cop/i18n/deprecated_helper_spec.rb b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb index ee2f7fb5cba..c91d188d566 100644 --- a/spec/rubocop/cop/i18n/deprecated_helper_spec.rb +++ b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb @@ -7,14 +7,14 @@ it "registers an offense when `ts` is used" do expect_offense(<<~INVALID) ts("Some String") - ^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable + ^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. INVALID end it "registers an offense when `ts` is used without parentheses" do expect_offense(<<~INVALID) ts "Another string" - ^^^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable + ^^^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. INVALID end From dd8a6d7d7e2bf64da5f6068452746debd7f49eea Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Thu, 28 Mar 2024 16:47:26 -0400 Subject: [PATCH 08/12] Extract config to YAML --- .rubocop.yml | 46 ++++++++++++++++++ rubocop/cop/i18n/deprecated_helper.rb | 2 +- .../cop/i18n/deprecated_translation_key.rb | 12 +++-- .../migration/large_table_schema_update.rb | 48 ++----------------- .../cop/i18n/deprecated_helper_spec.rb | 4 +- .../i18n/deprecated_translation_key_spec.rb | 10 ++++ .../large_table_schema_update_spec.rb | 4 ++ 7 files changed, 75 insertions(+), 51 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ff436c616ab..6ca41445023 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -19,6 +19,10 @@ AllCops: Bundler/OrderedGems: Enabled: false +I18n/DeprecatedTranslationKey: + Rules: + name_with_colon: "Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon`" + Layout/DotPosition: EnforcedStyle: leading @@ -73,6 +77,48 @@ Metrics/ParameterLists: Metrics/PerceivedComplexity: Enabled: false +Migration/LargeTableSchemaUpdate: + Tables: + - abuse_reports + - admin_activities + - audits + - bookmarks + - comments + - common_taggings + - collection_items + - collection_participants + - collection_profiles + - collections + - creatorships + - external_works + - favorite_tags + - feedbacks + - filter_counts + - filter_taggings + - gifts + - inbox_comments + - invitations + - kudos + - log_items + - mutes + - preferences + - profiles + - prompts + - pseuds + - readings + - set_taggings + - serial_works + - series + - skins + - stat_counters + - subscriptions + - tag_nominations + - tag_set_associations + - tags + - taggings + - users + - works + Rails/DefaultScope: Enabled: true diff --git a/rubocop/cop/i18n/deprecated_helper.rb b/rubocop/cop/i18n/deprecated_helper.rb index f12dc2e54b2..46465379a26 100644 --- a/rubocop/cop/i18n/deprecated_helper.rb +++ b/rubocop/cop/i18n/deprecated_helper.rb @@ -18,7 +18,7 @@ module I18n # t(".relative.path.to.translation") # t(".greeting", name: "world") class DeprecatedHelper < RuboCop::Cop::Base - MSG = "Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards." + MSG = "Prefer Rails built-in `t` helper over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards." RESTRICT_ON_SEND = %i[ts].freeze diff --git a/rubocop/cop/i18n/deprecated_translation_key.rb b/rubocop/cop/i18n/deprecated_translation_key.rb index b29694ed162..97f093486f6 100644 --- a/rubocop/cop/i18n/deprecated_translation_key.rb +++ b/rubocop/cop/i18n/deprecated_translation_key.rb @@ -16,10 +16,6 @@ module I18n # Category.human_attribute_name("name", count: 1) + I18n.t("mailer.general.metadata_label_indicator") # metadata_property(t(".relative.path.name", count: 2)) # views only class DeprecatedTranslationKey < RuboCop::Cop::Base - DENIED_TRANSLATION_KEYS = { - name_with_colon: "Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon`" - }.freeze - # Rubocop optimization: the check here is a little bit inefficient, # and we know which functions/methods to check, so only run it on those. RESTRICT_ON_SEND = %i[t translate human_attribute_name].freeze @@ -31,7 +27,7 @@ class DeprecatedTranslationKey < RuboCop::Cop::Base def on_send(node) translation_keys(node).each do |key_node, key_text| - denied_key = DENIED_TRANSLATION_KEYS.find do |key, _| + denied_key = deprecated_keys.find do |key, _| key_text.include?(key.to_s) end next unless denied_key @@ -39,6 +35,12 @@ def on_send(node) add_offense(key_node, message: denied_key[1]) end end + + private + + def deprecated_keys + cop_config["Rules"] || [] + end end end end diff --git a/rubocop/cop/migration/large_table_schema_update.rb b/rubocop/cop/migration/large_table_schema_update.rb index 36334579cf9..e18ccff50f4 100644 --- a/rubocop/cop/migration/large_table_schema_update.rb +++ b/rubocop/cop/migration/large_table_schema_update.rb @@ -23,48 +23,6 @@ module Migration class LargeTableSchemaUpdate < RuboCop::Cop::Base MSG = "Use departure to change the schema of large table `%s`" - LARGE_TABLES = %i[ - abuse_reports - admin_activities - audits - bookmarks - comments - common_taggings - collection_items - collection_participants - collection_profiles - collections - creatorships - external_works - favorite_tags - feedbacks - filter_counts - filter_taggings - gifts - inbox_comments - invitations - kudos - log_items - mutes - preferences - profiles - prompts - pseuds - readings - set_taggings - serial_works - series - skins - stat_counters - subscriptions - tag_nominations - tag_set_associations - tags - taggings - users - works - ].freeze - # @!method uses_departure?(node) def_node_search :uses_departure?, <<~PATTERN (send nil? :uses_departure!) @@ -80,7 +38,7 @@ def on_class(node) return if uses_departure?(node) schema_changes(node).each do |change_node, table_name| - next unless LARGE_TABLES.include?(table_name) + next unless large_tables.include?(table_name.to_s) add_offense(change_node.loc.expression, message: format(MSG, table_name)) end @@ -88,6 +46,10 @@ def on_class(node) private + def large_tables + cop_config["Tables"] || [] + end + def in_migration?(node) filename = node.location.expression.source_buffer.name directory = File.dirname(filename) diff --git a/spec/rubocop/cop/i18n/deprecated_helper_spec.rb b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb index c91d188d566..c8741f5807b 100644 --- a/spec/rubocop/cop/i18n/deprecated_helper_spec.rb +++ b/spec/rubocop/cop/i18n/deprecated_helper_spec.rb @@ -7,14 +7,14 @@ it "registers an offense when `ts` is used" do expect_offense(<<~INVALID) ts("Some String") - ^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. + ^^^^^^^^^^^^^^^^^ Prefer Rails built-in `t` helper over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. INVALID end it "registers an offense when `ts` is used without parentheses" do expect_offense(<<~INVALID) ts "Another string" - ^^^^^^^^^^^^^^^^^^^ Prefer Rails built-in `I18n.t` function over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. + ^^^^^^^^^^^^^^^^^^^ Prefer Rails built-in `t` helper over `ts`: the latter is not actually translatable. For more information, refer to https://github.com/otwcode/otwarchive/wiki/Internationalization-(i18n)-Standards. INVALID end diff --git a/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb b/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb index 22ee1c01dd1..bb73fe625ee 100644 --- a/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb +++ b/spec/rubocop/cop/i18n/deprecated_translation_key_spec.rb @@ -4,6 +4,16 @@ require_relative "../../../../rubocop/cop/i18n/deprecated_translation_key" describe RuboCop::Cop::I18n::DeprecatedTranslationKey do + subject(:cop) { described_class.new(config) } + + let(:config) do + RuboCop::Config.new("I18n/DeprecatedTranslationKey" => { + "Rules" => { + "name_with_colon" => "Prefer `name` with `mailer.general.metadata_label_indicator` over `name_with_colon`" + } + }) + end + context "when using I18n.translate" do it "records a violation for `name_with_colon`" do expect_offense(<<~INVALID) diff --git a/spec/rubocop/cop/migration/large_table_schema_update_spec.rb b/spec/rubocop/cop/migration/large_table_schema_update_spec.rb index 7afe41545a1..23d876f4faa 100644 --- a/spec/rubocop/cop/migration/large_table_schema_update_spec.rb +++ b/spec/rubocop/cop/migration/large_table_schema_update_spec.rb @@ -4,6 +4,10 @@ require_relative "../../../../rubocop/cop/migration/large_table_schema_update" describe RuboCop::Cop::Migration::LargeTableSchemaUpdate do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new("Migration/LargeTableSchemaUpdate" => { "Tables" => ["users"] }) } + context "when running on a migration file" do before do allow(cop).to receive(:in_migration?).and_return(true) From 1947b7d79923dfc645cf36ee8305221120ee1606 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Mar 2024 10:21:25 -0400 Subject: [PATCH 09/12] Cop: default kwarg in t helper --- rubocop/cop/i18n/default_translation.rb | 34 +++++++++++++++++ .../cop/i18n/default_translation_spec.rb | 37 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 rubocop/cop/i18n/default_translation.rb create mode 100644 spec/rubocop/cop/i18n/default_translation_spec.rb diff --git a/rubocop/cop/i18n/default_translation.rb b/rubocop/cop/i18n/default_translation.rb new file mode 100644 index 00000000000..c59839b7a9f --- /dev/null +++ b/rubocop/cop/i18n/default_translation.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module I18n + # Checks for uses of the `default` keyword argument within Rails translation helpers. + # + # @example + # # bad + # t(".translation_key", default: "English text") + # + # @example + # # good + # # assuming the translation is in a view, the key must be defined in config/locales/views/en.yml + # t(".translation_key") + class DefaultTranslation < RuboCop::Cop::Base + MSG = "Prefer setting a translation in the appropriate `en.yml` locale file instead of using `default`" + + RESTRICT_ON_SEND = %i[t translate].freeze + + # @!method default_kwarg(node) + def_node_search :default_kwarg, <<~PATTERN + (pair (sym :default) _) + PATTERN + + def on_send(node) + default_kwarg(node).each do |kwarg_node| + add_offense(kwarg_node) + end + end + end + end + end +end diff --git a/spec/rubocop/cop/i18n/default_translation_spec.rb b/spec/rubocop/cop/i18n/default_translation_spec.rb new file mode 100644 index 00000000000..6111fbbd558 --- /dev/null +++ b/spec/rubocop/cop/i18n/default_translation_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "rubocop_spec_helper" +require_relative "../../../../rubocop/cop/i18n/default_translation" + +describe RuboCop::Cop::I18n::DefaultTranslation do + context "when within the `t` helper" do + it "registers an offense if `default` is used alone" do + expect_offense(<<~INVALID) + t(".translation_key", default: "English text") + ^^^^^^^^^^^^^^^^^^^^^^^ Prefer setting a translation in the appropriate `en.yml` locale file instead of using `default` + INVALID + end + + it "registers an offense if `default` is used with other kwargs" do + expect_offense(<<~INVALID) + t(".translation_key", input: "hello", default: "I got %{input}") + ^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer setting a translation in the appropriate `en.yml` locale file instead of using `default` + INVALID + end + + it "does not register an offense if `default` is not used" do + expect_no_offenses(<<~RUBY) + t(".translation_key1") + t(".translation_key2", input: "hello") + RUBY + end + end + + context "when not within the `t` helper" do + it "does not register an offense if `default` is present in keyword args" do + expect_no_offenses(<<~RUBY) + my_method("arg", default: "something", kwarg1: "hi") + RUBY + end + end +end From a3f4320d11ef538af149b5d7aec57cecd1fc905e Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Mar 2024 10:51:02 -0400 Subject: [PATCH 10/12] Update severities --- rubocop/cop/cucumber/regex_step_name.rb | 2 +- rubocop/cop/migration/large_table_schema_update.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rubocop/cop/cucumber/regex_step_name.rb b/rubocop/cop/cucumber/regex_step_name.rb index 56998bf81c0..0feb88fe594 100644 --- a/rubocop/cop/cucumber/regex_step_name.rb +++ b/rubocop/cop/cucumber/regex_step_name.rb @@ -42,7 +42,7 @@ class RegexStepName < RuboCop::Cop::Base def on_send(node) regex_name(node) do |regex_node| - add_offense(regex_node) + add_offense(regex_node, severity: :refactor) end end end diff --git a/rubocop/cop/migration/large_table_schema_update.rb b/rubocop/cop/migration/large_table_schema_update.rb index e18ccff50f4..1061b9293fe 100644 --- a/rubocop/cop/migration/large_table_schema_update.rb +++ b/rubocop/cop/migration/large_table_schema_update.rb @@ -40,7 +40,7 @@ def on_class(node) schema_changes(node).each do |change_node, table_name| next unless large_tables.include?(table_name.to_s) - add_offense(change_node.loc.expression, message: format(MSG, table_name)) + add_offense(change_node.loc.expression, message: format(MSG, table_name), severity: :warning) end end From 1271c36c020f8127679edbbb66f45fe7b11030a8 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Mar 2024 11:14:42 -0400 Subject: [PATCH 11/12] Cleanup last I18n.t -> t comments --- rubocop/cop/i18n/deprecated_helper.rb | 2 +- rubocop/cop/i18n/deprecated_translation_key.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rubocop/cop/i18n/deprecated_helper.rb b/rubocop/cop/i18n/deprecated_helper.rb index 46465379a26..8f23b57b01d 100644 --- a/rubocop/cop/i18n/deprecated_helper.rb +++ b/rubocop/cop/i18n/deprecated_helper.rb @@ -5,7 +5,7 @@ module Cop module I18n # Checks for uses of the deprecated helper function, `ts`. # Strings passed to it cannot be translated, and all calls - # will need to be replaced with I18n.t to enable UI translations + # will need to be replaced with `t` to enable UI translations # in the future. # # @example diff --git a/rubocop/cop/i18n/deprecated_translation_key.rb b/rubocop/cop/i18n/deprecated_translation_key.rb index 97f093486f6..5f30b623fcd 100644 --- a/rubocop/cop/i18n/deprecated_translation_key.rb +++ b/rubocop/cop/i18n/deprecated_translation_key.rb @@ -13,7 +13,7 @@ module I18n # # @example # # good - # Category.human_attribute_name("name", count: 1) + I18n.t("mailer.general.metadata_label_indicator") + # Category.human_attribute_name("name", count: 1) + t("mailer.general.metadata_label_indicator") # metadata_property(t(".relative.path.name", count: 2)) # views only class DeprecatedTranslationKey < RuboCop::Cop::Base # Rubocop optimization: the check here is a little bit inefficient, From b9f669ed30a5dc2689d1e0e01a5ba7fbc99288aa Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Mon, 1 Apr 2024 20:49:46 -0400 Subject: [PATCH 12/12] link to rubocop suppress docs --- rubocop/cop/cucumber/regex_step_name.rb | 2 +- spec/rubocop/cop/cucumber/regex_step_name_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rubocop/cop/cucumber/regex_step_name.rb b/rubocop/cop/cucumber/regex_step_name.rb index 0feb88fe594..2c354ba267b 100644 --- a/rubocop/cop/cucumber/regex_step_name.rb +++ b/rubocop/cop/cucumber/regex_step_name.rb @@ -31,7 +31,7 @@ module Cucumber # ... # end class RegexStepName < RuboCop::Cop::Base - MSG = "Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names" + MSG = "Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required" RESTRICT_ON_SEND = %i[Given When Then].freeze diff --git a/spec/rubocop/cop/cucumber/regex_step_name_spec.rb b/spec/rubocop/cop/cucumber/regex_step_name_spec.rb index 0ee79c8404f..4e5d2e5e5bd 100644 --- a/spec/rubocop/cop/cucumber/regex_step_name_spec.rb +++ b/spec/rubocop/cop/cucumber/regex_step_name_spec.rb @@ -8,7 +8,7 @@ it "records a violation when named via regex" do expect_offense(<<~INVALID) Given /^I have no users$/ do - ^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + ^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required User.delete_all end INVALID @@ -27,7 +27,7 @@ it "records a violation when named via regex" do expect_offense(<<~INVALID) When /^I visit the change username page for (.*)$/ do |login| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required user = User.find_by(login: login) visit change_username_user_path(user) end @@ -50,7 +50,7 @@ it "records a violation when named via regex" do expect_offense(<<~INVALID) Then /^the user "([^"]*)" should be activated$/ do |login| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer Cucumber expressions (https://github.com/cucumber/cucumber-expressions) over regex for step names; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required user = User.find_by(login: login) expect(user).to be_active end