From 04fa6e24d79d6076153a768b7e4cb5049ddc7dab Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sun, 12 Mar 2023 17:06:29 -0700 Subject: [PATCH 1/5] Enable more typing --- Library/Homebrew/build.rb | 8 ++++---- Library/Homebrew/cask/cask.rb | 29 ++++++++++++++++++++------- Library/Homebrew/cask/cask_loader.rb | 17 +++++++++++++--- Library/Homebrew/cask/dsl.rb | 14 ++++++------- Library/Homebrew/cask/dsl/base.rb | 4 ++-- Library/Homebrew/extend/on_system.rbi | 6 ++++++ Library/Homebrew/os/linux.rb | 2 +- Library/Homebrew/os/mac.rb | 4 ++-- 8 files changed, 58 insertions(+), 26 deletions(-) create mode 100644 Library/Homebrew/extend/on_system.rbi diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index ebdc621149..1ba4a72793 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true # This script is loaded by formula_installer as a separate instance. @@ -235,12 +235,12 @@ rescue Exception => e # rubocop:disable Lint/RescueException # BuildErrors are specific to build processes and not other # children, which is why we create the necessary state here # and not in Utils.safe_fork. - case error_hash["json_class"] - when "BuildError" + case e + when BuildError error_hash["cmd"] = e.cmd error_hash["args"] = e.args error_hash["env"] = e.env - when "ErrorDuringExecution" + when ErrorDuringExecution error_hash["cmd"] = e.cmd error_hash["status"] = if e.status.is_a?(Process::Status) { diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 0ddf372e84..1a828c28f1 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "cask/cask_loader" @@ -83,6 +83,19 @@ module Cask @tap end + sig { + params( + token: String, + sourcefile_path: T.nilable(Pathname), + source: T.nilable(String), + tap: T.nilable(Tap), + loaded_from_api: T::Boolean, + config: T.nilable(Config), + allow_reassignment: T::Boolean, + loader: T.nilable(CaskLoader::ILoader), + block: T.nilable(T.proc.bind(DSL).void), + ).void + } def initialize(token, sourcefile_path: nil, source: nil, tap: nil, loaded_from_api: false, config: nil, allow_reassignment: false, loader: nil, &block) @token = token @@ -92,7 +105,8 @@ module Cask @allow_reassignment = allow_reassignment @loaded_from_api = loaded_from_api @loader = loader - @block = block + # https://github.com/sorbet/sorbet/issues/6843 + instance_variable_set(:@block, block) @default_config = config || Config.new @@ -123,10 +137,11 @@ module Cask sig { returns(T::Array[[String, String]]) } def timestamped_versions - Pathname.glob(metadata_timestamped_path(version: "*", timestamp: "*")) - .map { |p| p.relative_path_from(p.parent.parent) } - .sort_by(&:basename) # sort by timestamp - .map { |p| p.split.map(&:to_s) } + relative_paths = Pathname.glob(metadata_timestamped_path(version: "*", timestamp: "*")) + .map { |p| p.relative_path_from(p.parent.parent) } + # https://github.com/sorbet/sorbet/issues/6844 + T.unsafe(relative_paths).sort_by(&:basename) # sort by timestamp + .map { |p| p.split.map(&:to_s) } end def versions @@ -150,7 +165,7 @@ module Cask version_os_hash ensure - MacOS.full_version = actual_version + MacOS.full_version = actual_version || MacOS.full_version.to_s end end diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index d8a6bd41c9..0fa41924df 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "cask/cache" @@ -12,8 +12,18 @@ module Cask module CaskLoader extend Context + module ILoader + extend T::Sig + extend T::Helpers + interface! + + sig { abstract.params(config: Config).returns(Cask) } + def load(config:); end + end + # Loads a cask from a string. class FromContentLoader + include ILoader attr_reader :content, :tap def self.can_load?(ref) @@ -112,7 +122,6 @@ module Cask return false unless ref.to_s.match?(@uri_regex) uri = URI(ref) - return false unless uri return false unless uri.path true @@ -123,7 +132,7 @@ module Cask sig { params(url: T.any(URI::Generic, String)).void } def initialize(url) @url = URI(url) - super Cache.path/File.basename(@url.path) + super Cache.path/File.basename(T.must(@url.path)) end def load(config:) @@ -185,6 +194,7 @@ module Cask # Loads a cask from an existing {Cask} instance. class FromInstanceLoader + include ILoader def self.can_load?(ref) ref.is_a?(Cask) end @@ -200,6 +210,7 @@ module Cask # Loads a cask from the JSON API. class FromAPILoader + include ILoader attr_reader :token, :path def self.can_load?(ref) diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index f33472be57..461b87af5b 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "locale" @@ -196,7 +196,7 @@ module Cask # @api public def url(*args, **options, &block) - caller_location = caller_locations[0] + caller_location = T.must(caller_locations).fetch(0) set_unique_stanza(:url, args.empty? && options.empty? && !block) do if block @@ -237,14 +237,14 @@ module Cask set_unique_stanza(:sha256, should_return) do @on_system_blocks_exist = true if arm.present? || intel.present? - arg ||= on_arch_conditional(arm: arm, intel: intel) - case arg + val = arg || on_arch_conditional(arm: arm, intel: intel) + case val when :no_check - arg + val when String - Checksum.new(arg) + Checksum.new(val) else - raise CaskInvalidError.new(cask, "invalid 'sha256' value: #{arg.inspect}") + raise CaskInvalidError.new(cask, "invalid 'sha256' value: #{val.inspect}") end end end diff --git a/Library/Homebrew/cask/dsl/base.rb b/Library/Homebrew/cask/dsl/base.rb index 9a91b8381f..eb24dfc324 100644 --- a/Library/Homebrew/cask/dsl/base.rb +++ b/Library/Homebrew/cask/dsl/base.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "cask/utils" @@ -26,7 +26,7 @@ module Cask # rubocop:disable Style/MissingRespondToMissing def method_missing(method, *) if method - underscored_class = self.class.name.gsub(/([[:lower:]])([[:upper:]][[:lower:]])/, '\1_\2').downcase + underscored_class = T.must(self.class.name).gsub(/([[:lower:]])([[:upper:]][[:lower:]])/, '\1_\2').downcase section = underscored_class.split("::").last Utils.method_missing_message(method, @cask.to_s, section) nil diff --git a/Library/Homebrew/extend/on_system.rbi b/Library/Homebrew/extend/on_system.rbi new file mode 100644 index 0000000000..15534bb4b4 --- /dev/null +++ b/Library/Homebrew/extend/on_system.rbi @@ -0,0 +1,6 @@ +# typed: strict + +module OnSystem::MacOSOnly + sig { params(arm: T.nilable(String), intel: T.nilable(String)).returns(T.nilable(String)) } + def on_arch_conditional(arm: nil, intel: nil); end +end diff --git a/Library/Homebrew/os/linux.rb b/Library/Homebrew/os/linux.rb index f9e964e307..2b5df8cfc3 100644 --- a/Library/Homebrew/os/linux.rb +++ b/Library/Homebrew/os/linux.rb @@ -82,7 +82,7 @@ module OS nil end - def sdk_path + def sdk_path(_version = nil) nil end diff --git a/Library/Homebrew/os/mac.rb b/Library/Homebrew/os/mac.rb index f1c88ca288..b52a965130 100644 --- a/Library/Homebrew/os/mac.rb +++ b/Library/Homebrew/os/mac.rb @@ -1,4 +1,4 @@ -# typed: false +# typed: true # frozen_string_literal: true require "os/mac/version" @@ -38,7 +38,7 @@ module OS end end - sig { params(version: Version).void } + sig { params(version: String).void } def full_version=(version) @full_version = Version.new(version.chomp) @version = nil From 0fe595f9c2159b1701582597b1e3900b38934380 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 13 Mar 2023 09:08:51 -0700 Subject: [PATCH 2/5] Elaborate on sorbet comments --- Library/Homebrew/cask/cask.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 1a828c28f1..ce2b82519b 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -105,7 +105,7 @@ module Cask @allow_reassignment = allow_reassignment @loaded_from_api = loaded_from_api @loader = loader - # https://github.com/sorbet/sorbet/issues/6843 + # Sorbet has trouble with bound procs assigned to ivars: https://github.com/sorbet/sorbet/issues/6843 instance_variable_set(:@block, block) @default_config = config || Config.new @@ -139,7 +139,7 @@ module Cask def timestamped_versions relative_paths = Pathname.glob(metadata_timestamped_path(version: "*", timestamp: "*")) .map { |p| p.relative_path_from(p.parent.parent) } - # https://github.com/sorbet/sorbet/issues/6844 + # Sorbet is unaware that Pathname is sortable: https://github.com/sorbet/sorbet/issues/6844 T.unsafe(relative_paths).sort_by(&:basename) # sort by timestamp .map { |p| p.split.map(&:to_s) } end From e40d2b42c90521d266c87945294e8b34476364ba Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 13 Mar 2023 09:10:36 -0700 Subject: [PATCH 3/5] Update Library/Homebrew/cask/cask.rb Co-authored-by: Markus Reiter --- Library/Homebrew/cask/cask.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index ce2b82519b..04f68cda0c 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -165,7 +165,7 @@ module Cask version_os_hash ensure - MacOS.full_version = actual_version || MacOS.full_version.to_s + MacOS.full_version = actual_version if actual_version end end From e5a392c2562d51f5e70fd1fe7e44fa1351748a6b Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 13 Mar 2023 10:46:39 -0700 Subject: [PATCH 4/5] T.unsafe is now safe --- Library/Homebrew/cli/named_args.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index b167902f8e..8e43091d0b 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -149,8 +149,7 @@ module Homebrew if want_keg_like_cask cask_version = Cask::Cask.new(name, config: config).versions.first cask = Cask::Cask.new(name, config: config) do - # This block is dynamically evaluated in `Cask::Cask#refresh`, so sorbet cannot typecheck it. - T.unsafe(self).version cask_version if cask_version + self.version cask_version if cask_version end return cask end From 04f725800922dbe06ef46ae5942f21d5e83aae0f Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 13 Mar 2023 11:24:49 -0700 Subject: [PATCH 5/5] brew style --fix --- Library/Homebrew/cli/named_args.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index 8e43091d0b..d87ec0b655 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -149,7 +149,7 @@ module Homebrew if want_keg_like_cask cask_version = Cask::Cask.new(name, config: config).versions.first cask = Cask::Cask.new(name, config: config) do - self.version cask_version if cask_version + version cask_version if cask_version end return cask end