Merge pull request #18614 from Homebrew/inreplace-non-global

utils/inreplace: allow non-global substitution

Library/Homebrew/test/utils/inreplace_spec.rb

@@ -11,49 +11,42 @@
       a
       b
       c
+      aa
     EOS
   end
 
   after { file.unlink }
 
-  it "raises error if there are no files given to replace" do
-    expect do
-      described_class.inreplace [], "d", "f"
-    end.to raise_error(Utils::Inreplace::Error)
-  end
-
-  it "raises error if there is nothing to replace" do
-    expect do
-      described_class.inreplace file.path, "d", "f"
-    end.to raise_error(Utils::Inreplace::Error)
-  end
+  describe ".inreplace" do
+    it "raises error if there are no files given to replace" do
+      expect do
+        described_class.inreplace [], "d", "f"
+      end.to raise_error(Utils::Inreplace::Error)
+    end
 
-  it "raises error if there is nothing to replace in block form" do
-    expect do
-      described_class.inreplace(file.path) do |s|
-        s.gsub!("d", "f") # rubocop:disable Performance/StringReplacement
-      end
-    end.to raise_error(Utils::Inreplace::Error)
-  end
+    it "raises error if there is nothing to replace" do
+      expect do
+        described_class.inreplace file.path, "d", "f"
+      end.to raise_error(Utils::Inreplace::Error)
+    end
 
-  it "raises error if there is no make variables to replace" do
-    expect do
-      described_class.inreplace(file.path) do |s|
-        s.change_make_var! "VAR", "value"
-        s.remove_make_var! "VAR2"
-      end
-    end.to raise_error(Utils::Inreplace::Error)
-  end
+    it "raises error if there is nothing to replace in block form" do
+      expect do
+        described_class.inreplace(file.path) do |s|
+          s.gsub!("d", "f") # rubocop:disable Performance/StringReplacement
+        end
+      end.to raise_error(Utils::Inreplace::Error)
+    end
 
-  describe "#inreplace_pairs" do
-    it "raises error if there is no old value" do
+    it "raises error if there is no make variables to replace" do
       expect do
-        described_class.inreplace_pairs(file.path, [[nil, "f"]])
+        described_class.inreplace(file.path) do |s|
+          s.change_make_var! "VAR", "value"
+          s.remove_make_var! "VAR2"
+        end
       end.to raise_error(Utils::Inreplace::Error)
     end
-  end
 
-  describe "#gsub!" do
     it "substitutes pathname within file" do
       # For a specific instance of this, see https://github.com/Homebrew/homebrew-core/blob/a8b0b10/Formula/loki.rb#L48
       described_class.inreplace(file.path) do |s|
@@ -63,7 +56,74 @@
         a
         f
         c
+        aa
+      EOS
+    end
+
+    it "substitutes all occurrences within file when `global: true`" do
+      described_class.inreplace(file.path, "a", "foo")
+      expect(File.binread(file)).to eq <<~EOS
+        foo
+        b
+        c
+        foofoo
       EOS
     end
+
+    it "substitutes only the first occurrence when `global: false`" do
+      described_class.inreplace(file.path, "a", "foo", global: false)
+      expect(File.binread(file)).to eq <<~EOS
+        foo
+        b
+        c
+        aa
+      EOS
+    end
+  end
+
+  describe ".inreplace_pairs" do
+    it "raises error if there is no old value" do
+      expect do
+        described_class.inreplace_pairs(file.path, [[nil, "f"]])
+      end.to raise_error(Utils::Inreplace::Error)
+    end
+
+    it "substitutes returned string but not file when `read_only_run: true`" do
+      expect(described_class.inreplace_pairs(file.path, [["a", "foo"]], read_only_run: true)).to eq <<~EOS
+        foo
+        b
+        c
+        foofoo
+      EOS
+      expect(File.binread(file)).to eq <<~EOS
+        a
+        b
+        c
+        aa
+      EOS
+    end
+
+    it "substitutes both returned string and file when `read_only_run: false`" do
+      replace_result = <<~TEXT
+        foo
+        b
+        c
+        foofoo
+      TEXT
+      expect(described_class.inreplace_pairs(file.path, [["a", "foo"]])).to eq replace_result
+      expect(File.binread(file)).to eq replace_result
+    end
+
+    it "substitutes multiple pairs in order" do
+      pairs = [["a", "b"], ["bb", "test"], ["b", "z"]]
+      replace_result = <<~TEXT
+        z
+        z
+        c
+        test
+      TEXT
+      expect(described_class.inreplace_pairs(file.path, pairs)).to eq replace_result
+      expect(File.binread(file)).to eq replace_result
+    end
   end
 end

