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

Remove use of hidden-definitions #16586

Merged
merged 15 commits into from
Feb 12, 2024
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
/Library/Taps
/Library/PinnedTaps
/Library/Homebrew/.byebug_history
/Library/Homebrew/sorbet/rbi/hidden-definitions/errors.txt

# Ignore Bundler files
**/.bundle/bin
Expand Down
2 changes: 1 addition & 1 deletion Library/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ AllCops:
- "**/*.rbi"
Exclude:
- "Homebrew/sorbet/rbi/gems/**/*.rbi"
- "Homebrew/sorbet/rbi/hidden-definitions/*.rbi"
- "Homebrew/bin/*"
- "Homebrew/vendor/**/*"
- "Taps/*/*/vendor/**/*"
Expand Down Expand Up @@ -69,6 +68,7 @@ Homebrew/NegateInclude:
# `exclude?` is not available here:
- "Homebrew/standalone/init.rb"
- "Homebrew/rubocops/**/*"
- "Homebrew/sorbet/tapioca/**/*"

# `system` is a special case and aligns on second argument, so allow this for formulae.
Layout/ArgumentAlignment:
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ group :tests, optional: true do
gem "simplecov-cobertura", require: false
end
group :typecheck, optional: true do
gem "method_source", require: false
reitermarkus marked this conversation as resolved.
Show resolved Hide resolved
gem "parlour", require: false
gem "sorbet-static-and-runtime", require: false
gem "spoom", require: false
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ DEPENDENCIES
bootsnap
byebug
json_schemer
method_source
minitest
parallel_tests
parlour
Expand Down
17 changes: 4 additions & 13 deletions Library/Homebrew/dev-cmd/typecheck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ def self.typecheck

HOMEBREW_LIBRARY_PATH.cd do
if update
safe_system "bundle", "exec", "tapioca", "dsl"
# RBI files already provided by Sorbet:
excluded_gems = [
"json", # RBI file is already provided by Sorbet
"json",
"msgpack",
]
tapioca_args = ["--exclude", *excluded_gems, "--pre", "sorbet/tapioca/prerequire.rb"]
tapioca_args << "--all" if args.update_all?
Expand All @@ -59,18 +62,6 @@ def self.typecheck
safe_system "bundle", "exec", "tapioca", "gem", *tapioca_args
safe_system "bundle", "exec", "parlour"

safe_system({ "RUBYLIB" => "#{HOMEBREW_LIBRARY_PATH}/sorbet/hidden_definitions_hacks" },
"bundle", "exec", "srb", "rbi", "hidden-definitions")
# HACK: we'll phase out hidden-definitions soon
tmp_file = "sorbet/rbi/hidden-definitions/hidden.rbi.tmp"
orig_file = "sorbet/rbi/hidden-definitions/hidden.rbi"
File.open(tmp_file, "w") do |out_file|
File.foreach(orig_file) do |line|
out_file.puts line unless line.include?("def self.new(*args, **arg, &blk); end")
end
end
File.rename(tmp_file, orig_file)

if args.suggest_typed?
ohai "Bumping Sorbet `typed` sigils..."
# --sorbet needed because of https://github.com/Shopify/spoom/issues/488
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/ENV/shared.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def reset
end
private :reset

sig { returns(T::Hash[String, String]) }
sig { returns(T::Hash[String, T.nilable(String)]) }
def remove_cc_etc
keys = %w[CC CXX OBJC OBJCXX LD CPP CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS LDFLAGS CPPFLAGS]
keys.to_h { |key| [key, delete(key)] }
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/formula_cellar_checks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,11 @@ def check_binary_arches(formula)
else
true
end
return if mismatches.empty? && universal_binaries_expected
return if T.must(mismatches).empty? && universal_binaries_expected

mismatches_expected = formula.tap.blank? ||
formula.tap.audit_exception(:mismatched_binary_allowlist, formula.name)
return if compatible_universal_binaries.empty? && mismatches_expected
return if T.must(compatible_universal_binaries).empty? && mismatches_expected
dduugg marked this conversation as resolved.
Show resolved Hide resolved

return if universal_binaries_expected && mismatches_expected

Expand Down
6 changes: 0 additions & 6 deletions Library/Homebrew/rubocops/cask/extend/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,11 @@ class Node
include RuboCop::Cask::Constants

def_node_matcher :method_node, "{$(send ...) (block $(send ...) ...)}"
def_node_matcher :block_args, "(block _ $_ _)"
reitermarkus marked this conversation as resolved.
Show resolved Hide resolved
def_node_matcher :block_body, "(block _ _ $_)"

def_node_matcher :key_node, "{(pair $_ _) (hash (pair $_ _) ...)}"
def_node_matcher :val_node, "{(pair _ $_) (hash (pair _ $_) ...)}"

def_node_matcher :cask_block?, "(block (send nil? :cask ...) args ...)"
def_node_matcher :on_system_block?,
"(block (send nil? {#{ON_SYSTEM_METHODS.map(&:inspect).join(" ")}} ...) args ...)"
def_node_matcher :arch_variable?, "(lvasgn _ (send nil? :on_arch_conditional ...))"

def_node_matcher :begin_block?, "(begin ...)"

sig { returns(T::Boolean) }
Expand Down
21 changes: 21 additions & 0 deletions Library/Homebrew/rubocops/cask/extend/node.rbi
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# typed: strict

class RuboCop::AST::Node
dduugg marked this conversation as resolved.
Show resolved Hide resolved
sig { returns(T.nilable(RuboCop::AST::SendNode)) }
def method_node; end

sig { returns(T.nilable(RuboCop::AST::Node)) }
reitermarkus marked this conversation as resolved.
Show resolved Hide resolved
def block_body; end

sig { returns(T::Boolean) }
def cask_block?; end

sig { returns(T::Boolean) }
def on_system_block?; end

sig { returns(T::Boolean) }
def arch_variable?; end

sig { returns(T::Boolean) }
def begin_block?; end
end
2 changes: 1 addition & 1 deletion Library/Homebrew/rubocops/cask/mixin/cask_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Cop
module Cask
# Common functionality for cops checking casks.
module CaskHelp
prepend CommentsHelp
prepend CommentsHelp # Update the rbi file if changing this: https://github.com/sorbet/sorbet/issues/259

sig { overridable.params(cask_block: RuboCop::Cask::AST::CaskBlock).void }
def on_cask(cask_block); end
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/rubocops/cask/mixin/cask_help.rbi
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# typed: strict

module RuboCop::Cop::Cask::CaskHelp
# Sorbet doesn't understand `prepend`: https://github.com/sorbet/sorbet/issues/259
include RuboCop::Cop::CommentsHelp
requires_ancestor { RuboCop::Cop::Base }
end
2 changes: 1 addition & 1 deletion Library/Homebrew/software_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def depends_on(spec)
def uses_from_macos(deps, bounds = {})
if deps.is_a?(Hash)
bounds = deps.dup
deps = [bounds.shift].to_h
deps = [T.unsafe(bounds).shift].to_h
Copy link
Member

Choose a reason for hiding this comment

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

Can we just add a sig here instead?

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.

LMK if you have ideas, but i think it gets gnarly with type parameters and union types (and I think it would need to be in an rbi file due to the former). I think it's something I'd prefer to leave to a future PR (I'm thinking of doing a sweep over the T.unsafe call sites at some point anyway).

Copy link
Member

Choose a reason for hiding this comment

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

I think you can use multiple sigs.

Copy link
Member

Choose a reason for hiding this comment

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

sig { params(deps: String, bounds: T::Hash[String, T.any(Symbol, T::Array[Symbol])]).void }
sig { params(deps: T::Hash[String, T.any(Symbol, T::Array[Symbol])]).void }
def uses_from_macos(deps, bounds = {})

Actually, I think we don't even need the bounds parameter:

sig { params(deps: T.any(String, T::Hash[String, T.any(Symbol, T::Array[Symbol])])).void }
def uses_from_macos(deps)

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.

Overloading a method (by providing multiple signatures for the method) is only allowed for methods defined in the Ruby standard library.

Also, all params must be present in a sig.

(As a favor to the PR author, it would be appreciated to validate suggested changes before publishing them. Again, I also worry that there is excessive focus on this line, given the scope of this PR, and it may be better to refine in a future PR.)

Copy link
Member

Choose a reason for hiding this comment

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

As a favor to the PR author, it would be appreciated to validate suggested changes before publishing them.

I usually do, however I reviewed this on mobile.

end

spec, tags = deps.is_a?(Hash) ? deps.first : deps
Expand Down
4 changes: 0 additions & 4 deletions Library/Homebrew/sorbet/hidden_definitions_hacks/webrick.rb

This file was deleted.

1 change: 1 addition & 0 deletions Library/Homebrew/sorbet/rbi/dsl/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**/*.rbi linguist-generated=true
reitermarkus marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 17 additions & 0 deletions Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/cask/variables.rbi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.