diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index bb4e40a00a..65011773b3 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -2,7 +2,6 @@ require: - ./Homebrew/rubocops.rb - rubocop-performance - - rubocop-rails - rubocop-rspec - rubocop-sorbet @@ -54,8 +53,8 @@ Homebrew: Homebrew/Blank: Exclude: # Core extensions are not available here: - - "Homebrew/rubocops/**/*" - "Homebrew/startup/bootsnap.rb" + Homebrew/CompactBlank: Exclude: # `blank?` is not necessarily available here: @@ -210,19 +209,6 @@ Performance/CaseWhenSplat: Performance/MethodObjectAsBlock: Enabled: false -Rails: - # Selectively enable what we want. - Enabled: false - Exclude: - # This file is loaded before any extra methods are defined. - - "Homebrew/standalone/init.rb" - # Do not use ActiveSupport in RuboCops. - - "Homebrew/rubocops/**/*" - -# These relate to ActiveSupport and not other parts of Rails. -Rails/SafeNavigationWithBlank: - Enabled: true - # Intentionally disabled as it doesn't fit with our code style. RSpec/AnyInstance: Enabled: false @@ -368,8 +354,6 @@ Style/InvertibleUnlessCondition: :==: :!= # Unset this (prefer `unless a.zero?` over `if a.nonzero?`) :zero?: - # Don't require non-standard `exclude?` (for now at least) - it's not available in every file - :include?: :blank?: :present? Style/MutableConstant: diff --git a/Library/Homebrew/rubocops/all.rb b/Library/Homebrew/rubocops/all.rb index 1555aa22f8..53bc42a57e 100644 --- a/Library/Homebrew/rubocops/all.rb +++ b/Library/Homebrew/rubocops/all.rb @@ -2,12 +2,14 @@ # frozen_string_literal: true require_relative "../extend/array" +require_relative "../extend/blank" 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 "safe_navigation_with_blank" require_relative "shell_commands" # formula audit cops diff --git a/Library/Homebrew/rubocops/dependency_order.rb b/Library/Homebrew/rubocops/dependency_order.rb index 7d8efd707c..062d30d662 100644 --- a/Library/Homebrew/rubocops/dependency_order.rb +++ b/Library/Homebrew/rubocops/dependency_order.rb @@ -75,7 +75,7 @@ module RuboCop ordered.each_with_index do |dep, pos| idx = pos+1 match_nodes = build_with_dependency_name(dep) - next if !match_nodes || match_nodes.empty? + next if match_nodes.blank? idx1 = pos ordered.drop(idx1+1).each_with_index do |dep2, pos2| diff --git a/Library/Homebrew/rubocops/safe_navigation_with_blank.rb b/Library/Homebrew/rubocops/safe_navigation_with_blank.rb new file mode 100644 index 0000000000..4e104d2d5e --- /dev/null +++ b/Library/Homebrew/rubocops/safe_navigation_with_blank.rb @@ -0,0 +1,51 @@ +# typed: true +# frozen_string_literal: true + +module RuboCop + module Cop + module Homebrew + # Checks to make sure safe navigation isn't used with `blank?` in + # a conditional. + # + # @note + # While the safe navigation operator is generally a good idea, when + # checking `foo&.blank?` in a conditional, `foo` being `nil` will actually + # do the opposite of what the author intends. + # + # For example: + # + # [source,ruby] + # ---- + # foo&.blank? #=> nil + # foo.blank? #=> true + # ---- + # + # @example + # # bad + # do_something if foo&.blank? + # do_something unless foo&.blank? + # + # # good + # do_something if foo.blank? + # do_something unless foo.blank? + # + class SafeNavigationWithBlank < Base + extend AutoCorrector + + MSG = "Avoid calling `blank?` with the safe navigation operator in conditionals." + + def_node_matcher :safe_navigation_blank_in_conditional?, <<~PATTERN + (if $(csend ... :blank?) ...) + PATTERN + + def on_if(node) + return unless safe_navigation_blank_in_conditional?(node) + + add_offense(node) do |corrector| + corrector.replace(safe_navigation_blank_in_conditional?(node).location.dot, ".") + end + end + end + end + end +end diff --git a/Library/Homebrew/test/rubocops/safe_navigation_with_blank_spec.rb b/Library/Homebrew/test/rubocops/safe_navigation_with_blank_spec.rb new file mode 100644 index 0000000000..0b057fbece --- /dev/null +++ b/Library/Homebrew/test/rubocops/safe_navigation_with_blank_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require "rubocops/safe_navigation_with_blank" + +RSpec.describe RuboCop::Cop::Homebrew::SafeNavigationWithBlank, :config do + context "when in a conditional" do + it "registers an offense on a single conditional" do + expect_offense(<<~RUBY) + do_something unless foo&.blank? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `blank?` with the safe navigation operator in conditionals. + RUBY + + expect_correction(<<~RUBY) + do_something unless foo.blank? + RUBY + end + + it "registers an offense on chained conditionals" do + expect_offense(<<~RUBY) + do_something unless foo&.bar&.blank? + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid calling `blank?` with the safe navigation operator in conditionals. + RUBY + + expect_correction(<<~RUBY) + do_something unless foo&.bar.blank? + RUBY + end + + it "does not register an offense on `.blank?`" do + expect_no_offenses(<<~RUBY) + return if foo.blank? + RUBY + end + end + + context "when outside a conditional" do + it "registers no offense" do + expect_no_offenses(<<~RUBY) + bar = foo&.blank? + RUBY + end + end +end