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

Bump more files to Sorbet typed: strict #18354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Library/Homebrew/cask/migrator.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# typed: true # rubocop:todo Sorbet/StrictSigil
# typed: strict
# frozen_string_literal: true

require "cask/cask_loader"
require "utils/inreplace"

module Cask
class Migrator
sig { returns(Cask) }
attr_reader :old_cask, :new_cask

sig { params(old_cask: Cask, new_cask: Cask).void }
Expand Down Expand Up @@ -38,7 +39,9 @@ def migrate(dry_run: false)
old_caskroom_path = old_cask.caskroom_path
new_caskroom_path = new_cask.caskroom_path

old_installed_caskfile = old_cask.installed_caskfile.relative_path_from(old_caskroom_path)
return if (old_caskfile = old_cask.installed_caskfile).nil?
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return if (old_caskfile = old_cask.installed_caskfile).nil?
old_caskfile = old_cask.installed_caskfile
return if old_caskfile.nil?

Don't feel strongly but this format makes more sense to me when the variable's reused below so the definition is not "hidden".


old_installed_caskfile = old_caskfile.relative_path_from(old_caskroom_path)
new_installed_caskfile = old_installed_caskfile.dirname/old_installed_caskfile.basename.sub(
old_token,
new_token,
Expand Down
9 changes: 6 additions & 3 deletions Library/Homebrew/formula_cellar_checks.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# typed: true # rubocop:todo Sorbet/StrictSigil
# typed: strict
# frozen_string_literal: true

require "utils/shell"
Expand Down Expand Up @@ -191,6 +191,7 @@ def check_elisp_root(share, name)
EOS
end

sig { params(lib: Pathname, deps: T::Array[Formula]).returns(T.nilable(String)) }
def check_python_packages(lib, deps)
return unless lib.directory?

Expand Down Expand Up @@ -250,6 +251,7 @@ def check_shim_references(prefix)
EOS
end

sig { params(prefix: Pathname, plist: Pathname).returns(T.nilable(String)) }
def check_plist(prefix, plist)
return unless prefix.directory?

Expand Down Expand Up @@ -412,7 +414,7 @@ def check_binary_arches(formula)

sig { void }
def audit_installed
@new_formula ||= false
@new_formula ||= T.let(false, T.nilable(T::Boolean))

problem_if_output(check_manpages)
problem_if_output(check_infopages)
Expand Down Expand Up @@ -442,8 +444,9 @@ def relative_glob(dir, pattern)
File.directory?(dir) ? Dir.chdir(dir) { Dir[pattern] } : []
end

sig { params(file: T.any(Pathname, String), objdump: String).returns(T::Boolean) }
def cpuid_instruction?(file, objdump = "objdump")
Comment on lines +447 to 448
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sig { params(file: T.any(Pathname, String), objdump: String).returns(T::Boolean) }
def cpuid_instruction?(file, objdump = "objdump")
sig { params(file: T.any(Pathname, String), objdump: Pathname).returns(T::Boolean) }
def cpuid_instruction?(file, objdump)

Copy link
Member

Choose a reason for hiding this comment

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

The Pathname part is actually necessary since callers above pass objdump as a Pathname.

@instruction_column_index ||= {}
@instruction_column_index ||= T.let({}, T.nilable(T::Hash[String, T.untyped]))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@instruction_column_index ||= T.let({}, T.nilable(T::Hash[String, T.untyped]))
@instruction_column_index ||= T.let({}, T.nilable(T::Hash[String, Integer]))

I think?

@instruction_column_index[objdump] ||= begin
objdump_version = Utils.popen_read(objdump, "--version")

Expand Down
28 changes: 26 additions & 2 deletions Library/Homebrew/help.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# typed: true # rubocop:todo Sorbet/StrictSigil
# typed: strict
# frozen_string_literal: true

require "cli/parser"
Expand All @@ -7,6 +7,14 @@
module Homebrew
# Helper module for printing help output.
module Help
sig {
params(
cmd: T.nilable(String),
empty_argv: T::Boolean,
usage_error: T.nilable(String),
remaining_args: T.nilable(T::Array[String])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
remaining_args: T.nilable(T::Array[String])
remaining_args: T::Array[String])

feels nicer if possible given the default parameter is []

).void
}
def self.help(cmd = nil, empty_argv: false, usage_error: nil, remaining_args: [])
if cmd.nil?
# Handle `brew` (no arguments).
Expand Down Expand Up @@ -39,6 +47,13 @@ def self.help(cmd = nil, empty_argv: false, usage_error: nil, remaining_args: []
exit 0
end

sig {
params(
cmd: String,
path: Pathname,
remaining_args: T.nilable(T::Array[String]),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
remaining_args: T.nilable(T::Array[String]),
remaining_args: T::Array[String],

feels nicer if possible given there's no default parameter

).returns(String)
}
def self.command_help(cmd, path, remaining_args:)
# Only some types of commands can have a parser.
output = if Commands.valid_internal_cmd?(cmd) ||
Expand All @@ -58,25 +73,34 @@ def self.command_help(cmd, path, remaining_args:)
end
private_class_method :command_help

sig {
params(
path: Pathname,
remaining_args: T.nilable(T::Array[String])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
remaining_args: T.nilable(T::Array[String])
remaining_args: T::Array[String]

).returns(T.nilable(String))
}
def self.parser_help(path, remaining_args:)
# Let OptionParser generate help text for commands which have a parser.
cmd_parser = CLI::Parser.from_cmd_path(path)
return unless cmd_parser
return unless remaining_args

# Try parsing arguments here in order to show formula options in help output.
cmd_parser.parse(remaining_args, ignore_invalid_options: true)
cmd_parser.generate_help_text
end
private_class_method :parser_help

sig { params(path: Pathname).returns(T::Array[T.nilable(String)]) }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sig { params(path: Pathname).returns(T::Array[T.nilable(String)]) }
sig { params(path: Pathname).returns(T::Array[String]) }

def self.command_help_lines(path)
path.read
.lines
.grep(/^#:/)
.map { |line| line.slice(2..-1).delete_prefix(" ") }
.map { |line| line.slice(2..-1)&.delete_prefix(" ") }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.map { |line| line.slice(2..-1)&.delete_prefix(" ") }
.filter_map { |line| line.slice(2..-1)&.delete_prefix(" ") }

means it'll be T::Array[String]

end
private_class_method :command_help_lines

sig { params(path: Pathname).returns(T.nilable(String)) }
def self.comment_help(path)
# Otherwise read #: lines from the file.
help_lines = command_help_lines(path)
Expand Down
Loading