Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable types in extensions, etc. #15124

Merged
merged 4 commits into from Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions Library/Homebrew/cmd/postgresql-upgrade-database.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

require "cli/parser"
Expand Down Expand Up @@ -60,10 +60,10 @@ def postgresql_upgrade_database

odie "No #{name} #{pg_version_data}.* version installed!" unless old_bin

server_stopped = false
moved_data = false
initdb_run = false
upgraded = false
server_stopped = T.let(false, T::Boolean)
moved_data = T.let(false, T::Boolean)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost wonder if we want to implement our own helper for this. I find it gross how verbose and repetitive this is. Something like SORBET_FALSE might be nicer.

initdb_run = T.let(false, T::Boolean)
upgraded = T.let(false, T::Boolean)

begin
# Following instructions from:
Expand All @@ -88,7 +88,7 @@ def postgresql_upgrade_database
system "#{old_bin}/pg_ctl", "-w", "-D", datadir, "start"
end

initdb_args = []
initdb_args = T.let([], T::Array[String])
locale_settings = %w[
lc_collate
lc_ctype
Expand Down
20 changes: 10 additions & 10 deletions Library/Homebrew/dependencies.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

require "delegate"
Expand All @@ -16,19 +16,19 @@ def initialize(*args)
alias eql? ==

def optional
select(&:optional?)
__getobj__.select(&:optional?)
end

def recommended
select(&:recommended?)
__getobj__.select(&:recommended?)
end

def build
select(&:build?)
__getobj__.select(&:build?)
end

def required
select(&:required?)
__getobj__.select(&:required?)
end

def default
Expand All @@ -37,7 +37,7 @@ def default

sig { returns(String) }
def inspect
"#<#{self.class.name}: #{to_a}>"
"#<#{self.class.name}: #{__getobj__}>"
end
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses the explicit delegator method throughout. Since the delegated object is an array, we don't need to invoke to_a here though.

end

Expand All @@ -52,11 +52,11 @@ def initialize(*args)
end

def <<(other)
if other.is_a?(Comparable)
grep(other.class) do |req|
if other.is_a?(Object) && other.is_a?(Comparable)
__getobj__.grep(other.class) do |req|
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to type guard Object here to make .class type check below.

return self if req > other

delete(req)
__getobj__.delete(req)
end
end
super
Expand All @@ -65,6 +65,6 @@ def <<(other)

sig { returns(String) }
def inspect
"#<#{self.class.name}: {#{to_a.join(", ")}}>"
"#<#{self.class.name}: {#{__getobj__.to_a.join(", ")}}>"
end
end
4 changes: 4 additions & 0 deletions Library/Homebrew/dependencies.rbi
Expand Up @@ -2,6 +2,10 @@

class Dependencies < SimpleDelegator
include Kernel
# This is a workaround to enable `alias eql? ==`
# @see https://github.com/sorbet/sorbet/issues/2378#issuecomment-569474238
sig { params(arg0: BasicObject).returns(T::Boolean) }
def ==(arg0); end
end

class Requirements < SimpleDelegator
Expand Down
6 changes: 6 additions & 0 deletions Library/Homebrew/extend/object.rbi
@@ -0,0 +1,6 @@
# typed: strict

class Object
sig { returns(T::Boolean) }
def present?; end
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary for the return type of PyPi::Package#valid_pypi_package? to typecheck.

end
14 changes: 6 additions & 8 deletions Library/Homebrew/extend/os/linux/dev-cmd/update-test.rb
@@ -1,18 +1,16 @@
# typed: false
# typed: true
# frozen_string_literal: true

module Homebrew
extend T::Sig

module_function

class << self
alias generic_git_tags git_tags
end

def git_tags
tags = generic_git_tags
tags = Utils.popen_read("git tag --list | sort -rV") if tags.blank?
tags
def git_tags
tags = generic_git_tags
tags = Utils.popen_read("git tag --list | sort -rV") if tags.blank?
tags
end
end
end
10 changes: 5 additions & 5 deletions Library/Homebrew/extend/os/linux/diagnostic.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

require "tempfile"
Expand Down Expand Up @@ -46,7 +46,7 @@ def check_tmpdir_executable
f.write "#!/bin/sh\n"
f.chmod 0700
f.close
return if system f.path
return if system T.must(f.path)

<<~EOS
The directory #{HOMEBREW_TEMP} does not permit executing
Expand All @@ -56,12 +56,12 @@ def check_tmpdir_executable
echo 'export HOMEBREW_TEMP=~/tmp' >> #{shell_profile}
EOS
ensure
f.unlink
f&.unlink
end

def check_xdg_data_dirs
return if ENV["XDG_DATA_DIRS"].blank?
return if ENV["XDG_DATA_DIRS"].split("/").include?(HOMEBREW_PREFIX/"share")
xdg_data_dirs = ENV.fetch("XDG_DATA_DIRS", nil)
return if xdg_data_dirs.blank? || xdg_data_dirs.split("/").include?(HOMEBREW_PREFIX/"share")

<<~EOS
Homebrew's share was not found in your XDG_DATA_DIRS but you have
Expand Down
7 changes: 5 additions & 2 deletions Library/Homebrew/extend/os/linux/hardware/cpu.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

module Hardware
Expand Down Expand Up @@ -111,7 +111,10 @@ def features
end

%w[aes altivec avx avx2 lm ssse3 sse4_2].each do |flag|
define_method("#{flag}?") { flags.include? flag }
define_method("#{flag}?") do
T.bind(self, T.class_of(Hardware::CPU))
flags.include? flag
end
end

def sse3?
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/extend/os/mac/diagnostic.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

module Homebrew
Expand Down Expand Up @@ -379,7 +379,7 @@ def check_for_multiple_volumes
real_tmp = tmp.realpath.parent
where_tmp = volumes.which real_tmp
ensure
Dir.delete tmp
Dir.delete tmp.to_s
end
rescue
return
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/extend/os/mac/hardware/cpu.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

require "macho"
Expand Down Expand Up @@ -114,7 +114,7 @@ def arm_family
end
end

def intel_family
def intel_family(_family = nil, _cpu_model = nil)
case sysctl_int("hw.cpufamily")
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to match the arity of the same method in Library/Homebrew/extend/os/linux/hardware/cpu.rb

when 0x73d67300 # Yonah: Core Solo/Duo
:core
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/extend/os/mac/keg.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

class Keg
Expand Down Expand Up @@ -108,7 +108,7 @@ def prepare_debug_symbols
# Needed to make symlink permissions consistent on macOS and Linux for
# reproducible bottles.
def consistent_reproducible_symlink_permissions!
find do |file|
path.find do |file|
File.lchmod 0777, file if file.symlink?
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/predicable.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

module Predicable
Expand Down
5 changes: 5 additions & 0 deletions Library/Homebrew/extend/predicable.rbi
@@ -0,0 +1,5 @@
# typed: strict

module Predicable
requires_ancestor { Class }
end
5 changes: 3 additions & 2 deletions Library/Homebrew/extend/time.rb
@@ -1,16 +1,17 @@
# typed: false
# typed: true
# frozen_string_literal: true

module TimeRemaining
refine Time do
def remaining
T.bind(self, Time)
[0, self - Time.now].max
end

def remaining!
r = remaining

raise Timeout::Error if r <= 0
Kernel.raise Timeout::Error if r <= 0

r
end
Expand Down
8 changes: 4 additions & 4 deletions Library/Homebrew/livecheck/strategy.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

module Homebrew
Expand Down Expand Up @@ -161,7 +161,7 @@ def from_url(url, livecheck_strategy: nil, url_provided: false, regex_provided:
# specifies the strategy and contains a `strategy` block
next if (livecheck_strategy != strategy_symbol) || !block_provided
elsif strategy.const_defined?(:PRIORITY) &&
!strategy::PRIORITY.positive? &&
!strategy.const_get(:PRIORITY).positive? &&
livecheck_strategy != strategy_symbol
Copy link
Sponsor Member Author

@dduugg dduugg Apr 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorbet doesn't support dynamic constant references using the prior format, but i believe this is equivalent

# Ignore strategies with a priority of 0 or lower, unless the
# strategy is specified in the `livecheck` block
Expand All @@ -174,7 +174,7 @@ def from_url(url, livecheck_strategy: nil, url_provided: false, regex_provided:
# Sort usable strategies in descending order by priority, using the
# DEFAULT_PRIORITY when a strategy doesn't contain a PRIORITY constant
usable_strategies.sort_by do |strategy|
(strategy.const_defined?(:PRIORITY) ? -strategy::PRIORITY : -DEFAULT_PRIORITY)
(strategy.const_defined?(:PRIORITY) ? -strategy.const_get(:PRIORITY) : -DEFAULT_PRIORITY)
end
end

Expand Down Expand Up @@ -216,7 +216,7 @@ def self.page_headers(url, homebrew_curl: false)
# @return [Hash]
sig { params(url: String, homebrew_curl: T::Boolean).returns(T::Hash[Symbol, T.untyped]) }
def self.page_content(url, homebrew_curl: false)
stderr = nil
stderr = T.let(nil, T.nilable(String))
[:default, :browser].each do |user_agent|
stdout, stderr, status = curl_with_workarounds(
*PAGE_CONTENT_CURL_ARGS, url,
Expand Down
26 changes: 14 additions & 12 deletions Library/Homebrew/utils/pypi.rb
@@ -1,4 +1,4 @@
# typed: false
# typed: true
# frozen_string_literal: true

# Helper functions for updating PyPI resources.
Expand All @@ -7,8 +7,6 @@
module PyPI
extend T::Sig

module_function

PYTHONHOSTED_URL_PREFIX = "https://files.pythonhosted.org/packages/"
private_constant :PYTHONHOSTED_URL_PREFIX

Expand All @@ -35,13 +33,16 @@ def initialize(package_string, is_url: false)
return
end

@name = package_string
@name, @version = @name.split("==") if @name.include? "=="
if package_string.include? "=="
@name, @version = package_string.split("==")
else
@name = package_string
end

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a slight rearrangement to reduce the number of T.must calls that would otherwise be necessary.

return unless (match = @name.match(/^(.*?)\[(.+)\]$/))
return unless (match = T.must(@name).match(/^(.*?)\[(.+)\]$/))

@name = match[1]
@extras = match[2].split ","
@extras = T.must(match[2]).split ","
end

# Get name, URL, SHA-256 checksum, and latest version for a given PyPI package.
Expand Down Expand Up @@ -87,7 +88,7 @@ def to_s

sig { params(other: Package).returns(T::Boolean) }
def same_package?(other)
@name.tr("_", "-").casecmp(other.name.tr("_", "-")).zero?
T.must(@name.tr("_", "-").casecmp(other.name.tr("_", "-"))).zero?
end

# Compare only names so we can use .include? and .uniq on a Package array
Expand All @@ -109,7 +110,7 @@ def <=>(other)
end

sig { params(url: String, version: T.any(String, Version)).returns(T.nilable(String)) }
def update_pypi_url(url, version)
def self.update_pypi_url(url, version)
package = Package.new url, is_url: true

return unless package.valid_pypi_package?
Expand All @@ -133,8 +134,9 @@ def update_pypi_url(url, version)
ignore_non_pypi_packages: T.nilable(T::Boolean),
).returns(T.nilable(T::Boolean))
}
def update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil, exclude_packages: nil,
print_only: false, silent: false, ignore_non_pypi_packages: false)
def self.update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil,
exclude_packages: nil, print_only: false, silent: false,
ignore_non_pypi_packages: false)

auto_update_list = formula.tap&.pypi_formula_mappings
if auto_update_list.present? && auto_update_list.key?(formula.full_name) &&
Expand Down Expand Up @@ -282,7 +284,7 @@ def update_python_resources!(formula, version: nil, package_name: nil, extra_pac
true
end

def json_to_packages(json_tree, main_package, exclude_packages)
def self.json_to_packages(json_tree, main_package, exclude_packages)
return [] if json_tree.blank?

json_tree.flat_map do |package_json|
Expand Down