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

Conversation

dduugg
Copy link
Sponsor Member

@dduugg dduugg commented Apr 2, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Enables typing in some extend files and elsewhere. After this change, ~15 non-test files will remain untyped.

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

sig { returns(String) }
def inspect
"#<#{self.class.name}: #{to_a}>"
"#<#{self.class.name}: #{__getobj__}>"
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.

@@ -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)
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.


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.

@@ -114,7 +114,7 @@ def arm_family
end
end

def intel_family
def intel_family(_family = nil, _cpu_model = nil)
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

@@ -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|
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.

@@ -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? &&
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

@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.

moved_data = false
initdb_run = false
upgraded = false
server_stopped = 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.

@MikeMcQuaid MikeMcQuaid merged commit 7b4c09f into Homebrew:master Apr 3, 2023
24 checks passed
@MikeMcQuaid
Copy link
Member

Thanks again @dduugg, nice work!

@dduugg dduugg deleted the enable-types branch April 3, 2023 17:39
@github-actions github-actions bot added the outdated PR was locked due to age label May 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants