From d4d1b4a22a576f9f2adb1ec0b34072bf99b67233 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Thu, 10 Jul 2025 22:45:56 +0100 Subject: [PATCH] rubocops/text: Prefer `lib/"string"` over `lib+"string"` - I found a few occurrences of this pattern from https://github.com/orgs/Homebrew/projects/5?pane=issue&itemId=97021840, that is an automated style request for: `core: use / instead of + operator in e.g. (lib+"lv").install "lv.hlp"`. - Upon adding tests I realised that there's also the `prefix + "bin"` case that's already handled differently, so let's combine the handling given it's the same `+` that's wrong. --- Library/Homebrew/rubocops/text.rb | 36 ++++++++++++++------- Library/Homebrew/test/rubocops/text_spec.rb | 13 ++++++++ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index 7b7469b10f..3323aeb176 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -90,6 +90,30 @@ module RuboCop problem "Use separate `make` calls" end + find_every_method_call_by_name(body_node, :+).each do |plus_node| + next unless plus_node.receiver&.send_type? + next unless plus_node.first_argument&.str_type? + + receiver_method = plus_node.receiver.method_name + path_arg = plus_node.first_argument.str_content + + case receiver_method + when :prefix + next unless (match = path_arg.match(%r{^(bin|include|libexec|lib|sbin|share|Frameworks)(?:/| |$)})) + + offending_node(plus_node) + problem "Use `#{match[1].downcase}` instead of `prefix + \"#{match[1]}\"`" + when :bin, :include, :libexec, :lib, :sbin, :share + next if path_arg.empty? + + offending_node(plus_node) + good = "#{receiver_method}/\"#{path_arg}\"" + problem "Use `#{good}` instead of `#{plus_node.source}`" do |corrector| + corrector.replace(plus_node.loc.expression, good) + end + end + end + body_node.each_descendant(:dstr) do |dstr_node| dstr_node.each_descendant(:begin) do |interpolation_node| next unless interpolation_node.source.match?(/#\{\w+\s*\+\s*['"][^}]+\}/) @@ -98,19 +122,7 @@ module RuboCop problem "Do not concatenate paths in string interpolation" end end - - prefix_path(body_node) do |prefix_node, path| - next unless (match = path.match(%r{^(bin|include|libexec|lib|sbin|share|Frameworks)(?:/| |$)})) - - offending_node(prefix_node) - problem "Use `#{match[1].downcase}` instead of `prefix + \"#{match[1]}\"`" - end end - - # Find: prefix + "foo" - def_node_search :prefix_path, <<~EOS - $(send (send nil? :prefix) :+ (str $_)) - EOS end end diff --git a/Library/Homebrew/test/rubocops/text_spec.rb b/Library/Homebrew/test/rubocops/text_spec.rb index f45b917da5..8a8ca38e0b 100644 --- a/Library/Homebrew/test/rubocops/text_spec.rb +++ b/Library/Homebrew/test/rubocops/text_spec.rb @@ -231,6 +231,19 @@ RSpec.describe RuboCop::Cop::FormulaAudit::Text do RUBY end + it 'reports offenses if eg `lib+"thing"` is present' do + expect_offense(<<~RUBY) + class Foo < Formula + def install + (lib+"foo").install + ^^^^^^^^^ FormulaAudit/Text: Use `lib/"foo"` instead of `lib+"foo"` + (bin+"foobar").install + ^^^^^^^^^^^^ FormulaAudit/Text: Use `bin/"foobar"` instead of `bin+"foobar"` + end + end + RUBY + end + it 'reports an offense if `prefix + "bin"` is present' do expect_offense(<<~RUBY) class Foo < Formula