From 665bda0fbd9fdd36689498fecb1e94881dea0b95 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 26 Jan 2024 13:32:29 -0800 Subject: [PATCH] Vendor Presence cop --- Library/.rubocop.yml | 2 - Library/Homebrew/rubocops/all.rb | 1 + Library/Homebrew/rubocops/presence.rb | 153 ++++++ Library/Homebrew/test/rubocops/blank_spec.rb | 38 +- .../test/rubocops/compact_blank_spec.rb | 18 +- .../Homebrew/test/rubocops/presence_spec.rb | 446 ++++++++++++++++++ .../Homebrew/test/rubocops/present_spec.rb | 34 +- 7 files changed, 642 insertions(+), 50 deletions(-) create mode 100644 Library/Homebrew/rubocops/presence.rb create mode 100644 Library/Homebrew/test/rubocops/presence_spec.rb diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 77c0b1a0db..bb4e40a00a 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -220,8 +220,6 @@ Rails: - "Homebrew/rubocops/**/*" # These relate to ActiveSupport and not other parts of Rails. -Rails/Presence: - Enabled: true Rails/SafeNavigationWithBlank: Enabled: true diff --git a/Library/Homebrew/rubocops/all.rb b/Library/Homebrew/rubocops/all.rb index be71a3aca3..1555aa22f8 100644 --- a/Library/Homebrew/rubocops/all.rb +++ b/Library/Homebrew/rubocops/all.rb @@ -6,6 +6,7 @@ require_relative "blank" require_relative "compact_blank" require_relative "io_read" require_relative "move_to_extend_os" +require_relative "presence" require_relative "present" require_relative "shell_commands" diff --git a/Library/Homebrew/rubocops/presence.rb b/Library/Homebrew/rubocops/presence.rb new file mode 100644 index 0000000000..0c74c572f8 --- /dev/null +++ b/Library/Homebrew/rubocops/presence.rb @@ -0,0 +1,153 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module Homebrew + # Checks code that can be written more easily using + # `Object#presence` defined by Active Support. + # + # @example + # # bad + # a.present? ? a : nil + # + # # bad + # !a.present? ? nil : a + # + # # bad + # a.blank? ? nil : a + # + # # bad + # !a.blank? ? a : nil + # + # # good + # a.presence + # + # @example + # # bad + # a.present? ? a : b + # + # # bad + # !a.present? ? b : a + # + # # bad + # a.blank? ? b : a + # + # # bad + # !a.blank? ? a : b + # + # # good + # a.presence || b + class Presence < Base + include RangeHelp + extend AutoCorrector + + MSG = "Use `%s` instead of `%s`." + + def_node_matcher :redundant_receiver_and_other, <<~PATTERN + { + (if + (send $_recv :present?) + _recv + $!begin + ) + (if + (send $_recv :blank?) + $!begin + _recv + ) + } + PATTERN + + def_node_matcher :redundant_negative_receiver_and_other, <<~PATTERN + { + (if + (send (send $_recv :present?) :!) + $!begin + _recv + ) + (if + (send (send $_recv :blank?) :!) + _recv + $!begin + ) + } + PATTERN + + def on_if(node) + return if ignore_if_node?(node) + + redundant_receiver_and_other(node) do |receiver, other| + return if ignore_other_node?(other) || receiver.nil? + + register_offense(node, receiver, other) + end + + redundant_negative_receiver_and_other(node) do |receiver, other| + return if ignore_other_node?(other) || receiver.nil? + + register_offense(node, receiver, other) + end + end + + private + + def register_offense(node, receiver, other) + add_offense(node, message: message(node, receiver, other)) do |corrector| + corrector.replace(node, replacement(receiver, other, node.left_sibling)) + end + end + + def ignore_if_node?(node) + node.elsif? + end + + def ignore_other_node?(node) + node && (node.if_type? || node.rescue_type? || node.while_type?) + end + + def message(node, receiver, other) + prefer = replacement(receiver, other, node.left_sibling).gsub(/^\s*|\n/, "") + current = current(node).gsub(/^\s*|\n/, "") + format(MSG, prefer: prefer, current: current) + end + + def current(node) + if !node.ternary? && node.source.include?("\n") + "#{node.loc.keyword.with(end_pos: node.condition.loc.selector.end_pos).source} ... end" + else + node.source.gsub(/\n\s*/, " ") + end + end + + def replacement(receiver, other, left_sibling) + or_source = if other&.send_type? + build_source_for_or_method(other) + elsif other.nil? || other.nil_type? + "" + else + " || #{other.source}" + end + + replaced = "#{receiver.source}.presence#{or_source}" + left_sibling ? "(#{replaced})" : replaced + end + + def build_source_for_or_method(other) + if other.parenthesized? || other.method?("[]") || other.arithmetic_operation? || !other.arguments? + " || #{other.source}" + else + method = method_range(other).source + arguments = other.arguments.map(&:source).join(", ") + + " || #{method}(#{arguments})" + end + end + + def method_range(node) + range_between(node.source_range.begin_pos, node.first_argument.source_range.begin_pos - 1) + end + end + end + end +end diff --git a/Library/Homebrew/test/rubocops/blank_spec.rb b/Library/Homebrew/test/rubocops/blank_spec.rb index c151dd523e..4b71576137 100644 --- a/Library/Homebrew/test/rubocops/blank_spec.rb +++ b/Library/Homebrew/test/rubocops/blank_spec.rb @@ -2,9 +2,7 @@ require "rubocops/blank" -describe RuboCop::Cop::Homebrew::Blank do - subject(:cop) { described_class.new } - +describe RuboCop::Cop::Homebrew::Blank, :config do shared_examples "offense" do |source, correction, message| it "registers an offense and corrects" do expect_offense(<<~RUBY, source: source, message: message) @@ -47,62 +45,62 @@ describe RuboCop::Cop::Homebrew::Blank do context "when nil or empty" do it_behaves_like "offense", "foo.nil? || foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || foo.empty?`." - it_behaves_like "offense", "nil? || empty?", "blank?", "Homebrew/Blank: Use `blank?` instead of `nil? || empty?`." + "Use `foo.blank?` instead of `foo.nil? || foo.empty?`." + it_behaves_like "offense", "nil? || empty?", "blank?", "Use `blank?` instead of `nil? || empty?`." it_behaves_like "offense", "foo == nil || foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of `foo == nil || foo.empty?`." + "Use `foo.blank?` instead of `foo == nil || foo.empty?`." it_behaves_like "offense", "nil == foo || foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of `nil == foo || foo.empty?`." + "Use `foo.blank?` instead of `nil == foo || foo.empty?`." it_behaves_like "offense", "!foo || foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of `!foo || foo.empty?`." + "Use `foo.blank?` instead of `!foo || foo.empty?`." it_behaves_like "offense", "foo.nil? || !!foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || !!foo.empty?`." + "Use `foo.blank?` instead of `foo.nil? || !!foo.empty?`." it_behaves_like "offense", "foo == nil || !!foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of " \ + "Use `foo.blank?` instead of " \ "`foo == nil || !!foo.empty?`." it_behaves_like "offense", "nil == foo || !!foo.empty?", "foo.blank?", - "Homebrew/Blank: Use `foo.blank?` instead of " \ + "Use `foo.blank?` instead of " \ "`nil == foo || !!foo.empty?`." end context "when checking all variable types" do it_behaves_like "offense", "foo.bar.nil? || foo.bar.empty?", "foo.bar.blank?", - "Homebrew/Blank: Use `foo.bar.blank?` instead of " \ + "Use `foo.bar.blank?` instead of " \ "`foo.bar.nil? || foo.bar.empty?`." it_behaves_like "offense", "FOO.nil? || FOO.empty?", "FOO.blank?", - "Homebrew/Blank: Use `FOO.blank?` instead of `FOO.nil? || FOO.empty?`." + "Use `FOO.blank?` instead of `FOO.nil? || FOO.empty?`." it_behaves_like "offense", "Foo.nil? || Foo.empty?", "Foo.blank?", - "Homebrew/Blank: Use `Foo.blank?` instead of `Foo.nil? || Foo.empty?`." + "Use `Foo.blank?` instead of `Foo.nil? || Foo.empty?`." it_behaves_like "offense", "Foo::Bar.nil? || Foo::Bar.empty?", "Foo::Bar.blank?", - "Homebrew/Blank: Use `Foo::Bar.blank?` instead of " \ + "Use `Foo::Bar.blank?` instead of " \ "`Foo::Bar.nil? || Foo::Bar.empty?`." it_behaves_like "offense", "@foo.nil? || @foo.empty?", "@foo.blank?", - "Homebrew/Blank: Use `@foo.blank?` instead of `@foo.nil? || @foo.empty?`." + "Use `@foo.blank?` instead of `@foo.nil? || @foo.empty?`." it_behaves_like "offense", "$foo.nil? || $foo.empty?", "$foo.blank?", - "Homebrew/Blank: Use `$foo.blank?` instead of `$foo.nil? || $foo.empty?`." + "Use `$foo.blank?` instead of `$foo.nil? || $foo.empty?`." it_behaves_like "offense", "@@foo.nil? || @@foo.empty?", "@@foo.blank?", - "Homebrew/Blank: Use `@@foo.blank?` instead of " \ + "Use `@@foo.blank?` instead of " \ "`@@foo.nil? || @@foo.empty?`." it_behaves_like "offense", "foo[bar].nil? || foo[bar].empty?", "foo[bar].blank?", - "Homebrew/Blank: Use `foo[bar].blank?` instead of " \ + "Use `foo[bar].blank?` instead of " \ "`foo[bar].nil? || foo[bar].empty?`." it_behaves_like "offense", "foo(bar).nil? || foo(bar).empty?", "foo(bar).blank?", - "Homebrew/Blank: Use `foo(bar).blank?` instead of " \ + "Use `foo(bar).blank?` instead of " \ "`foo(bar).nil? || foo(bar).empty?`." end end diff --git a/Library/Homebrew/test/rubocops/compact_blank_spec.rb b/Library/Homebrew/test/rubocops/compact_blank_spec.rb index 5762e63f57..5e0917f95c 100644 --- a/Library/Homebrew/test/rubocops/compact_blank_spec.rb +++ b/Library/Homebrew/test/rubocops/compact_blank_spec.rb @@ -2,13 +2,11 @@ require "rubocops/compact_blank" -RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do - subject(:cop) { described_class.new } - +RSpec.describe RuboCop::Cop::Homebrew::CompactBlank, :config do it "registers and corrects an offense when using `reject { |e| e.blank? }`" do expect_offense(<<~RUBY) collection.reject { |e| e.blank? } - ^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead. + ^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank` instead. RUBY expect_correction(<<~RUBY) @@ -19,7 +17,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `reject(&:blank?)`" do expect_offense(<<~RUBY) collection.reject(&:blank?) - ^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead. + ^^^^^^^^^^^^^^^^ Use `compact_blank` instead. RUBY expect_correction(<<~RUBY) @@ -30,7 +28,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `delete_if { |e| e.blank? }`" do expect_offense(<<~RUBY) collection.delete_if { |e| e.blank? } - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead. + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead. RUBY expect_correction(<<~RUBY) @@ -41,7 +39,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `delete_if(&:blank?)`" do expect_offense(<<~RUBY) collection.delete_if(&:blank?) - ^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead. + ^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead. RUBY expect_correction(<<~RUBY) @@ -52,7 +50,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `reject! { |e| e.blank? }`" do expect_offense(<<~RUBY) collection.reject! { |e| e.blank? } - ^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead. + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead. RUBY expect_correction(<<~RUBY) @@ -63,7 +61,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `reject!(&:blank?)`" do expect_offense(<<~RUBY) collection.reject!(&:blank?) - ^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead. + ^^^^^^^^^^^^^^^^^ Use `compact_blank!` instead. RUBY expect_correction(<<~RUBY) @@ -74,7 +72,7 @@ RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do it "registers and corrects an offense when using `reject(&:blank?)` in block" do expect_offense(<<~RUBY) hash.transform_values { |value| value.reject(&:blank?) } - ^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead. + ^^^^^^^^^^^^^^^^ Use `compact_blank` instead. RUBY expect_correction(<<~RUBY) diff --git a/Library/Homebrew/test/rubocops/presence_spec.rb b/Library/Homebrew/test/rubocops/presence_spec.rb new file mode 100644 index 0000000000..23297c66ac --- /dev/null +++ b/Library/Homebrew/test/rubocops/presence_spec.rb @@ -0,0 +1,446 @@ +# frozen_string_literal: true + +require "rubocops/presence" + +RSpec.describe RuboCop::Cop::Homebrew::Presence, :config do + it "registers an offense and corrects when `a.present? ? a : nil`" do + expect_offense(<<~RUBY) + a.present? ? a : nil + ^^^^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `a.present? ? a : nil`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `!a.present? ? nil: a`" do + expect_offense(<<~RUBY) + !a.present? ? nil: a + ^^^^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `!a.present? ? nil: a`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `a.blank? ? nil : a`" do + expect_offense(<<~RUBY) + a.blank? ? nil : a + ^^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `a.blank? ? nil : a`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `!a.blank? ? a : nil`" do + expect_offense(<<~RUBY) + !a.blank? ? a : nil + ^^^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `!a.blank? ? a : nil`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `a.present? ? a : b`" do + expect_offense(<<~RUBY) + a.present? ? a : b + ^^^^^^^^^^^^^^^^^^ Use `a.presence || b` instead of `a.present? ? a : b`. + RUBY + + expect_correction(<<~RUBY) + a.presence || b + RUBY + end + + it "registers an offense and corrects when `!a.present? ? b : a`" do + expect_offense(<<~RUBY) + !a.present? ? b : a + ^^^^^^^^^^^^^^^^^^^ Use `a.presence || b` instead of `!a.present? ? b : a`. + RUBY + + expect_correction(<<~RUBY) + a.presence || b + RUBY + end + + it "registers an offense and corrects when `a.blank? ? b : a`" do + expect_offense(<<~RUBY) + a.blank? ? b : a + ^^^^^^^^^^^^^^^^ Use `a.presence || b` instead of `a.blank? ? b : a`. + RUBY + + expect_correction(<<~RUBY) + a.presence || b + RUBY + end + + it "registers an offense and corrects when `!a.blank? ? a : b`" do + expect_offense(<<~RUBY) + !a.blank? ? a : b + ^^^^^^^^^^^^^^^^^ Use `a.presence || b` instead of `!a.blank? ? a : b`. + RUBY + + expect_correction(<<~RUBY) + a.presence || b + RUBY + end + + it "registers an offense and corrects when `a.present? ? a : 1`" do + expect_offense(<<~RUBY) + a.present? ? a : 1 + ^^^^^^^^^^^^^^^^^^ Use `a.presence || 1` instead of `a.present? ? a : 1`. + RUBY + + expect_correction(<<~RUBY) + a.presence || 1 + RUBY + end + + it "registers an offense and corrects when `a.blank? ? 1 : a`" do + expect_offense(<<~RUBY) + a.blank? ? 1 : a + ^^^^^^^^^^^^^^^^ Use `a.presence || 1` instead of `a.blank? ? 1 : a`. + RUBY + + expect_correction(<<~RUBY) + a.presence || 1 + RUBY + end + + it "registers an offense and corrects when `a(:bar).map(&:baz).present? ? a(:bar).map(&:baz) : nil`" do + expect_offense(<<~RUBY) + a(:bar).map(&:baz).present? ? a(:bar).map(&:baz) : nil + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `a(:bar).map(&:baz).presence` instead of `a(:bar).map(&:baz).present? ? a(:bar).map(&:baz) : nil`. + RUBY + + expect_correction(<<~RUBY) + a(:bar).map(&:baz).presence + RUBY + end + + it "registers an offense and corrects when `a.present? ? a : b[:c]`" do + expect_offense(<<~RUBY) + a.present? ? a : b[:c] + ^^^^^^^^^^^^^^^^^^^^^^ Use `a.presence || b[:c]` instead of `a.present? ? a : b[:c]`. + RUBY + + expect_correction(<<~RUBY) + a.presence || b[:c] + RUBY + end + + it "registers an offense and corrects when multi-line if node" do + expect_offense(<<~RUBY) + if a.present? + ^^^^^^^^^^^^^ Use `a.presence` instead of `if a.present? ... end`. + a + else + nil + end + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when multi-line unless node" do + expect_offense(<<~RUBY) + unless a.present? + ^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `unless a.present? ... end`. + nil + else + a + end + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when multi-line if node with `+` operators in the else branch" do + expect_offense(<<~RUBY) + if a.present? + ^^^^^^^^^^^^^ Use `a.presence || b.to_f + 12.0` instead of `if a.present? ... end`. + a + else + b.to_f + 12.0 + end + RUBY + + expect_correction(<<~RUBY) + a.presence || b.to_f + 12.0 + RUBY + end + + it "registers an offense and corrects when multi-line if `*` operators in the else branch" do + expect_offense(<<~RUBY) + if a.present? + ^^^^^^^^^^^^^ Use `a.presence || b.to_f * 12.0` instead of `if a.present? ... end`. + a + else + b.to_f * 12.0 + end + RUBY + + expect_correction(<<~RUBY) + a.presence || b.to_f * 12.0 + RUBY + end + + it "registers an offense and corrects when `a if a.present?`" do + expect_offense(<<~RUBY) + a if a.present? + ^^^^^^^^^^^^^^^ Use `a.presence` instead of `a if a.present?`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `a unless a.blank?`" do + expect_offense(<<~RUBY) + a unless a.blank? + ^^^^^^^^^^^^^^^^^ Use `a.presence` instead of `a unless a.blank?`. + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + + it "registers an offense and corrects when `.present?` with method chain" do + expect_offense(<<~RUBY) + if [1, 2, 3].map { |num| num + 1 } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `[1, 2, 3].map { |num| num + 1 }.map { |num| num + 2 }.presence || b` instead of `if [1, 2, 3].map { |num| num + 1 }.map { |num| num + 2 }.present? ... end`. + .map { |num| num + 2 } + .present? + [1, 2, 3].map { |num| num + 1 }.map { |num| num + 2 } + else + b + end + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3].map { |num| num + 1 } + .map { |num| num + 2 }.presence || b + RUBY + end + + context "when multiline ternary can be replaced" do + it "registers an offense and corrects" do + expect_offense(<<~RUBY) + a.present? ? + ^^^^^^^^^^^^ Use `a.presence` instead of `a.present? ? a : nil`. + a : + nil + RUBY + + expect_correction(<<~RUBY) + a.presence + RUBY + end + end + + context "when a method argument of `else` branch is enclosed in parentheses" do + it "registers an offense and corrects" do + expect_offense(<<~RUBY) + if value.present? + ^^^^^^^^^^^^^^^^^ Use `value.presence || do_something(value)` instead of `if value.present? ... end`. + value + else + do_something(value) + end + RUBY + + expect_correction(<<~RUBY) + value.presence || do_something(value) + RUBY + end + end + + context "when a method argument of `else` branch is not enclosed in parentheses" do + it "registers an offense and corrects" do + expect_offense(<<~RUBY) + if value.present? + ^^^^^^^^^^^^^^^^^ Use `value.presence || do_something(value)` instead of `if value.present? ... end`. + value + else + do_something value + end + RUBY + + expect_correction(<<~RUBY) + value.presence || do_something(value) + RUBY + end + end + + context "when multiple method arguments of `else` branch is not enclosed in parentheses" do + it "registers an offense and corrects" do + expect_offense(<<~RUBY) + if value.present? + ^^^^^^^^^^^^^^^^^ Use `value.presence || do_something(arg1, arg2)` instead of `if value.present? ... end`. + value + else + do_something arg1, arg2 + end + RUBY + + expect_correction(<<~RUBY) + value.presence || do_something(arg1, arg2) + RUBY + end + end + + context "when a method argument with a receiver of `else` branch is not enclosed in parentheses" do + it "registers an offense and corrects" do + expect_offense(<<~RUBY) + if value.present? + ^^^^^^^^^^^^^^^^^ Use `value.presence || foo.do_something(value)` instead of `if value.present? ... end`. + value + else + foo.do_something value + end + RUBY + + expect_correction(<<~RUBY) + value.presence || foo.do_something(value) + RUBY + end + end + + context "when a right-hand side of the relational operator" do + %w[< > <= >= == !=].each do |operator| + it "registers an offense and corrects when `#{operator}`" do + expect_offense(<<~RUBY, operator: operator) + a #{operator} if b.present? + _{operator} ^^^^^^^^^^^^^ Use `(b.presence || c)` instead of `if b.present? ... end`. + b + else + c + end + RUBY + + expect_correction(<<~RUBY) + a #{operator} (b.presence || c) + RUBY + end + end + end + + it "does not register an offense when using `#presence`" do + expect_no_offenses(<<~RUBY) + a.presence + RUBY + end + + it "does not register an offense when the expression does not return the receiver of `#present?`" do + expect_no_offenses(<<~RUBY) + a.present? ? b : nil + RUBY + + expect_no_offenses(<<~RUBY) + puts foo if present? + puts foo if !present? + RUBY + end + + it "does not register an offense when the expression does not return the receiver of `#blank?`" do + expect_no_offenses(<<~RUBY) + a.blank? ? nil : b + RUBY + + expect_no_offenses(<<~RUBY) + puts foo if blank? + puts foo if !blank? + RUBY + end + + it "does not register an offense when if or unless modifier is used" do + [ + "a if a.blank?", + "a unless a.present?", + ].each { |source| expect_no_offenses(source) } + end + + it "does not register an offense when the else block is multiline" do + expect_no_offenses(<<~RUBY) + if a.present? + a + else + something + something + something + end + RUBY + end + + it "does not register an offense when the else block has multiple statements" do + expect_no_offenses(<<~RUBY) + if a.present? + a + else + something; something; something + end + RUBY + end + + it "does not register an offense when including the elsif block" do + expect_no_offenses(<<~RUBY) + if a.present? + a + elsif b + b + end + RUBY + end + + it "does not register an offense when the else block has `if` node" do + expect_no_offenses(<<~RUBY) + if a.present? + a + else + b if c + end + RUBY + end + + it "does not register an offense when the else block has `rescue` node" do + expect_no_offenses(<<~RUBY) + if something_method.present? + something_method + else + invalid_method rescue StandardError + end + RUBY + end + + it "does not register an offense when the else block has `while` node" do + expect_no_offenses(<<~RUBY) + if a.present? + a + else + fetch_state while waiting? + end + RUBY + end + + it "does not register an offense when using #present? with elsif block" do + expect_no_offenses(<<~RUBY) + if something? + a + elsif b.present? + b + end + RUBY + end +end diff --git a/Library/Homebrew/test/rubocops/present_spec.rb b/Library/Homebrew/test/rubocops/present_spec.rb index ec9280a935..489b5af4e5 100644 --- a/Library/Homebrew/test/rubocops/present_spec.rb +++ b/Library/Homebrew/test/rubocops/present_spec.rb @@ -2,9 +2,7 @@ require "rubocops/present" -describe RuboCop::Cop::Homebrew::Present do - subject(:cop) { described_class.new } - +describe RuboCop::Cop::Homebrew::Present, :config do shared_examples "offense" do |source, correction, message| it "registers an offense and corrects" do expect_offense(<<~RUBY, source: source, message: message) @@ -36,59 +34,59 @@ describe RuboCop::Cop::Homebrew::Present do it_behaves_like "offense", "foo && !foo.empty?", "foo.present?", - "Homebrew/Present: Use `foo.present?` instead of `foo && !foo.empty?`." + "Use `foo.present?` instead of `foo && !foo.empty?`." it_behaves_like "offense", "!foo.nil? && !foo.empty?", "foo.present?", - "Homebrew/Present: Use `foo.present?` instead of `!foo.nil? && !foo.empty?`." + "Use `foo.present?` instead of `!foo.nil? && !foo.empty?`." it_behaves_like "offense", "!nil? && !empty?", "present?", - "Homebrew/Present: Use `present?` instead of `!nil? && !empty?`." + "Use `present?` instead of `!nil? && !empty?`." it_behaves_like "offense", "foo != nil && !foo.empty?", "foo.present?", - "Homebrew/Present: Use `foo.present?` instead of `foo != nil && !foo.empty?`." + "Use `foo.present?` instead of `foo != nil && !foo.empty?`." it_behaves_like "offense", "!!foo && !foo.empty?", "foo.present?", - "Homebrew/Present: Use `foo.present?` instead of `!!foo && !foo.empty?`." + "Use `foo.present?` instead of `!!foo && !foo.empty?`." context "when checking all variable types" do it_behaves_like "offense", "!foo.nil? && !foo.empty?", "foo.present?", - "Homebrew/Present: Use `foo.present?` instead of " \ + "Use `foo.present?` instead of " \ "`!foo.nil? && !foo.empty?`." it_behaves_like "offense", "!foo.bar.nil? && !foo.bar.empty?", "foo.bar.present?", - "Homebrew/Present: Use `foo.bar.present?` instead of " \ + "Use `foo.bar.present?` instead of " \ "`!foo.bar.nil? && !foo.bar.empty?`." it_behaves_like "offense", "!FOO.nil? && !FOO.empty?", "FOO.present?", - "Homebrew/Present: Use `FOO.present?` instead of " \ + "Use `FOO.present?` instead of " \ "`!FOO.nil? && !FOO.empty?`." it_behaves_like "offense", "!Foo.nil? && !Foo.empty?", "Foo.present?", - "Homebrew/Present: Use `Foo.present?` instead of " \ + "Use `Foo.present?` instead of " \ "`!Foo.nil? && !Foo.empty?`." it_behaves_like "offense", "!@foo.nil? && !@foo.empty?", "@foo.present?", - "Homebrew/Present: Use `@foo.present?` instead of " \ + "Use `@foo.present?` instead of " \ "`!@foo.nil? && !@foo.empty?`." it_behaves_like "offense", "!$foo.nil? && !$foo.empty?", "$foo.present?", - "Homebrew/Present: Use `$foo.present?` instead of " \ + "Use `$foo.present?` instead of " \ "`!$foo.nil? && !$foo.empty?`." it_behaves_like "offense", "!@@foo.nil? && !@@foo.empty?", "@@foo.present?", - "Homebrew/Present: Use `@@foo.present?` instead of " \ + "Use `@@foo.present?` instead of " \ "`!@@foo.nil? && !@@foo.empty?`." it_behaves_like "offense", "!foo[bar].nil? && !foo[bar].empty?", "foo[bar].present?", - "Homebrew/Present: Use `foo[bar].present?` instead of " \ + "Use `foo[bar].present?` instead of " \ "`!foo[bar].nil? && !foo[bar].empty?`." it_behaves_like "offense", "!Foo::Bar.nil? && !Foo::Bar.empty?", "Foo::Bar.present?", - "Homebrew/Present: Use `Foo::Bar.present?` instead of " \ + "Use `Foo::Bar.present?` instead of " \ "`!Foo::Bar.nil? && !Foo::Bar.empty?`." it_behaves_like "offense", "!foo(bar).nil? && !foo(bar).empty?", "foo(bar).present?", - "Homebrew/Present: Use `foo(bar).present?` instead of " \ + "Use `foo(bar).present?` instead of " \ "`!foo(bar).nil? && !foo(bar).empty?`." end end