From f82c58383c32a1c755a4e86c062eeb260d310cd7 Mon Sep 17 00:00:00 2001 From: Michael Cho Date: Wed, 23 Oct 2024 12:20:24 -0400 Subject: [PATCH] utils/inreplace: allow non-global substitution Also increase test coverage --- Library/Homebrew/test/utils/inreplace_spec.rb | 122 +++++++++++++----- .../utils/string_inreplace_extension_spec.rb | 8 ++ Library/Homebrew/utils/inreplace.rb | 7 +- .../utils/string_inreplace_extension.rb | 10 +- 4 files changed, 109 insertions(+), 38 deletions(-) diff --git a/Library/Homebrew/test/utils/inreplace_spec.rb b/Library/Homebrew/test/utils/inreplace_spec.rb index a6b0a044b5d54..bc2fa549cabf1 100644 --- a/Library/Homebrew/test/utils/inreplace_spec.rb +++ b/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 diff --git a/Library/Homebrew/test/utils/string_inreplace_extension_spec.rb b/Library/Homebrew/test/utils/string_inreplace_extension_spec.rb index 1285613a24a46..c59c4a342a4b6 100644 --- a/Library/Homebrew/test/utils/string_inreplace_extension_spec.rb +++ b/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 diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index a7e3e9bddf6ff..3d380301d1c83 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/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? diff --git a/Library/Homebrew/utils/string_inreplace_extension.rb b/Library/Homebrew/utils/string_inreplace_extension.rb index bca6fa1936786..872a252a31fcf 100644 --- a/Library/Homebrew/utils/string_inreplace_extension.rb +++ b/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