From 3713939e0dfe85fa8d00974fdf619f3b2c92528c Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 24 Jul 2024 21:57:45 +0100 Subject: [PATCH] rubocops/text: Include dashed binaries in `bin/` interpolation check - Previously this only included the formula name. - But, for example in tests, we have "#{bin}/ansible-test", not just "#{bin}/ansible". So handle that too. - I decided to make the error message better by extracting the binary name from the interpolation, but I'm not sure it was worth it. ``` $ brew audit --strict ansible ansible * line 580, col 29: Use `bin/"ansible-test"` instead of `"#{bin}/ansible-test"` Error: 1 problem in 1 formula detected. ``` --- Library/Homebrew/rubocops/text.rb | 15 +++++++++------ .../Homebrew/test/rubocops/text/strict_spec.rb | 4 +++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index b3e06f050e..9f3454c033 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -137,9 +137,10 @@ module RuboCop problem "Use `\#{pkgshare}` instead of `\#{share}/#{@formula_name}`" end - interpolated_bin_path_starts_with(body_node, "/#{@formula_name}") do |bin_node| + interpolated_bin_path_starts_with(body_node, "/#{@formula_name}", true) do |bin_node| offending_node(bin_node) - problem "Use `bin/\"#{@formula_name}\"` instead of `\"\#{bin}/#{@formula_name}\"`" + 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" @@ -150,8 +151,10 @@ 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 # Find "#{share}/foo" @@ -159,9 +162,9 @@ module RuboCop $(dstr (begin (send nil? :share)) (str #path_starts_with?(%1))) EOS - # Find "#{bin}/foo" + # 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?(%1))) + $(dstr (begin (send nil? :bin)) (str #path_starts_with?(%1, %2))) EOS # Find share/"foo" diff --git a/Library/Homebrew/test/rubocops/text/strict_spec.rb b/Library/Homebrew/test/rubocops/text/strict_spec.rb index e39288dcdc..f802cb376d 100644 --- a/Library/Homebrew/test/rubocops/text/strict_spec.rb +++ b/Library/Homebrew/test/rubocops/text/strict_spec.rb @@ -132,12 +132,14 @@ RSpec.describe RuboCop::Cop::FormulaAuditStrict::Text do RUBY end - it 'reports an offense if "\#{bin}/" is present' do + it 'reports an offense if "\#{bin}/" 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