diff --git a/.rubocop.yml b/.rubocop.yml index a8fb856c8..0d46853e9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -3,6 +3,7 @@ require: - rubocop-rspec - rubocop-performance - ./rubocop/custom_cops/required_inverse_of_relations.rb + - ./rubocop/custom_cops/class_template.rb AllCops: Exclude: @@ -16,6 +17,7 @@ AllCops: - 'node_modules/**/*' - 'config/**/*' - 'tmp/**/*' + - 'rubocop/**/*' TargetRubyVersion: 3 NewCops: enable @@ -60,3 +62,6 @@ CustomCops/RequiredInverseOfRelations: Include: # Only Rails model files - !ruby/regexp /models\// + +CustomCops/ClassTemplate: + Enabled: true diff --git a/.template/addons/custom_cops/template.rb b/.template/addons/custom_cops/template.rb index aec9816f8..356c330b9 100644 --- a/.template/addons/custom_cops/template.rb +++ b/.template/addons/custom_cops/template.rb @@ -3,3 +3,5 @@ require 'fileutils' copy_file 'rubocop/custom_cops/required_inverse_of_relations.rb', mode: :preserve +directory 'rubocop/custom_cops/class_template', mode: :preserve +copy_file 'rubocop/custom_cops/class_template.rb', mode: :preserve diff --git a/.template/spec/base/config/environments/development_spec.rb b/.template/spec/base/config/environments/development_spec.rb index 762528c84..71bb04196 100644 --- a/.template/spec/base/config/environments/development_spec.rb +++ b/.template/spec/base/config/environments/development_spec.rb @@ -8,7 +8,7 @@ end it 'configures the mailer asset host' do - expect(subject).to contain("config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST')") + expect(subject).to contain("config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST', 'localhost')") end it 'configures the mailer default url options' do @@ -24,8 +24,8 @@ def mailer_default_url_config <<~RUBY config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end diff --git a/.template/spec/base/config/environments/production_spec.rb b/.template/spec/base/config/environments/production_spec.rb index 2aed5269b..87b65a1cc 100644 --- a/.template/spec/base/config/environments/production_spec.rb +++ b/.template/spec/base/config/environments/production_spec.rb @@ -4,7 +4,7 @@ subject { file('config/environments/production.rb') } it 'configures the mailer asset host' do - expect(subject).to contain("config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST')") + expect(subject).to contain("config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST', 'localhost')") end it 'configures the mailer default url options' do @@ -24,8 +24,8 @@ def mailer_default_url_config <<~RUBY config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end @@ -33,13 +33,13 @@ def mailer_default_url_config def i18n_config <<~RUBY # eg: AVAILABLE_LOCALES = 'en,th' - config.i18n.available_locales = ENV.fetch('AVAILABLE_LOCALES').split(',') + config.i18n.available_locales = ENV.fetch('AVAILABLE_LOCALES', 'en').split(',') # eg: DEFAULT_LOCALE = 'en' - config.i18n.default_locale = ENV.fetch('DEFAULT_LOCALE') + config.i18n.default_locale = ENV.fetch('DEFAULT_LOCALE', 'en') # eg: FALLBACK_LOCALES = 'en,th' - config.i18n.fallbacks = ENV.fetch('FALLBACK_LOCALES').split(',') + config.i18n.fallbacks = ENV.fetch('FALLBACK_LOCALES', 'en').split(',') RUBY end end diff --git a/.template/spec/base/config/environments/test_spec.rb b/.template/spec/base/config/environments/test_spec.rb index 3791b1978..77e2d1bac 100644 --- a/.template/spec/base/config/environments/test_spec.rb +++ b/.template/spec/base/config/environments/test_spec.rb @@ -20,8 +20,8 @@ def mailer_default_url_config <<~RUBY config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end diff --git a/config/environments/development.rb b/config/environments/development.rb index 007a8aa54..bce773555 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -5,11 +5,11 @@ config.action_mailer.delivery_method = :letter_opener - config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST') + config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST', 'localhost') config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end diff --git a/config/environments/production.rb b/config/environments/production.rb index 5f8fc7a00..5f1a7e838 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -6,11 +6,11 @@ insert_into_file 'config/environments/production.rb', after: /config.action_mailer.perform_caching.+\n/ do <<~RUBY.indent(2) - config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST') + config.action_mailer.asset_host = ENV.fetch('MAILER_DEFAULT_HOST', 'localhost') config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end @@ -23,13 +23,13 @@ environment do <<~RUBY # eg: AVAILABLE_LOCALES = 'en,th' - config.i18n.available_locales = ENV.fetch('AVAILABLE_LOCALES').split(',') + config.i18n.available_locales = ENV.fetch('AVAILABLE_LOCALES', 'en').split(',') # eg: DEFAULT_LOCALE = 'en' - config.i18n.default_locale = ENV.fetch('DEFAULT_LOCALE') + config.i18n.default_locale = ENV.fetch('DEFAULT_LOCALE', 'en') # eg: FALLBACK_LOCALES = 'en,th' - config.i18n.fallbacks = ENV.fetch('FALLBACK_LOCALES').split(',') + config.i18n.fallbacks = ENV.fetch('FALLBACK_LOCALES', 'en').split(',') RUBY end diff --git a/config/environments/test.rb b/config/environments/test.rb index 38046c583..6c1f7f8ef 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -4,8 +4,8 @@ <<~RUBY.indent(2) config.action_mailer.default_url_options = { - host: ENV.fetch('MAILER_DEFAULT_HOST'), - port: ENV.fetch('MAILER_DEFAULT_PORT') + host: ENV.fetch('MAILER_DEFAULT_HOST', 'localhost'), + port: ENV.fetch('MAILER_DEFAULT_PORT', '3000') } RUBY end diff --git a/rubocop/custom_cops/class_template.rb b/rubocop/custom_cops/class_template.rb new file mode 100644 index 000000000..a7b4345b5 --- /dev/null +++ b/rubocop/custom_cops/class_template.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require_relative 'class_template/expression_category' + +module CustomCops + class ClassTemplate < RuboCop::Cop::Base + include CustomCops::ExpressionCategory + + EXPRESSION_TYPE_ORDER = { + extend: 0, + include: 1, + constant_assignment: 2, + attribute_macro: 3, + other_macro: 4, + self_class_block: 5, + public_class_method: 5, + initialization: 6, + public_instance_method: 7, + alias: 8, + alias_method: 8, + protected_instance_method: 9, + private_instance_method: 10 + }.freeze + + # Scan class body + def on_class(node) + expressions = top_level_expressions(node) + + state = { function_visibility: :public, expression_types: [] } + expressions.each_with_object(state) { |expression, acc| process_expression(expression, acc) } + + validate_expressions_order(state[:expression_types]) + end + + private + + def top_level_expressions(class_node) + return [] unless class_node.body + + # Multi-expression body + return class_node.body.children if class_node.body.type == :begin + + # Single-expression body + [class_node.body] + end + + def process_expression(expression, acc) + category = categorize(expression) + + if %i[protected private].include?(category) + acc[:function_visibility] = category + else + category = "#{acc[:function_visibility]}_#{category}".to_sym if category == :instance_method + acc[:expression_types] << { category: category, expression: expression } + end + end + + def validate_expressions_order(expressions) + expressions = expressions.filter { _1[:category] != :unknown } + + expressions.each_cons(2) do |first, second| + next unless EXPRESSION_TYPE_ORDER[first[:category]] > EXPRESSION_TYPE_ORDER[second[:category]] + + add_offense( + second[:expression], + message: error_message(second[:category], expressions) + ) + end + end + + # Find the correct spot for the out of order expression + # rubocop:disable Metrics/MethodLength + def error_message(out_of_order_expression, expressions) + expressions.filter { _1[:category] != out_of_order_expression } + .each_with_index do |expression, index| + error = if index.zero? + before_first_expression(expression, out_of_order_expression) + elsif index == expressions.size - 1 + after_last_expression(expression, out_of_order_expression) + else + previous_expression = expressions[index - 1] + in_between_expressions(previous_expression, expression, out_of_order_expression) + end + + return error if error + end + end + # rubocop:enable Metrics/MethodLength + + def before_first_expression(current_expression, out_of_order_expression) + unless EXPRESSION_TYPE_ORDER[out_of_order_expression] < EXPRESSION_TYPE_ORDER[current_expression[:category]] + return + end + + "#{out_of_order_expression} should come before `#{current_expression[:expression].source}`." + end + + def after_last_expression(current_expression, out_of_order_expression) + unless EXPRESSION_TYPE_ORDER[out_of_order_expression] > EXPRESSION_TYPE_ORDER[current_expression[:category]] + return + end + + "#{out_of_order_expression} should come after `#{current_expression[:expression].source}`." + end + + def in_between_expressions(previous_expression, current_expression, out_of_order_expression) + unless EXPRESSION_TYPE_ORDER[out_of_order_expression] < EXPRESSION_TYPE_ORDER[current_expression[:category]] && + EXPRESSION_TYPE_ORDER[out_of_order_expression] > EXPRESSION_TYPE_ORDER[previous_expression[:category]] + return + end + + "#{out_of_order_expression} should come after " \ + "`#{previous_expression[:expression].source}` " \ + "and before `#{current_expression[:expression].source}`." + end + end +end diff --git a/rubocop/custom_cops/class_template/expression_category.rb b/rubocop/custom_cops/class_template/expression_category.rb new file mode 100644 index 000000000..725712ef2 --- /dev/null +++ b/rubocop/custom_cops/class_template/expression_category.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'rubocop' + +module CustomCops + module ExpressionCategory + extend RuboCop::NodePattern::Macros + + ATTRIBUTE_MACROS = %i[attr_reader attr_writer attr_accessor].to_set + + CATEGORIES = %i[ + extend include constant_assignment attribute_macro alias_method protected private + other_macro self_class_block public_class_method initialization instance_method alias + ].freeze + + # extend SomeModule + def_node_matcher :extend?, <<~PATTERN + (send {nil? | self} :extend const) + PATTERN + + # include SomeModule + def_node_matcher :include?, <<~PATTERN + (send {nil? | self} :include const) + PATTERN + + # HELLO = 'world'.freeze + def_node_matcher :constant_assignment?, <<~PATTERN + (casgn nil? _ _) + PATTERN + + # attr_reader :foo, :bar + # attr_writer :baz + def_node_matcher :attribute_macro?, <<~PATTERN + (send {nil? | self} ATTRIBUTE_MACROS sym+) + PATTERN + + # validates :foo, presence: true + def_node_matcher :other_macro?, <<~PATTERN + (send {nil? | self} _ _+) + PATTERN + + # class << self + # def foo + # 'bar' + # end + # end + def_node_matcher :self_class_block?, <<~PATTERN + (sclass self _) + PATTERN + + # def self.foo + # "bar" + # end + def_node_matcher :public_class_method?, <<~PATTERN + (defs self _ args _) + PATTERN + + # def initialize(a, b) + # end + def_node_matcher :initialization?, <<~PATTERN + (def :initialize args _) + PATTERN + + # def method(a, b) + # end + def_node_matcher :instance_method?, <<~PATTERN + (def _ args _) + PATTERN + + # alias foo bar + # alias :foo :bar + def_node_matcher :alias?, <<~PATTERN + (alias sym sym) + PATTERN + + # alias_method :foo, :bar + # alias_method 'foo', 'bar' + def_node_matcher :alias_method?, <<~PATTERN + (send {nil? | self} :alias_method _ _) + PATTERN + + # protected + def_node_matcher :protected?, <<~PATTERN + (send {nil? | self} :protected) + PATTERN + + # private + def_node_matcher :private?, <<~PATTERN + (send {nil? | self} :private) + PATTERN + + # Categorize the expression of the class body + def categorize(expression) + CATEGORIES.find { |category| send(:"#{category}?", expression) } || :unknown + end + end +end diff --git a/rubocop/spec/class_template/expression_category_spec.rb b/rubocop/spec/class_template/expression_category_spec.rb new file mode 100644 index 000000000..6b08480b2 --- /dev/null +++ b/rubocop/spec/class_template/expression_category_spec.rb @@ -0,0 +1,230 @@ +# frozen_string_literal: true + +require 'rubocop' +require_relative '../../custom_cops/class_template/expression_category' + +describe CustomCops::ExpressionCategory, :config do + context 'given an include expression' do + it 'returns :include' do + expect(expression_category('include Foo')).to be(:include) + expect(expression_category('self.include Foo')).to be(:include) + end + end + + context 'given an extend expression' do + it 'returns :extend' do + expect(expression_category('extend Foo')).to be(:extend) + expect(expression_category('self.extend Foo')).to be(:extend) + end + end + + context 'given a constant assignment' do + it 'returns :constant_assignment' do + expect(expression_category("HELLO = 'world'.freeze")).to be(:constant_assignment) + expect(expression_category('HELLO = WORLD')).to be(:constant_assignment) + end + end + + context 'given an attribute macro' do + context 'given the attribute macro only has a single field' do + it 'returns :attribute_macro' do + expect(expression_category('attr_reader :hello')).to be(:attribute_macro) + expect(expression_category('self.attr_reader :hello')).to be(:attribute_macro) + + expect(expression_category('attr_writer :hello')).to be(:attribute_macro) + expect(expression_category('self.attr_writer :hello')).to be(:attribute_macro) + + expect(expression_category('attr_accessor :hello')).to be(:attribute_macro) + expect(expression_category('self.attr_accessor :hello')).to be(:attribute_macro) + end + end + + context 'given the attribute macro has multiple fields' do + it 'returns :attribute_macro' do + expect(expression_category('attr_reader :hello, :world')).to be(:attribute_macro) + expect(expression_category('self.attr_reader :hello, :world')).to be(:attribute_macro) + + expect(expression_category('attr_writer :hello, :world')).to be(:attribute_macro) + expect(expression_category('self.attr_writer :hello, :world')).to be(:attribute_macro) + + expect(expression_category('attr_accessor :hello, :world')).to be(:attribute_macro) + expect(expression_category('self.attr_accessor :hello, :world')).to be(:attribute_macro) + end + end + end + + context 'given a macro expression' do + it 'returns :other_macro' do + expect(expression_category('validates :name')).to be(:other_macro) + expect(expression_category("scope :in_advance, -> { where('started_at > ?', Time.current) }")).to be(:other_macro) + end + end + + context 'given a self class block' do + context 'given the block has no methods' do + it 'returns :self_class_block' do + code = <<~RUBY + class << self + end + RUBY + + expect(expression_category(code)).to be(:self_class_block) + end + end + + context 'given the block has ONE method' do + it 'returns :self_class_block' do + code = <<~RUBY + class << self + def foo + 'foo' + end + end + RUBY + + expect(expression_category(code)).to be(:self_class_block) + end + end + + context 'given the block has multiple methods' do + it 'returns :self_class_block' do + code = <<~RUBY + class << self + def foo + 'foo' + end + + def bar + 'bar' + end + end + RUBY + + expect(expression_category(code)).to be(:self_class_block) + end + end + end + + context 'given a public class method definition' do + context 'given the method has no arguments' do + it 'returns :public_class_method' do + code = <<~RUBY + def self.foo + "bar" + end + RUBY + + expect(expression_category(code)).to be(:public_class_method) + end + end + + context 'given the method has arguments' do + it 'returns :public_class_method' do + code = <<~RUBY + def self.foo(a, _b, c) + "bar" + end + RUBY + + expect(expression_category(code)).to be(:public_class_method) + end + end + end + + context 'given an initialize method definition' do + it 'returns :initialization' do + code = <<~RUBY + def initialize(a, _b, c) + @sum = a + c + end + RUBY + + expect(expression_category(code)).to be(:initialization) + end + end + + context 'given an instance method definition' do + context 'given the method has no arguments' do + it 'returns :instance_method' do + code = <<~RUBY + def foo + "bar" + end + RUBY + + expect(expression_category(code)).to be(:instance_method) + end + end + + context 'given the method has arguments' do + it 'returns :instance_method' do + code = <<~RUBY + def foo(a, _b, c) + "bar" + end + RUBY + + expect(expression_category(code)).to be(:instance_method) + end + end + end + + context 'given an alias expression' do + it 'returns :alias' do + code = <<~RUBY + alias foo bar + RUBY + + expect(expression_category(code)).to be(:alias) + end + end + + context 'given an alias_method expression' do + context 'given arguments are atoms' do + it 'returns :alias_method' do + code = <<~RUBY + alias_method :foo, :bar + RUBY + + expect(expression_category(code)).to be(:alias_method) + end + end + + context 'given arguments are strings' do + it 'returns :alias_method' do + code = <<~RUBY + alias_method 'foo', "bar_#{1 + 1}" + RUBY + + expect(expression_category(code)).to be(:alias_method) + end + end + end + + context 'given an protected statement' do + it 'returns :protected' do + code = <<~RUBY + protected + RUBY + + expect(expression_category(code)).to be(:protected) + end + end + + context 'given an private statement' do + it 'returns :private' do + code = <<~RUBY + private + RUBY + + expect(expression_category(code)).to be(:private) + end + end + + def expression_category(code) + dummy_class = Class.new { include CustomCops::ExpressionCategory } + source = RuboCop::ProcessedSource.new(code, RUBY_VERSION.to_f) + + dummy_class.new.categorize(source.ast) + end +end diff --git a/rubocop/spec/class_template_spec.rb b/rubocop/spec/class_template_spec.rb new file mode 100644 index 000000000..b0b1bcd0f --- /dev/null +++ b/rubocop/spec/class_template_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../custom_cops/class_template' + +RSpec.configure do |config| + config.include RuboCop::RSpec::ExpectOffense +end + +describe CustomCops::ClassTemplate, :config do + context 'given an out of order expression' do + it 'registers an offense' do + expect_offense(<<~RUBY) + class Test + include B + extend A + ^^^^^^^^ extend should come before `include B`. + end + RUBY + + expect_offense(<<~RUBY) + class Test + extend A + include B + + def initialize + {foo: "bar"} + end + + CONSTANT = 1 + ^^^^^^^^^^^^ constant_assignment should come after `include B` and before `def initialize[...] + end + RUBY + + expect_offense(<<~RUBY) + class Test + extend A + include B + + CONSTANT = 1 + + def initialize + {foo: "bar"} + end + + private + + def foo + "foo" + end + + def self.bar + ^^^^^^^^^^^^ public_class_method should come after `CONSTANT = 1` and before `def initialize[...] + "bar" + end + end + RUBY + end + end + + context 'given NO out of order expression' do + it 'registers NO offense' do + expect_no_offenses(<<~RUBY) + class Test + extend A + include B + + CONSTANT = 1 + + def initialize + {foo: "bar"} + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class Test + extend A + include B + + CONSTANT = 1 + + def self.bar + "bar" + end + + def initialize + {foo: "bar"} + end + + private + + def foo + "foo" + end + end + RUBY + end + end + + context 'given an unknown expression' do + it 'ignores it' do + expect_no_offenses(<<~RUBY) + class Test + extend A + include B + + def initialize + {foo: "bar"} + end + + # unknown expression + foo = "bar" + end + RUBY + end + end +end diff --git a/rubocop/spec/required_inverse_of_relations_spec.rb b/rubocop/spec/required_inverse_of_relations_spec.rb index d427e24c0..f3b56af48 100644 --- a/rubocop/spec/required_inverse_of_relations_spec.rb +++ b/rubocop/spec/required_inverse_of_relations_spec.rb @@ -2,7 +2,7 @@ require 'rubocop' require 'rubocop/rspec/support' -require_relative '../cops/required_inverse_of_relations' +require_relative '../custom_cops/required_inverse_of_relations' RSpec.configure do |config| config.include RuboCop::RSpec::ExpectOffense