From 26cc1cdc2d0e8a54a941d6fd0e4c3a02737a42ad Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sun, 27 Mar 2022 02:20:35 +0200 Subject: [PATCH] Add new `Rails/MailerPreviews` cop --- .gitignore | 2 + changelog/new_add_mailer_previews_cop.md | 1 + config/default.yml | 9 ++ lib/rubocop-rails.rb | 1 + ...ode_helper.rb => class_elements_helper.rb} | 14 ++- .../cop/mixin/parent_class_matchers.rb | 21 +++++ lib/rubocop/cop/mixin/parsing_helper.rb | 19 ++++ .../cop/rails/after_commit_override.rb | 2 +- .../cop/rails/duplicate_association.rb | 2 +- lib/rubocop/cop/rails/duplicate_scope.rb | 2 +- lib/rubocop/cop/rails/mailer_name.rb | 8 +- lib/rubocop/cop/rails/mailer_previews.rb | 78 +++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 4 +- lib/rubocop/rails/schema_loader.rb | 12 +-- .../rubocop/cop/rails/mailer_previews_spec.rb | 86 +++++++++++++++++++ 15 files changed, 238 insertions(+), 23 deletions(-) create mode 100644 changelog/new_add_mailer_previews_cop.md rename lib/rubocop/cop/mixin/{class_send_node_helper.rb => class_elements_helper.rb} (57%) create mode 100644 lib/rubocop/cop/mixin/parent_class_matchers.rb create mode 100644 lib/rubocop/cop/mixin/parsing_helper.rb create mode 100644 lib/rubocop/cop/rails/mailer_previews.rb create mode 100644 spec/rubocop/cop/rails/mailer_previews_spec.rb diff --git a/.gitignore b/.gitignore index b9dc26c851..b4b2bb3c15 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ rdoc doc .yardoc +tmp + # bundler .bundle Gemfile.lock diff --git a/changelog/new_add_mailer_previews_cop.md b/changelog/new_add_mailer_previews_cop.md new file mode 100644 index 0000000000..e592274490 --- /dev/null +++ b/changelog/new_add_mailer_previews_cop.md @@ -0,0 +1 @@ +* [#654](https://github.com/rubocop/rubocop-rails/issues/654): Add new `Rails/MailerPreviews` cop. ([@fatkodima][]) diff --git a/config/default.yml b/config/default.yml index c20ae180e0..2cdd99f2e2 100644 --- a/config/default.yml +++ b/config/default.yml @@ -663,6 +663,15 @@ Rails/MailerName: Include: - app/mailers/**/*.rb +Rails/MailerPreviews: + Description: 'Checks for existence of mailer previews.' + Enabled: pending + VersionAdded: '<>' + PreviewPaths: + - test/mailers/previews + Include: + - app/mailers/**/*.rb + Rails/MatchRoute: Description: >- Don't use `match` to define any routes unless there is a need to map multiple request types diff --git a/lib/rubocop-rails.rb b/lib/rubocop-rails.rb index 45c869c079..4e7de24e52 100644 --- a/lib/rubocop-rails.rb +++ b/lib/rubocop-rails.rb @@ -8,6 +8,7 @@ require_relative 'rubocop/rails' require_relative 'rubocop/rails/version' require_relative 'rubocop/rails/inject' +require_relative 'rubocop/cop/mixin/parsing_helper' require_relative 'rubocop/rails/schema_loader' require_relative 'rubocop/rails/schema_loader/schema' diff --git a/lib/rubocop/cop/mixin/class_send_node_helper.rb b/lib/rubocop/cop/mixin/class_elements_helper.rb similarity index 57% rename from lib/rubocop/cop/mixin/class_send_node_helper.rb rename to lib/rubocop/cop/mixin/class_elements_helper.rb index e68737d47f..6cfe9b3a9f 100644 --- a/lib/rubocop/cop/mixin/class_send_node_helper.rb +++ b/lib/rubocop/cop/mixin/class_elements_helper.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # A mixin to return all of the class send nodes. - module ClassSendNodeHelper + module ClassElementsHelper def class_send_nodes(class_node) class_def = class_node.body @@ -15,6 +15,18 @@ def class_send_nodes(class_node) class_def.each_child_node(:send) end end + + def class_def_nodes(class_node) + class_def = class_node.body + + return [] unless class_def + + if class_def.def_type? + [class_def] + else + class_def.each_child_node(:def) + end + end end end end diff --git a/lib/rubocop/cop/mixin/parent_class_matchers.rb b/lib/rubocop/cop/mixin/parent_class_matchers.rb new file mode 100644 index 0000000000..1685a6ad49 --- /dev/null +++ b/lib/rubocop/cop/mixin/parent_class_matchers.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # A mixin with predefined parent classes matchers + module ParentClassMatchers + extend NodePattern::Macros + + def_node_matcher :mailer_base_class?, <<~PATTERN + { + (const (const {nil? cbase} :ActionMailer) :Base) + (const {nil? cbase} :ApplicationMailer) + } + PATTERN + + def_node_matcher :mailer_preview_base_class?, <<~PATTERN + (const (const nil? :ActionMailer) :Preview) + PATTERN + end + end +end diff --git a/lib/rubocop/cop/mixin/parsing_helper.rb b/lib/rubocop/cop/mixin/parsing_helper.rb new file mode 100644 index 0000000000..738f498172 --- /dev/null +++ b/lib/rubocop/cop/mixin/parsing_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # A mixin with helpers related to source code parsing + module ParsingHelper + def parse(path, target_ruby_version) + klass_name = :"Ruby#{target_ruby_version.to_s.sub('.', '')}" + klass = ::Parser.const_get(klass_name) + parser = klass.new(RuboCop::AST::Builder.new) + + buffer = Parser::Source::Buffer.new(path, 1) + buffer.source = path.read + + parser.parse(buffer) + end + end + end +end diff --git a/lib/rubocop/cop/rails/after_commit_override.rb b/lib/rubocop/cop/rails/after_commit_override.rb index 7fb1fa0fa2..65aa4329ac 100644 --- a/lib/rubocop/cop/rails/after_commit_override.rb +++ b/lib/rubocop/cop/rails/after_commit_override.rb @@ -32,7 +32,7 @@ module Rails # after_update_commit :log_update_action # class AfterCommitOverride < Base - include ClassSendNodeHelper + include ClassElementsHelper MSG = 'There can only be one `after_*_commit :%s` hook defined for a model.' diff --git a/lib/rubocop/cop/rails/duplicate_association.rb b/lib/rubocop/cop/rails/duplicate_association.rb index 0373fa4530..b9a5149b98 100644 --- a/lib/rubocop/cop/rails/duplicate_association.rb +++ b/lib/rubocop/cop/rails/duplicate_association.rb @@ -32,7 +32,7 @@ module Rails class DuplicateAssociation < Base include RangeHelp extend AutoCorrector - include ClassSendNodeHelper + include ClassElementsHelper include ActiveRecordHelper MSG = "Association `%s` is defined multiple times. Don't repeat associations." diff --git a/lib/rubocop/cop/rails/duplicate_scope.rb b/lib/rubocop/cop/rails/duplicate_scope.rb index e5c2e0fdac..139957dbc0 100644 --- a/lib/rubocop/cop/rails/duplicate_scope.rb +++ b/lib/rubocop/cop/rails/duplicate_scope.rb @@ -17,7 +17,7 @@ module Rails # scope :hidden, -> { where(visible: false) } # class DuplicateScope < Base - include ClassSendNodeHelper + include ClassElementsHelper MSG = 'Multiple scopes share this same where clause.' diff --git a/lib/rubocop/cop/rails/mailer_name.rb b/lib/rubocop/cop/rails/mailer_name.rb index 168434b7df..753368485d 100644 --- a/lib/rubocop/cop/rails/mailer_name.rb +++ b/lib/rubocop/cop/rails/mailer_name.rb @@ -29,16 +29,10 @@ module Rails # class MailerName < Base extend AutoCorrector + include ParentClassMatchers MSG = 'Mailer should end with `Mailer` suffix.' - def_node_matcher :mailer_base_class?, <<~PATTERN - { - (const (const {nil? cbase} :ActionMailer) :Base) - (const {nil? cbase} :ApplicationMailer) - } - PATTERN - def_node_matcher :class_definition?, <<~PATTERN (class $(const _ !#mailer_suffix?) #mailer_base_class? ...) PATTERN diff --git a/lib/rubocop/cop/rails/mailer_previews.rb b/lib/rubocop/cop/rails/mailer_previews.rb new file mode 100644 index 0000000000..0e7885aa49 --- /dev/null +++ b/lib/rubocop/cop/rails/mailer_previews.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # Enforces the existence of mailer previews. + # + # @example + # # bad + # # app/mailer/user_mailer.rb + # class UserMailer < ApplicationMailer + # def welcome_email + # end + # end + # + # # No file exists in mailer previews directory. + # + # # good + # # app/mailer/user_mailer.rb + # class UserMailer < ApplicationMailer + # def welcome_email + # end + # end + # + # # test/mailers/previews/user_mailer_preview.rb + # class UserMailer < ActionMailer::Preview + # def welcome_email + # end + # end + # + class MailerPreviews < Base + include ParentClassMatchers + include ClassElementsHelper + include ParsingHelper + include VisibilityHelp + + MSG = 'Add a mailer preview for `%s`.' + + def on_class(node) + return unless mailer_base_class?(node.parent_class) + + actions(node).each do |action_node| + mailer_name = node.identifier.source + action_name = action_node.method_name + message = format(MSG, action_name: action_name) + + add_offense(action_node, message: message) unless preview_action_exists?(mailer_name, action_name) + end + end + + private + + def preview_action_exists?(mailer_name, action_name) + preview_files(mailer_name).any? do |preview_path| + if preview_path.exist? + node = parse(preview_path, target_ruby_version) + + node&.class_type? && + mailer_preview_base_class?(node.parent_class) && + actions(node).map(&:method_name).include?(action_name) + end + end + end + + def actions(class_node) + class_def_nodes(class_node).select { |def_node| node_visibility(def_node) == :public } + end + + def preview_files(class_name) + path = Pathname.pwd + Array(cop_config['PreviewPaths']).map do |preview_path| + path.join(preview_path, "#{class_name.underscore}_preview.rb") + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 0d92ff4ca5..08176474cd 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -2,11 +2,12 @@ require_relative 'mixin/active_record_helper' require_relative 'mixin/active_record_migrations_helper' -require_relative 'mixin/class_send_node_helper' +require_relative 'mixin/class_elements_helper' require_relative 'mixin/database_type_resolvable' require_relative 'mixin/enforce_superclass' require_relative 'mixin/index_method' require_relative 'mixin/migrations_helper' +require_relative 'mixin/parent_class_matchers' require_relative 'mixin/target_rails_version' require_relative 'rails/action_controller_flash_before_render' @@ -74,6 +75,7 @@ require_relative 'rails/lexically_scoped_action_filter' require_relative 'rails/link_to_blank' require_relative 'rails/mailer_name' +require_relative 'rails/mailer_previews' require_relative 'rails/match_route' require_relative 'rails/migration_class_name' require_relative 'rails/negate_include' diff --git a/lib/rubocop/rails/schema_loader.rb b/lib/rubocop/rails/schema_loader.rb index a3f457f2c2..a299b80448 100644 --- a/lib/rubocop/rails/schema_loader.rb +++ b/lib/rubocop/rails/schema_loader.rb @@ -5,6 +5,7 @@ module Rails # It loads db/schema.rb and return Schema object. # Cops refers database schema information with this module. module SchemaLoader + include Cop::ParsingHelper extend self # It parses `db/schema.rb` and return it. @@ -45,17 +46,6 @@ def load!(target_ruby_version) ast = parse(path, target_ruby_version) Schema.new(ast) if ast end - - def parse(path, target_ruby_version) - klass_name = :"Ruby#{target_ruby_version.to_s.sub('.', '')}" - klass = ::Parser.const_get(klass_name) - parser = klass.new(RuboCop::AST::Builder.new) - - buffer = Parser::Source::Buffer.new(path, 1) - buffer.source = path.read - - parser.parse(buffer) - end end end end diff --git a/spec/rubocop/cop/rails/mailer_previews_spec.rb b/spec/rubocop/cop/rails/mailer_previews_spec.rb new file mode 100644 index 0000000000..aeb8d4704c --- /dev/null +++ b/spec/rubocop/cop/rails/mailer_previews_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::MailerPreviews, :config do + include FileHelper + + let(:cop_config) { { 'PreviewPaths' => 'tmp/mailers/previews' } } + + after { FileUtils.rm_rf('tmp') } + + it 'registers an offense when there is no mailer preview file' do + expect_offense(<<~RUBY) + class UserMailer < ApplicationMailer + def welcome_email + ^^^^^^^^^^^^^^^^^ Add a mailer preview for `welcome_email`. + end + end + RUBY + end + + it 'registers an offense when there is no mailer preview method' do + create_preview(<<~RUBY) + class UserMailerPreview < ActionMailer::Preview + end + RUBY + + expect_offense(<<~RUBY) + class UserMailer < ApplicationMailer + def welcome_email + ^^^^^^^^^^^^^^^^^ Add a mailer preview for `welcome_email`. + end + end + RUBY + end + + it 'registers an offense when there is a private mailer preview method' do + create_preview(<<~RUBY) + class UserMailerPreview < ActionMailer::Preview + private + def welcome_email + end + end + RUBY + + expect_offense(<<~RUBY) + class UserMailer < ApplicationMailer + def welcome_email + ^^^^^^^^^^^^^^^^^ Add a mailer preview for `welcome_email`. + end + end + RUBY + end + + it 'registers an offense when there is no mailer preview in the file' do + create_preview + + expect_offense(<<~RUBY) + class UserMailer < ApplicationMailer + def welcome_email + ^^^^^^^^^^^^^^^^^ Add a mailer preview for `welcome_email`. + end + end + RUBY + end + + it 'does not register an offense when there is mailer preview' do + create_preview(<<~RUBY) + class UserMailerPreview < ActionMailer::Preview + def welcome_email + end + end + RUBY + + expect_no_offenses(<<~RUBY) + class UserMailer < ApplicationMailer + def welcome_email + end + end + RUBY + end + + private + + def create_preview(content = '') + create_file('tmp/mailers/previews/user_mailer_preview.rb', content) + end +end