From fd382d2ecdb2aedc28aa3186d3d25618c9235548 Mon Sep 17 00:00:00 2001 From: vidusheeamoli Date: Sat, 25 Jul 2020 00:48:15 +0530 Subject: [PATCH 1/2] srb: set utils/inreplace.rb to true and refactor - Sorbet gives preference to class methods over methods defined in included modules, hence Sorbet was unavailable to resolve the definition of the gsub! method. - The gsub! method in StringInreplaceExtension conflicts with the definition in String. - This PR refactors the call to the gsub! method so that a custom object is exposed instead of a string. --- Library/Homebrew/dev-cmd/bottle.rb | 2 +- Library/Homebrew/extend/string.rb | 15 ++++++---- Library/Homebrew/sorbet/files.yaml | 2 +- .../Homebrew/sorbet/rbi/utils/inreplace.rbi | 27 ++++++++++++++++++ Library/Homebrew/test/inreplace_spec.rb | 28 +++++++++---------- Library/Homebrew/test/string_spec.rb | 2 +- Library/Homebrew/utils/inreplace.rb | 5 ++-- 7 files changed, 57 insertions(+), 24 deletions(-) create mode 100644 Library/Homebrew/sorbet/rbi/utils/inreplace.rbi diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 8386d76e3b..113c0bad39 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -480,7 +480,7 @@ module Homebrew update_or_add = nil Utils::Inreplace.inreplace(path) do |s| - if s.include? "bottle do" + if s.inreplace_string.include? "bottle do" update_or_add = "update" if args.keep_old? mismatches = [] diff --git a/Library/Homebrew/extend/string.rb b/Library/Homebrew/extend/string.rb index 903168af60..90f3134f9d 100644 --- a/Library/Homebrew/extend/string.rb +++ b/Library/Homebrew/extend/string.rb @@ -3,22 +3,27 @@ require "active_support/core_ext/object/blank" # Used by the inreplace function (in `utils.rb`). -module StringInreplaceExtension - attr_accessor :errors +class StringInreplaceExtension + attr_accessor :errors, :inreplace_string + + def initialize(str) + @inreplace_string = str + @errors = [] + end def self.extended(str) str.errors = [] end def sub!(before, after) - result = super + result = inreplace_string.sub!(before, after) errors << "expected replacement of #{before.inspect} with #{after.inspect}" unless result result end # Warn if nothing was replaced def gsub!(before, after, audit_result = true) - result = super(before, after) + result = inreplace_string.gsub!(before, after) errors << "expected replacement of #{before.inspect} with #{after.inspect}" if audit_result && result.nil? result end @@ -43,6 +48,6 @@ module StringInreplaceExtension # Finds the specified variable def get_make_var(flag) - self[/^#{Regexp.escape(flag)}[ \t]*[\\?+:!]?=[ \t]*((?:.*\\\n)*.*)$/, 1] + inreplace_string[/^#{Regexp.escape(flag)}[ \t]*[\\?+:!]?=[ \t]*((?:.*\\\n)*.*)$/, 1] end end diff --git a/Library/Homebrew/sorbet/files.yaml b/Library/Homebrew/sorbet/files.yaml index 1b5eed62eb..d81983cb6c 100644 --- a/Library/Homebrew/sorbet/files.yaml +++ b/Library/Homebrew/sorbet/files.yaml @@ -824,7 +824,6 @@ false: - ./test/version_spec.rb - ./unpack_strategy/uncompressed.rb - ./utils/gems.rb - - ./utils/inreplace.rb - ./version.rb true: @@ -888,6 +887,7 @@ true: - ./test/support/helper/fixtures.rb - ./test/support/lib/config.rb - ./utils/bottles.rb + - ./utils/inreplace.rb - ./utils/link.rb - ./utils/notability.rb - ./utils/shebang.rb diff --git a/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi b/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi new file mode 100644 index 0000000000..126e961833 --- /dev/null +++ b/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi @@ -0,0 +1,27 @@ +# typed: strict + +module Utils::Inreplace + include Kernel + + sig { params(paths: T::Array[T.untyped], before: T.nilable(String), after: T.nilable(String), audit_result: T::Boolean).void } + def inreplace(paths, before = nil, after = nil, audit_result = true); end + +end + +class StringInreplaceExtension + sig { params(before: String, after: String).returns(T.nilable(String)) } + def sub!(before, after) + end + + sig { params(before: T.nilable(String), after: T.nilable(String), audit_result: T::Boolean).returns(T.nilable(String)) } + def gsub!(before, after, audit_result = true); end + + def change_make_var!(flag, new_value) + end + + def remove_make_var!(flags) + end + + def get_make_var(flag) + end +end diff --git a/Library/Homebrew/test/inreplace_spec.rb b/Library/Homebrew/test/inreplace_spec.rb index 965cd0035b..e35e5bd3ae 100644 --- a/Library/Homebrew/test/inreplace_spec.rb +++ b/Library/Homebrew/test/inreplace_spec.rb @@ -5,7 +5,7 @@ require "tempfile" require "utils/inreplace" describe StringInreplaceExtension do - subject { string.dup.extend(described_class) } + subject { described_class.new(string.dup) } describe "#change_make_var!" do context "flag" do @@ -20,7 +20,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "FLAG", "def" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def FLAG=def FLAG2=abc @@ -29,7 +29,7 @@ describe StringInreplaceExtension do it "is successfully appended" do subject.change_make_var! "FLAG", "\\1 def" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def FLAG=abc def FLAG2=abc @@ -47,7 +47,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "CFLAGS", "-O3" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS CFLAGS=-O3 LDFLAGS\t=\t-lcrypto -lssl EOS @@ -65,7 +65,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "CFLAGS", "-O3" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS CFLAGS=-O3 LDFLAGS = -lcrypto -lssl EOS @@ -84,7 +84,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "FLAG", "def" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def FLAG=def FLAG2=abc @@ -102,7 +102,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "FLAG", "def" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS FLAG=def mv file_a file_b EOS @@ -120,7 +120,7 @@ describe StringInreplaceExtension do it "is successfully replaced" do subject.change_make_var! "FLAG", "def" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def FLAG=def FLAG2=abc @@ -142,7 +142,7 @@ describe StringInreplaceExtension do it "is successfully removed" do subject.remove_make_var! "FLAG" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def FLAG2 = def EOS @@ -159,7 +159,7 @@ describe StringInreplaceExtension do it "is successfully removed" do subject.remove_make_var! "LDFLAGS" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS CFLAGS\t=\t-Wall -O2 EOS end @@ -176,7 +176,7 @@ describe StringInreplaceExtension do it "is successfully removed" do subject.remove_make_var! "CFLAGS" - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS LDFLAGS = -lcrypto -lssl EOS end @@ -195,7 +195,7 @@ describe StringInreplaceExtension do specify "are be successfully removed" do subject.remove_make_var! ["FLAG", "FLAG2"] - expect(subject).to eq <<~EOS + expect(subject.inreplace_string).to eq <<~EOS OTHER=def OTHER2=def EOS @@ -250,7 +250,7 @@ describe StringInreplaceExtension do it "replaces the first occurrence" do subject.sub!("o", "e") - expect(subject).to eq("feo") + expect(subject.inreplace_string).to eq("feo") end end @@ -259,7 +259,7 @@ describe StringInreplaceExtension do it "replaces all occurrences" do subject.gsub!("o", "e") # rubocop:disable Performance/StringReplacement - expect(subject).to eq("fee") + expect(subject.inreplace_string).to eq("fee") end end end diff --git a/Library/Homebrew/test/string_spec.rb b/Library/Homebrew/test/string_spec.rb index ed9cdc2deb..646d53765a 100644 --- a/Library/Homebrew/test/string_spec.rb +++ b/Library/Homebrew/test/string_spec.rb @@ -3,7 +3,7 @@ require "extend/string" describe StringInreplaceExtension do - subject { string.extend(described_class) } + subject { described_class.new(string) } let(:string) { +"foobar" } diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index 59f2552679..bf0aeb7add 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -25,7 +25,8 @@ module Utils errors["`paths` (first) parameter"] = ["`paths` was empty"] if paths.blank? Array(paths).each do |path| - s = File.open(path, "rb", &:read).extend(StringInreplaceExtension) + str = File.open(path, "rb", &:read) || "" + s = StringInreplaceExtension.new(str) if before.nil? && after.nil? yield s @@ -36,7 +37,7 @@ module Utils errors[path] = s.errors unless s.errors.empty? - Pathname(path).atomic_write(s) + Pathname(path).atomic_write(s.inreplace_string) end raise InreplaceError, errors unless errors.empty? From c27916cd8723b279b0c36d3d7f3ec01c76823990 Mon Sep 17 00:00:00 2001 From: vidusheeamoli Date: Sun, 26 Jul 2020 13:32:17 +0530 Subject: [PATCH 2/2] srb/inreplace.rbi: add method signatures --- Library/Homebrew/sorbet/rbi/utils/inreplace.rbi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi b/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi index 126e961833..6c9bfb5b3e 100644 --- a/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi +++ b/Library/Homebrew/sorbet/rbi/utils/inreplace.rbi @@ -16,12 +16,15 @@ class StringInreplaceExtension sig { params(before: T.nilable(String), after: T.nilable(String), audit_result: T::Boolean).returns(T.nilable(String)) } def gsub!(before, after, audit_result = true); end + sig {params(flag: String, new_value: String).void} def change_make_var!(flag, new_value) end + sig {params(flags: T::Array[String]).void} def remove_make_var!(flags) end + sig {params(flag: String).returns(String)} def get_make_var(flag) end end