From 332778025ae3e59d7a2072306b63870f70a60d16 Mon Sep 17 00:00:00 2001 From: Seeker Date: Wed, 6 Jan 2021 09:11:34 -0800 Subject: [PATCH] utils/ast: add Sorbet method signatures --- Library/Homebrew/Gemfile | 1 + Library/Homebrew/Gemfile.lock | 1 + Library/Homebrew/ast_constants.rb | 44 ++++++++++++ Library/Homebrew/dev-cmd/bottle.rb | 4 +- Library/Homebrew/dev-cmd/bump-revision.rb | 4 +- Library/Homebrew/rubocops/components_order.rb | 45 +------------ Library/Homebrew/utils/ast.rb | 67 ++++++++++++++----- 7 files changed, 105 insertions(+), 61 deletions(-) create mode 100644 Library/Homebrew/ast_constants.rb diff --git a/Library/Homebrew/Gemfile b/Library/Homebrew/Gemfile index 1004258583..afd899baf0 100644 --- a/Library/Homebrew/Gemfile +++ b/Library/Homebrew/Gemfile @@ -15,6 +15,7 @@ gem "rspec-retry", require: false gem "rspec-sorbet", require: false gem "rspec-wait", require: false gem "rubocop", require: false +gem "rubocop-ast", require: false gem "simplecov", require: false gem "sorbet", require: false gem "sorbet-runtime", require: false diff --git a/Library/Homebrew/Gemfile.lock b/Library/Homebrew/Gemfile.lock index 949cee426b..d42305d8a9 100644 --- a/Library/Homebrew/Gemfile.lock +++ b/Library/Homebrew/Gemfile.lock @@ -183,6 +183,7 @@ DEPENDENCIES rspec-sorbet rspec-wait rubocop + rubocop-ast rubocop-performance rubocop-rails rubocop-rspec diff --git a/Library/Homebrew/ast_constants.rb b/Library/Homebrew/ast_constants.rb new file mode 100644 index 0000000000..a957c7ffd0 --- /dev/null +++ b/Library/Homebrew/ast_constants.rb @@ -0,0 +1,44 @@ +# typed: true +# frozen_string_literal: true + +FORMULA_COMPONENT_PRECEDENCE_LIST = [ + [{ name: :include, type: :method_call }], + [{ name: :desc, type: :method_call }], + [{ name: :homepage, type: :method_call }], + [{ name: :url, type: :method_call }], + [{ name: :mirror, type: :method_call }], + [{ name: :version, type: :method_call }], + [{ name: :sha256, type: :method_call }], + [{ name: :license, type: :method_call }], + [{ name: :revision, type: :method_call }], + [{ name: :version_scheme, type: :method_call }], + [{ name: :head, type: :method_call }], + [{ name: :stable, type: :block_call }], + [{ name: :livecheck, type: :block_call }], + [{ name: :bottle, type: :block_call }], + [{ name: :pour_bottle?, type: :block_call }], + [{ name: :head, type: :block_call }], + [{ name: :bottle, type: :method_call }], + [{ name: :keg_only, type: :method_call }], + [{ name: :option, type: :method_call }], + [{ name: :deprecated_option, type: :method_call }], + [{ name: :disable!, type: :method_call }], + [{ name: :deprecate!, type: :method_call }], + [{ name: :depends_on, type: :method_call }], + [{ name: :uses_from_macos, type: :method_call }], + [{ name: :on_macos, type: :block_call }], + [{ name: :on_linux, type: :block_call }], + [{ name: :conflicts_with, type: :method_call }], + [{ name: :skip_clean, type: :method_call }], + [{ name: :cxxstdlib_check, type: :method_call }], + [{ name: :link_overwrite, type: :method_call }], + [{ name: :fails_with, type: :method_call }, { name: :fails_with, type: :block_call }], + [{ name: :go_resource, type: :block_call }, { name: :resource, type: :block_call }], + [{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }], + [{ name: :needs, type: :method_call }], + [{ name: :install, type: :method_definition }], + [{ name: :post_install, type: :method_definition }], + [{ name: :caveats, type: :method_definition }], + [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }], + [{ name: :test, type: :block_call }], +].freeze diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 2aad3b7df4..766146f457 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -9,7 +9,6 @@ require "formula_versions" require "cli/parser" require "utils/inreplace" require "erb" -require "utils/ast" BOTTLE_ERB = <<-EOS bottle do @@ -490,6 +489,9 @@ module Homebrew end if args.write? + Homebrew.install_bundler_gems! + require "utils/ast" + path = Pathname.new((HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]).to_s) checksums = old_checksums(path, bottle_hash, args: args) update_or_add = checksums.nil? ? "add" : "update" diff --git a/Library/Homebrew/dev-cmd/bump-revision.rb b/Library/Homebrew/dev-cmd/bump-revision.rb index ebc5143487..f07a5f9dfc 100644 --- a/Library/Homebrew/dev-cmd/bump-revision.rb +++ b/Library/Homebrew/dev-cmd/bump-revision.rb @@ -3,7 +3,6 @@ require "formula" require "cli/parser" -require "utils/ast" module Homebrew extend T::Sig @@ -50,6 +49,9 @@ module Homebrew end end else + Homebrew.install_bundler_gems! + require "utils/ast" + Utils::Inreplace.inreplace(formula.path) do |s| s = s.inreplace_string if current_revision.zero? diff --git a/Library/Homebrew/rubocops/components_order.rb b/Library/Homebrew/rubocops/components_order.rb index f197379730..148605d1e9 100644 --- a/Library/Homebrew/rubocops/components_order.rb +++ b/Library/Homebrew/rubocops/components_order.rb @@ -1,6 +1,7 @@ # typed: false # frozen_string_literal: true +require "ast_constants" require "rubocops/extend/formula" module RuboCop @@ -11,50 +12,8 @@ module RuboCop # - `component_precedence_list` has component hierarchy in a nested list # where each sub array contains components' details which are at same precedence level class ComponentsOrder < FormulaCop - COMPONENT_PRECEDENCE_LIST = [ - [{ name: :include, type: :method_call }], - [{ name: :desc, type: :method_call }], - [{ name: :homepage, type: :method_call }], - [{ name: :url, type: :method_call }], - [{ name: :mirror, type: :method_call }], - [{ name: :version, type: :method_call }], - [{ name: :sha256, type: :method_call }], - [{ name: :license, type: :method_call }], - [{ name: :revision, type: :method_call }], - [{ name: :version_scheme, type: :method_call }], - [{ name: :head, type: :method_call }], - [{ name: :stable, type: :block_call }], - [{ name: :livecheck, type: :block_call }], - [{ name: :bottle, type: :block_call }], - [{ name: :pour_bottle?, type: :block_call }], - [{ name: :head, type: :block_call }], - [{ name: :bottle, type: :method_call }], - [{ name: :keg_only, type: :method_call }], - [{ name: :option, type: :method_call }], - [{ name: :deprecated_option, type: :method_call }], - [{ name: :disable!, type: :method_call }], - [{ name: :deprecate!, type: :method_call }], - [{ name: :depends_on, type: :method_call }], - [{ name: :uses_from_macos, type: :method_call }], - [{ name: :on_macos, type: :block_call }], - [{ name: :on_linux, type: :block_call }], - [{ name: :conflicts_with, type: :method_call }], - [{ name: :skip_clean, type: :method_call }], - [{ name: :cxxstdlib_check, type: :method_call }], - [{ name: :link_overwrite, type: :method_call }], - [{ name: :fails_with, type: :method_call }, { name: :fails_with, type: :block_call }], - [{ name: :go_resource, type: :block_call }, { name: :resource, type: :block_call }], - [{ name: :patch, type: :method_call }, { name: :patch, type: :block_call }], - [{ name: :needs, type: :method_call }], - [{ name: :install, type: :method_definition }], - [{ name: :post_install, type: :method_definition }], - [{ name: :caveats, type: :method_definition }], - [{ name: :plist_options, type: :method_call }, { name: :plist, type: :method_definition }], - [{ name: :test, type: :block_call }], - ].freeze - def audit_formula(_node, _class_node, _parent_class_node, body_node) - @present_components, @offensive_nodes = check_order(COMPONENT_PRECEDENCE_LIST, body_node) + @present_components, @offensive_nodes = check_order(FORMULA_COMPONENT_PRECEDENCE_LIST, body_node) component_problem @offensive_nodes[0], @offensive_nodes[1] if @offensive_nodes diff --git a/Library/Homebrew/utils/ast.rb b/Library/Homebrew/utils/ast.rb index 47c29bae6e..5c2bed0446 100644 --- a/Library/Homebrew/utils/ast.rb +++ b/Library/Homebrew/utils/ast.rb @@ -1,14 +1,23 @@ -# typed: true +# typed: strict # frozen_string_literal: true +require "ast_constants" +require "rubocop-ast" + module Utils # Helper functions for editing Ruby files. # # @api private module AST + Node = RuboCop::AST::Node + SendNode = RuboCop::AST::SendNode + BlockNode = RuboCop::AST::BlockNode + ProcessedSource = RuboCop::AST::ProcessedSource + class << self extend T::Sig + sig { params(body_node: Node).returns(T::Array[Node]) } def body_children(body_node) if body_node.nil? [] @@ -19,23 +28,35 @@ module Utils end end + sig { params(formula_contents: String).returns(T.nilable(Node)) } def bottle_block(formula_contents) formula_stanza(formula_contents, :bottle, type: :block_call) end + sig { params(formula_contents: String, name: Symbol, type: T.nilable(Symbol)).returns(T.nilable(Node)) } def formula_stanza(formula_contents, name, type: nil) _, children = process_formula(formula_contents) children.find { |child| call_node_match?(child, name: name, type: type) } end + sig { params(formula_contents: String, bottle_output: String).void } def replace_bottle_stanza!(formula_contents, bottle_output) replace_formula_stanza!(formula_contents, :bottle, bottle_output.chomp, type: :block_call) end + sig { params(formula_contents: String, bottle_output: String).void } def add_bottle_stanza!(formula_contents, bottle_output) add_formula_stanza!(formula_contents, :bottle, "\n#{bottle_output.chomp}", type: :block_call) end + sig do + params( + formula_contents: String, + name: Symbol, + replacement: T.any(Numeric, String, Symbol), + type: T.nilable(Symbol), + ).void + end def replace_formula_stanza!(formula_contents, name, replacement, type: nil) processed_source, children = process_formula(formula_contents) stanza_node = children.find { |child| call_node_match?(child, name: name, type: type) } @@ -46,6 +67,14 @@ module Utils formula_contents.replace(tree_rewriter.process) end + sig do + params( + formula_contents: String, + name: Symbol, + value: T.any(Numeric, String, Symbol), + type: T.nilable(Symbol), + ).void + end def add_formula_stanza!(formula_contents, name, value, type: nil) processed_source, children = process_formula(formula_contents) @@ -62,7 +91,7 @@ module Utils else children.first end - preceding_component = preceding_component.last_argument if preceding_component.send_type? + preceding_component = preceding_component.last_argument if preceding_component.is_a?(SendNode) preceding_expr = preceding_component.location.expression processed_source.comments.each do |comment| @@ -88,7 +117,7 @@ module Utils def stanza_text(name, value, indent: nil) text = if value.is_a?(String) _, node = process_source(value) - value if (node.send_type? || node.block_type?) && node.method_name == name + value if (node.is_a?(SendNode) || node.is_a?(BlockNode)) && node.method_name == name end text ||= "#{name} #{value.inspect}" text = text.indent(indent) if indent && !text.match?(/\A\n* +/) @@ -97,16 +126,15 @@ module Utils private + sig { params(source: String).returns([ProcessedSource, Node]) } def process_source(source) - Homebrew.install_bundler_gems! - require "rubocop-ast" - ruby_version = Version.new(HOMEBREW_REQUIRED_RUBY_VERSION).major_minor.to_f - processed_source = RuboCop::AST::ProcessedSource.new(source, ruby_version) + processed_source = ProcessedSource.new(source, ruby_version) root_node = processed_source.ast [processed_source, root_node] end + sig { params(formula_contents: String).returns([ProcessedSource, T::Array[Node]]) } def process_formula(formula_contents) processed_source, root_node = process_source(formula_contents) @@ -124,10 +152,9 @@ module Utils [processed_source, children] end + sig { params(node: Node, target_name: Symbol, target_type: T.nilable(Symbol)).returns(T::Boolean) } def formula_component_before_target?(node, target_name:, target_type: nil) - require "rubocops/components_order" - - RuboCop::Cop::FormulaAudit::ComponentsOrder::COMPONENT_PRECEDENCE_LIST.each do |components| + FORMULA_COMPONENT_PRECEDENCE_LIST.each do |components| return false if components.any? do |component| component_match?(component_name: component[:name], component_type: component[:type], @@ -142,19 +169,27 @@ module Utils false end + sig do + params( + component_name: Symbol, + component_type: Symbol, + target_name: Symbol, + target_type: T.nilable(Symbol), + ).returns(T::Boolean) + end def component_match?(component_name:, component_type:, target_name:, target_type: nil) component_name == target_name && (target_type.nil? || component_type == target_type) end + sig { params(node: Node, name: Symbol, type: T.nilable(Symbol)).returns(T::Boolean) } def call_node_match?(node, name:, type: nil) - node_type = if node.send_type? - :method_call - elsif node.block_type? - :block_call + node_type = case node + when SendNode then :method_call + when BlockNode then :block_call + else return false end - return false if node_type.nil? - component_match?(component_name: T.unsafe(node).method_name, + component_match?(component_name: node.method_name, component_type: node_type, target_name: name, target_type: type)