diff --git a/config/default.yml b/config/default.yml index 513affb..e1b421d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -90,3 +90,9 @@ Discourse/Plugins/UseRequireRelative: Discourse/Plugins/NoMonkeyPatching: Enabled: true + +Discourse/Services/GroupKeywords: + Enabled: true + +Discourse/Services/EmptyLinesAroundBlocks: + Enabled: true diff --git a/lib/rubocop-discourse.rb b/lib/rubocop-discourse.rb index f8ad552..6144196 100644 --- a/lib/rubocop-discourse.rb +++ b/lib/rubocop-discourse.rb @@ -3,6 +3,8 @@ require "rubocop" require "active_support" require "active_support/core_ext/string/inflections" +require "active_support/core_ext/object/blank" +require "active_support/core_ext/enumerable" require_relative "rubocop/discourse" require_relative "rubocop/discourse/inject" diff --git a/lib/rubocop/cop/discourse/services/empty_lines_around_blocks.rb b/lib/rubocop/cop/discourse/services/empty_lines_around_blocks.rb new file mode 100644 index 0000000..efc497f --- /dev/null +++ b/lib/rubocop/cop/discourse/services/empty_lines_around_blocks.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Discourse + module Services + # Put empty lines around multiline blocks. + # + # @example + # # bad + # model :my_model + # params do + # attribute :my_attribute + # validates :my_attribute, presence: true + # end + # policy :my_policy + # step :another_step + # + # # good + # model :my_model + # + # params do + # attribute :my_attribute + # validates :my_attribute, presence: true + # end + # + # policy :my_policy + # step :another_step + # + class EmptyLinesAroundBlocks < Base + extend AutoCorrector + + MSG = "Add empty lines around a step block." + + def_node_matcher :service_include?, <<~MATCHER + (class _ _ + { + (begin <(send nil? :include (const (const nil? :Service) :Base)) ...>) + <(send nil? :include (const (const nil? :Service) :Base)) ...> + } + ) + MATCHER + + def_node_matcher :top_level_block?, <<~MATCHER + (block (send nil? _) ...) + MATCHER + + def on_class(node) + return unless service_include?(node) + @service = true + end + + def on_block(node) + return unless service? + return unless top_level_block?(node) + return if node.single_line? + + if missing_empty_lines?(node) + add_offense(node, message: MSG) do |corrector| + if missing_empty_line_before?(node) + corrector.insert_before( + node.loc.expression.adjust( + begin_pos: -node.loc.expression.column + ), + "\n" + ) + end + if missing_empty_line_after?(node) + corrector.insert_after(node.loc.end, "\n") + end + end + end + end + + private + + def service? + @service + end + + def missing_empty_lines?(node) + missing_empty_line_before?(node) || missing_empty_line_after?(node) + end + + def missing_empty_line_before?(node) + processed_source[node.loc.expression.line - 2].present? && + node.left_siblings.present? + end + + def missing_empty_line_after?(node) + processed_source[node.loc.end.line].present? && + node.right_siblings.present? + end + end + end + end + end +end diff --git a/lib/rubocop/cop/discourse/services/group_keywords.rb b/lib/rubocop/cop/discourse/services/group_keywords.rb new file mode 100644 index 0000000..66fde42 --- /dev/null +++ b/lib/rubocop/cop/discourse/services/group_keywords.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Discourse + module Services + # Don’t put empty lines between keywords that are not multiline blocks. + # + # @example + # # bad + # model :my_model + # + # policy :my_policy + # + # try { step :might_raise } + # + # # good + # model :my_model + # policy :my_policy + # try { step :might_raise } + # + class GroupKeywords < Base + extend AutoCorrector + + MSG = "Group one-liner steps together by removing extra empty lines." + RESTRICT_ON_SEND = %i[step model policy].freeze + + def_node_matcher :service_include?, <<~MATCHER + (class _ _ + { + (begin <(send nil? :include (const (const nil? :Service) :Base)) ...>) + <(send nil? :include (const (const nil? :Service) :Base)) ...> + } + ) + MATCHER + + def on_class(node) + return unless service_include?(node) + @service = true + end + + def on_send(node) + return unless service? + return unless top_level?(node) + return unless extra_empty_line_after?(node) + + add_offense(node, message: MSG) do |corrector| + range = + node.loc.expression.end.with( + end_pos: node.right_sibling.loc.expression.begin_pos + ) + content = range.source.gsub(/^(\n)+/, "\n") + corrector.replace(range, content) + end + end + + private + + def service? + @service + end + + def extra_empty_line_after?(node) + processed_source[node.loc.expression.line].blank? && + ( + service_keyword?(node.right_sibling) || + single_line_block?(node.right_sibling) + ) + end + + def service_keyword?(node) + return unless node + node.send_type? && RESTRICT_ON_SEND.include?(node.method_name) + end + + def single_line_block?(node) + return unless node + node.block_type? && node.single_line? + end + + def top_level?(node) + while (!node.root?) + node = node.parent + return if %i[begin class block].exclude?(node.type) + end + true + end + end + end + end + end +end diff --git a/rubocop-discourse.gemspec b/rubocop-discourse.gemspec index a71251d..283380e 100644 --- a/rubocop-discourse.gemspec +++ b/rubocop-discourse.gemspec @@ -2,7 +2,7 @@ Gem::Specification.new do |s| s.name = "rubocop-discourse" - s.version = "3.8.6" + s.version = "3.9.0" s.summary = "Custom rubocop cops used by Discourse" s.authors = ["Discourse Team"] s.license = "MIT" diff --git a/spec/lib/rubocop/cop/discourse/services/empty_lines_around_blocks_spec.rb b/spec/lib/rubocop/cop/discourse/services/empty_lines_around_blocks_spec.rb new file mode 100644 index 0000000..5e16008 --- /dev/null +++ b/spec/lib/rubocop/cop/discourse/services/empty_lines_around_blocks_spec.rb @@ -0,0 +1,257 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe RuboCop::Cop::Discourse::Services::EmptyLinesAroundBlocks, + :config do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + context "when not in a service class" do + it "does nothing" do + expect_no_offenses(<<~RUBY) + class NotAService + step :first_step + params do + attribute :my_attribute + end + step :another_step + end + RUBY + end + end + + context "when in a service class" do + context "when a blank line is missing before a block" do + it "registers an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + step :first_step + params do + ^^^^^^^^^ Discourse/Services/EmptyLinesAroundBlocks: Add empty lines around a step block. + attribute :my_attribute + end + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + step :first_step + + params do + attribute :my_attribute + end + end + RUBY + end + end + + context "when a blank line is missing after a block" do + it "registers an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + params do + ^^^^^^^^^ Discourse/Services/EmptyLinesAroundBlocks: Add empty lines around a step block. + attribute :my_attribute + end + step :last_step + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + params do + attribute :my_attribute + end + + step :last_step + end + RUBY + end + end + + context "when a block is a one-liner" do + it "does not register an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + try { step :might_raise } + step :last_step + end + RUBY + end + end + + context "when blocks are nested" do + context "when the nested block is in the first position" do + context "when there is no empty line before" do + it "does not register an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + transaction do + try do + step :first_step + step :second_step + end + + step :third_step + end + end + RUBY + end + end + + context "when there is no empty line after" do + it "registers an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + transaction do + try do + ^^^^^^ Discourse/Services/EmptyLinesAroundBlocks: Add empty lines around a step block. + step :first_step + step :second_step + end + step :third_step + end + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + transaction do + try do + step :first_step + step :second_step + end + + step :third_step + end + end + RUBY + end + end + end + + context "when the nested block is in the last position" do + context "when there is no empty line after" do + it "does not register an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + transaction do + step :first_step + + try do + step :second_step + step :third_step + end + end + end + RUBY + end + end + + context "when there is no empty line before" do + it "registers an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + transaction do + step :first_step + try do + ^^^^^^ Discourse/Services/EmptyLinesAroundBlocks: Add empty lines around a step block. + step :second_step + step :third_step + end + end + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + transaction do + step :first_step + + try do + step :second_step + step :third_step + end + end + end + RUBY + end + end + end + end + + context "when blocks are used in methods" do + it "does not register an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + step :first_step + + def first_step(model:) + model.transaction do + do_something + end + end + end + RUBY + end + end + + context "with a full valid example" do + it "does not register an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + step :first_step + + params do + attribute :my_attribute + + validates :my_attributes, presence: true + end + + policy :allowed? + model :user + + transaction do + try do + step :save_user + step :log + end + + step :other_step + end + + step :last_step + end + RUBY + end + end + end +end diff --git a/spec/lib/rubocop/cop/discourse/services/group_keywords_spec.rb b/spec/lib/rubocop/cop/discourse/services/group_keywords_spec.rb new file mode 100644 index 0000000..97f6e02 --- /dev/null +++ b/spec/lib/rubocop/cop/discourse/services/group_keywords_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe RuboCop::Cop::Discourse::Services::GroupKeywords, :config do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + context "when not in a service class" do + it "does nothing" do + expect_no_offenses(<<~RUBY) + class NotAService + step :first_step + + step :another_step + end + RUBY + end + end + + context "when in a service class" do + context "when keywords are not grouped together" do + it "reports an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + model :user + ^^^^^^^^^^^ Discourse/Services/GroupKeywords: Group one-liner steps together by removing extra empty lines. + + policy :allowed? + step :save + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + model :user + policy :allowed? + step :save + end + RUBY + end + end + + context "when a one-liner block has an empty line before a keyword" do + it "reports an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + model :user + policy :allowed? + ^^^^^^^^^^^^^^^^ Discourse/Services/GroupKeywords: Group one-liner steps together by removing extra empty lines. + + try { step :save } + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + model :user + policy :allowed? + try { step :save } + end + RUBY + end + end + + context "when keywords with empty lines appear in a nested block" do + it "reports an offense" do + expect_offense(<<~RUBY) + class MyService + include Service::Base + + transaction do + step :save + ^^^^^^^^^^ Discourse/Services/GroupKeywords: Group one-liner steps together by removing extra empty lines. + + step :log + end + end + RUBY + + expect_correction(<<~RUBY) + class MyService + include Service::Base + + transaction do + step :save + step :log + end + end + RUBY + end + end + + context "when keywords are grouped together" do + it "does not report an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + model :user + policy :allowed? + + transaction do + step :save + step :log + end + end + RUBY + end + end + + context "when keywords are not at the top level" do + it "does not report an offense" do + expect_no_offenses(<<~RUBY) + class MyService + include Service::Base + + private + + def my_method + step(:save) + + step(:log) + end + end + RUBY + end + end + end +end