From 654a58237e0892e5c1b28e4e96a639d335e36ba7 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Sat, 31 Aug 2024 12:29:43 +0800 Subject: [PATCH 1/2] formula: make `audit_result` a kwarg in `inreplace` This allows us to get rid of a RuboCop disable, and will make formulae easier to read. --- Library/Homebrew/formula.rb | 19 +++++++++++++------ .../utils/string_inreplace_extension.rb | 16 +++++++++++++--- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index f0f5704ea1..4438c1f1ed 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2863,14 +2863,21 @@ class Formula # @api public sig { params( - paths: T.any(T::Enumerable[T.any(String, Pathname)], String, Pathname), - before: T.nilable(T.any(Pathname, Regexp, String)), - after: T.nilable(T.any(Pathname, String, Symbol)), - audit_result: T::Boolean, - block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), + paths: T.any(T::Enumerable[T.any(String, Pathname)], String, Pathname), + before: T.nilable(T.any(Pathname, Regexp, String)), + after: T.nilable(T.any(Pathname, String, Symbol)), + old_audit_result: T.nilable(T::Boolean), + audit_result: T::Boolean, + block: T.nilable(T.proc.params(s: StringInreplaceExtension).void), ).void } - def inreplace(paths, before = nil, after = nil, audit_result = true, &block) # rubocop:disable Style/OptionalBooleanParameter + def inreplace(paths, before = nil, after = nil, old_audit_result = nil, audit_result: true, &block) + # NOTE: must check for `#nil?` and not `#blank?`. + unless old_audit_result.nil? + # odeprecated "inreplace(paths, before, after, #{old_audit_result})", + # "inreplace(paths, before, after, audit_result: #{old_audit_result})" + audit_result = old_audit_result + end Utils::Inreplace.inreplace(paths, before, after, audit_result:, &block) rescue Utils::Inreplace::Error => e onoe e.to_s diff --git a/Library/Homebrew/utils/string_inreplace_extension.rb b/Library/Homebrew/utils/string_inreplace_extension.rb index fffaf771df..b140c1fa72 100644 --- a/Library/Homebrew/utils/string_inreplace_extension.rb +++ b/Library/Homebrew/utils/string_inreplace_extension.rb @@ -29,10 +29,20 @@ class StringInreplaceExtension # # @api public sig { - params(before: T.any(Pathname, Regexp, String), after: T.any(Pathname, String), audit_result: T::Boolean) - .returns(T.nilable(String)) + params( + before: T.any(Pathname, Regexp, String), + after: T.any(Pathname, String), + old_audit_result: T.nilable(T::Boolean), + audit_result: T::Boolean, + ).returns(T.nilable(String)) } - def gsub!(before, after, audit_result = true) # rubocop:disable Style/OptionalBooleanParameter + def gsub!(before, after, old_audit_result = nil, audit_result: true) + # NOTE: must check for `#nil?` and not `#blank?`. + unless old_audit_result.nil? + # odeprecated "gsub!(before, after, #{old_audit_result})", + # "gsub!(before, after, audit_result: #{old_audit_result})" + audit_result = old_audit_result + end before = before.to_s if before.is_a?(Pathname) result = inreplace_string.gsub!(before, after.to_s) errors << "expected replacement of #{before.inspect} with #{after.inspect}" if audit_result && result.nil? From c73d08c75b9f928a462f7514b8239c04911e9d34 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Mon, 2 Sep 2024 16:02:35 +0800 Subject: [PATCH 2/2] Update comment re `#nil?` vs `#blank?` --- Library/Homebrew/formula.rb | 2 +- Library/Homebrew/utils/string_inreplace_extension.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 4438c1f1ed..75e1a2a066 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2872,7 +2872,7 @@ class Formula ).void } def inreplace(paths, before = nil, after = nil, old_audit_result = nil, audit_result: true, &block) - # NOTE: must check for `#nil?` and not `#blank?`. + # NOTE: must check for `#nil?` and not `#blank?`, or else `old_audit_result = false` will not call `odeprecated`. unless old_audit_result.nil? # odeprecated "inreplace(paths, before, after, #{old_audit_result})", # "inreplace(paths, before, after, audit_result: #{old_audit_result})" diff --git a/Library/Homebrew/utils/string_inreplace_extension.rb b/Library/Homebrew/utils/string_inreplace_extension.rb index b140c1fa72..e1a1cc625e 100644 --- a/Library/Homebrew/utils/string_inreplace_extension.rb +++ b/Library/Homebrew/utils/string_inreplace_extension.rb @@ -37,7 +37,7 @@ class StringInreplaceExtension ).returns(T.nilable(String)) } def gsub!(before, after, old_audit_result = nil, audit_result: true) - # NOTE: must check for `#nil?` and not `#blank?`. + # NOTE: must check for `#nil?` and not `#blank?`, or else `old_audit_result = false` will not call `odeprecated`. unless old_audit_result.nil? # odeprecated "gsub!(before, after, #{old_audit_result})", # "gsub!(before, after, audit_result: #{old_audit_result})"