Library/Homebrew/test/utils/string_inreplace_extension_spec.rb

@@ -249,12 +249,20 @@
     it "replaces the first occurrence" do
       string_extension.sub!("o", "e")
       expect(string_extension.inreplace_string).to eq("feo")
+      expect(string_extension.errors).to be_empty
     end
 
     it "adds an error to #errors when no replacement was made" do
       string_extension.sub! "not here", "test"
+      expect(string_extension.inreplace_string).to eq(string)
       expect(string_extension.errors).to eq(['expected replacement of "not here" with "test"'])
     end
+
+    it "doesn't add an error to #errors when no replace was made and `audit_result: false`" do
+      string_extension.sub! "not here", "test", audit_result: false
+      expect(string_extension.inreplace_string).to eq(string)
+      expect(string_extension.errors).to be_empty
+    end
   end
 
   describe "#gsub!" do

Library/Homebrew/utils/inreplace.rb

@@ -47,10 +47,11 @@ def initialize(errors)
         before:       T.nilable(T.any(Pathname, Regexp, String)),
         after:        T.nilable(T.any(Pathname, String, Symbol)),
         audit_result: T::Boolean,
+        global:       T::Boolean,
         block:        T.nilable(T.proc.params(s: StringInreplaceExtension).void),
       ).void
     }
-    def self.inreplace(paths, before = nil, after = nil, audit_result: true, &block)
+    def self.inreplace(paths, before = nil, after = nil, audit_result: true, global: true, &block)
       paths = Array(paths)
       after &&= after.to_s
       before = before.to_s if before.is_a?(Pathname)
@@ -67,8 +68,10 @@ def self.inreplace(paths, before = nil, after = nil, audit_result: true, &block)
           raise ArgumentError, "Must supply a block or before/after params" unless block
 
           yield s
-        else
+        elsif global
           s.gsub!(T.must(before), T.must(after), audit_result:)
+        else
+          s.sub!(T.must(before), T.must(after), audit_result:)
         end
 
         errors[path] = s.errors unless s.errors.empty?

Library/Homebrew/utils/string_inreplace_extension.rb

@@ -1,7 +1,7 @@
-# typed: strict
+# typed: strong
 # frozen_string_literal: true
 
-# Used by the `inreplace` function (in `utils.rb`).
+# Used by the {Utils::Inreplace.inreplace} function.
 class StringInreplaceExtension
   sig { returns(T::Array[String]) }
   attr_accessor :errors
@@ -18,10 +18,10 @@ def initialize(string)
   # Same as `String#sub!`, but warns if nothing was replaced.
   #
   # @api public
-  sig { params(before: T.any(Regexp, String), after: String).returns(T.nilable(String)) }
-  def sub!(before, after)
+  sig { params(before: T.any(Regexp, String), after: String, audit_result: T::Boolean).returns(T.nilable(String)) }
+  def sub!(before, after, audit_result: true)
     result = inreplace_string.sub!(before, after)
-    errors << "expected replacement of #{before.inspect} with #{after.inspect}" unless result
+    errors << "expected replacement of #{before.inspect} with #{after.inspect}" if audit_result && result.nil?
     result
   end