Skip to content

Commit

Permalink
AO3-6691 Custom cops for house rules (#4763)
Browse files Browse the repository at this point in the history
* Helpers for custom cops

* Cop: large table migrations

* Cop: regex cucumber steps

* Cop: guard against `ts`

* Cop: guard against name_with_colon

* Cop: prevent default scope

* Link to i18n wiki

* Extract config to YAML

* Cop: default kwarg in t helper

* Update severities

* Cleanup last I18n.t -> t comments

* link to rubocop suppress docs
  • Loading branch information
brianjaustin authored Apr 2, 2024
1 parent 3e9c339 commit cedf5b6
Show file tree
Hide file tree
Showing 13 changed files with 525 additions and 0 deletions.
50 changes: 50 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require:
- rubocop-rails
- rubocop-rspec
- ./rubocop/rubocop

inherit_mode:
merge:
Expand All @@ -18,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

Expand Down Expand Up @@ -72,6 +77,51 @@ 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

Rails/DynamicFindBy:
AllowedMethods:
# Exception for Tag.find_by_name
Expand Down
51 changes: 51 additions & 0 deletions rubocop/cop/cucumber/regex_step_name.rb
Original file line number Diff line number Diff line change
@@ -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; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required"

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, severity: :refactor)
end
end
end
end
end
end
34 changes: 34 additions & 0 deletions rubocop/cop/i18n/default_translation.rb
Original file line number Diff line number Diff line change
@@ -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
31 changes: 31 additions & 0 deletions rubocop/cop/i18n/deprecated_helper.rb
Original file line number Diff line number Diff line change
@@ -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 `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 `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

def on_send(node)
add_offense(node)
end
end
end
end
end
47 changes: 47 additions & 0 deletions rubocop/cop/i18n/deprecated_translation_key.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# 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) + 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,
# 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 = deprecated_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

private

def deprecated_keys
cop_config["Rules"] || []
end
end
end
end
end
61 changes: 61 additions & 0 deletions rubocop/cop/migration/large_table_schema_update.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# 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`"

# @!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.to_s)

add_offense(change_node.loc.expression, message: format(MSG, table_name), severity: :warning)
end
end

private

def large_tables
cop_config["Tables"] || []
end

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
5 changes: 5 additions & 0 deletions rubocop/rubocop.rb
Original file line number Diff line number Diff line change
@@ -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 }
69 changes: 69 additions & 0 deletions spec/rubocop/cop/cucumber/regex_step_name_spec.rb
Original file line number Diff line number Diff line change
@@ -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; refer to https://github.com/otwcode/otwarchive/wiki/Reviewdog-and-RuboCop if regex is still required
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; 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
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; 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
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
Loading

0 comments on commit cedf5b6

Please sign in to comment.