diff --git a/changelog/new_fix_internalaffairssinglelinecomparison.md b/changelog/new_fix_internalaffairssinglelinecomparison.md new file mode 100644 index 00000000000..018a9cbcec5 --- /dev/null +++ b/changelog/new_fix_internalaffairssinglelinecomparison.md @@ -0,0 +1 @@ +* [#10170](https://github.com/rubocop/rubocop/pull/10170): Add new `InternalAffairs/SingleLineComparison` cop. ([@dvandersluis][]) diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 55ef735b0f1..19eadb9ee1e 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -16,6 +16,7 @@ require_relative 'rubocop/ast_aliases' require_relative 'rubocop/ext/regexp_node' require_relative 'rubocop/ext/regexp_parser' +require_relative 'rubocop/ext/range' require_relative 'rubocop/core_ext/string' require_relative 'rubocop/ext/processed_source' diff --git a/lib/rubocop/cop/internal_affairs.rb b/lib/rubocop/cop/internal_affairs.rb index 5ebcfdc92ff..eb006c6aa48 100644 --- a/lib/rubocop/cop/internal_affairs.rb +++ b/lib/rubocop/cop/internal_affairs.rb @@ -17,6 +17,7 @@ require_relative 'internal_affairs/redundant_location_argument' require_relative 'internal_affairs/redundant_message_argument' require_relative 'internal_affairs/redundant_method_dispatch_node' +require_relative 'internal_affairs/single_line_comparison' require_relative 'internal_affairs/style_detected_api_use' require_relative 'internal_affairs/undefined_config' require_relative 'internal_affairs/useless_message_assertion' diff --git a/lib/rubocop/cop/internal_affairs/single_line_comparison.rb b/lib/rubocop/cop/internal_affairs/single_line_comparison.rb new file mode 100644 index 00000000000..121aa275c03 --- /dev/null +++ b/lib/rubocop/cop/internal_affairs/single_line_comparison.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module InternalAffairs + # Enforces the use of `node.single_line?` instead of + # comparing `first_line` and `last_line` for equality. + # + # @example + # # bad + # node.loc.first_line == node.loc.last_line + # + # # bad + # node.loc.last_line == node.loc.first_line + # + # # bad + # node.loc.line == node.loc.last_line + # + # # bad + # node.loc.last_line == node.loc.line + # + # # bad + # node.first_line == node.last_line + # + # # good + # node.single_line? + # + class SingleLineComparison < Base + extend AutoCorrector + + MSG = 'Use `%s`.' + RESTRICT_ON_SEND = %i[==].freeze + + # @!method single_line_comparison(node) + def_node_matcher :single_line_comparison, <<~PATTERN + { + (send (send $_receiver {:line :first_line}) :== (send _receiver :last_line)) + (send (send $_receiver :last_line) :== (send _receiver {:line :first_line})) + } + PATTERN + + def on_send(node) + return unless (receiver = single_line_comparison(node)) + + preferred = "#{extract_receiver(receiver)}.single_line?" + + add_offense(node, message: format(MSG, preferred: preferred)) do |corrector| + corrector.replace(node, preferred) + end + end + + private + + def extract_receiver(node) + node = node.receiver if node.send_type? && %i[loc source_range].include?(node.method_name) + node.source + end + end + end + end +end diff --git a/lib/rubocop/ext/range.rb b/lib/rubocop/ext/range.rb new file mode 100644 index 00000000000..f00e9fedb18 --- /dev/null +++ b/lib/rubocop/ext/range.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module RuboCop + module Ext + # Extensions to Parser::Source::Range + module Range + # Adds `Range#single_line?` to parallel `Node#single_line?` + def single_line? + first_line == last_line + end + end + end +end + +Parser::Source::Range.include RuboCop::Ext::Range diff --git a/spec/rubocop/cop/internal_affairs/single_line_comparison_spec.rb b/spec/rubocop/cop/internal_affairs/single_line_comparison_spec.rb new file mode 100644 index 00000000000..cfc82a2f92f --- /dev/null +++ b/spec/rubocop/cop/internal_affairs/single_line_comparison_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::InternalAffairs::SingleLineComparison, :config do + it 'registers and corrects an offense when comparing `loc.first_line` with `loc.last_line`' do + expect_offense(<<~RUBY) + node.loc.first_line == node.loc.last_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `loc.last_line` with `loc.first_line`' do + expect_offense(<<~RUBY) + node.loc.last_line == node.loc.first_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `loc.line` with `loc.last_line`' do + expect_offense(<<~RUBY) + node.loc.line == node.loc.last_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `loc.last_line` with `loc.line`' do + expect_offense(<<~RUBY) + node.loc.last_line == node.loc.line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `source_range.first_line` with `source_range.last_line`' do + expect_offense(<<~RUBY) + node.source_range.first_line == node.source_range.last_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `source_range.last_line` with `source_range.first_line`' do + expect_offense(<<~RUBY) + node.source_range.last_line == node.source_range.first_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `first_line` with `last_line`' do + expect_offense(<<~RUBY) + node.first_line == node.last_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'registers and corrects an offense when comparing `last_line` with `first_line`' do + expect_offense(<<~RUBY) + node.last_line == node.first_line + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `node.single_line?`. + RUBY + + expect_correction(<<~RUBY) + node.single_line? + RUBY + end + + it 'does not register an offense when comparing the same line' do + expect_no_offenses(<<~RUBY) + node.loc.first_line == node.loc.line + RUBY + end + + it 'does not register an offense when the receivers are not a match' do + expect_no_offenses(<<~RUBY) + nodes.first.first_line == nodes.last.last_line + RUBY + end +end