Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new InternalAffairs/SingleLineComparison cop #10170

Merged
merged 2 commits into from Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/new_fix_internalaffairssinglelinecomparison.md
@@ -0,0 +1 @@
* [#10170](https://github.com/rubocop/rubocop/pull/10170): Add new `InternalAffairs/SingleLineComparison` cop. ([@dvandersluis][])
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs.rb
Expand Up @@ -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'
Expand Down
61 changes: 61 additions & 0 deletions 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 `%<preferred>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
2 changes: 1 addition & 1 deletion lib/rubocop/cop/layout/multiline_assignment_layout.rb
Expand Up @@ -73,7 +73,7 @@ def check_assignment(node, rhs)
return if node.send_type? && node.loc.operator&.source != '='
return unless rhs
return unless supported_types.include?(rhs.type)
return if rhs.first_line == rhs.last_line
return if rhs.single_line?

check_by_enforced_style(node, rhs)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/if_unless_modifier.rb
Expand Up @@ -96,7 +96,7 @@ def too_long_single_line?(node)
return false unless max_line_length

range = node.source_range
return false unless range.first_line == range.last_line
return false unless range.single_line?
return false unless line_length_enabled_at_line?(range.first_line)

line = range.source_line
Expand Down
15 changes: 15 additions & 0 deletions 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
2 changes: 1 addition & 1 deletion lib/rubocop/formatter/clang_style_formatter.rb
Expand Up @@ -41,7 +41,7 @@ def valid_line?(offense)
def report_line(location)
source_line = location.source_line

if location.first_line == location.last_line
if location.single_line?
output.puts(source_line)
else
output.puts("#{source_line} #{yellow(ELLIPSES)}")
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/formatter/html_formatter.rb
Expand Up @@ -115,7 +115,7 @@ def source_after_highlight(offense)
end

def possible_ellipses(location)
location.first_line == location.last_line ? '' : " #{ELLIPSES}"
location.single_line? ? '' : " #{ELLIPSES}"
end

def escape(string)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/formatter/markdown_formatter.rb
Expand Up @@ -71,7 +71,7 @@ def write_code(offense)
end

def possible_ellipses(location)
location.first_line == location.last_line ? '' : ' ...'
location.single_line? ? '' : ' ...'
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/formatter/tap_formatter.rb
Expand Up @@ -29,7 +29,7 @@ def file_finished(file, offenses)
def report_line(location)
source_line = location.source_line

if location.first_line == location.last_line
if location.single_line?
output.puts("# #{source_line}")
else
output.puts("# #{source_line} #{yellow(ELLIPSES)}")
Expand Down
103 changes: 103 additions & 0 deletions 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