Merge pull request #17826 from Homebrew/no-interpolated-bin

rubocops/text: Enforce `bin/"formula"` instead of `"#{bin}/formula"`

Library/Homebrew/rubocops/text.rb

@@ -137,6 +137,12 @@ module RuboCop
             problem "Use `\#{pkgshare}` instead of `\#{share}/#{@formula_name}`"
           end
 
+          interpolated_bin_path_starts_with(body_node, "/#{@formula_name}") do |bin_node|
+            offending_node(bin_node)
+            cmd = bin_node.source.match(%r{\#{bin}/(\S+)})[1]&.delete_suffix('"') || @formula_name
+            problem "Use `bin/\"#{cmd}\"` instead of `\"\#{bin}/#{cmd}\"`"
+          end
+
           return if formula_tap != "homebrew-core"
 
           find_method_with_args(body_node, :env, :std) do
@@ -145,8 +151,14 @@ module RuboCop
         end
 
         # Check whether value starts with the formula name and then a "/", " " or EOS.
-        def path_starts_with?(path, starts_with)
-          path.match?(%r{^#{Regexp.escape(starts_with)}(/| |$)})
+        # If we're checking for "#{bin}", we also check for "-" since similar binaries also don't need interpolation.
+        def path_starts_with?(path, starts_with, bin: false)
+          ending = bin ? "/| |-|$" : "/| |$"
+          path.match?(/^#{Regexp.escape(starts_with)}(#{ending})/)
+        end
+
+        def path_starts_with_bin?(path, starts_with)
+          path_starts_with?(path, starts_with, bin: true)
         end
 
         # Find "#{share}/foo"
@@ -154,6 +166,11 @@ module RuboCop
           $(dstr (begin (send nil? :share)) (str #path_starts_with?(%1)))
         EOS
 
+        # Find "#{bin}/foo" and "#{bin}/foo-bar"
+        def_node_search :interpolated_bin_path_starts_with, <<~EOS
+          $(dstr (begin (send nil? :bin)) (str #path_starts_with_bin?(%1)))
+        EOS
+
         # Find share/"foo"
         def_node_search :share_path_starts_with, <<~EOS
           $(send (send nil? :share) :/ (str #path_starts_with?(%1)))

Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/formula_audit_strict/text.rbi

@@ -6,6 +6,16 @@
 
 
 class RuboCop::Cop::FormulaAuditStrict::Text
+  sig do
+    params(
+      node: RuboCop::AST::Node,
+      pattern: T.any(String, Symbol),
+      kwargs: T.untyped,
+      block: T.untyped
+    ).returns(T.untyped)
+  end
+  def interpolated_bin_path_starts_with(node, *pattern, **kwargs, &block); end
+
   sig do
     params(
       node: RuboCop::AST::Node,

Library/Homebrew/test/rubocops/text/strict_spec.rb

@@ -131,5 +131,29 @@ RSpec.describe RuboCop::Cop::FormulaAuditStrict::Text do
         end
       RUBY
     end
+
+    it 'reports an offense if "\#{bin}/<formula_name>" or other dashed binaries too are present' do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          test do
+            ohai "\#{bin}/foo", "-v"
+                 ^^^^^^^^^^^^ FormulaAuditStrict/Text: Use `bin/"foo"` instead of `"\#{bin}/foo"`
+            ohai "\#{bin}/foo-bar", "-v"
+                 ^^^^^^^^^^^^^^^^ FormulaAuditStrict/Text: Use `bin/"foo-bar"` instead of `"\#{bin}/foo-bar"`
+          end
+        end
+      RUBY
+    end
+
+    it 'reports an offense if "\#{bin}" is in a `shell_output` string' do
+      expect_offense(<<~RUBY, "/homebrew-core/Formula/foo.rb")
+        class Foo < Formula
+          test do
+            shell_output("\#{bin}/foo --version")
+                         ^^^^^^^^^^^^^^^^^^^^^^ FormulaAuditStrict/Text: Use `bin/"foo"` instead of `"\#{bin}/foo"`
+          end
+        end
+      RUBY
+    end
   end
 end