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

utils/ast: add Sorbet method signatures #10241

Merged
merged 1 commit into from Jan 7, 2021
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
1 change: 1 addition & 0 deletions Library/Homebrew/Gemfile
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/Gemfile.lock
Expand Up @@ -183,6 +183,7 @@ DEPENDENCIES
rspec-sorbet
rspec-wait
rubocop
rubocop-ast
rubocop-performance
rubocop-rails
rubocop-rspec
Expand Down
44 changes: 44 additions & 0 deletions 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
4 changes: 3 additions & 1 deletion Library/Homebrew/dev-cmd/bottle.rb
Expand Up @@ -9,7 +9,6 @@
require "cli/parser"
require "utils/inreplace"
require "erb"
require "utils/ast"

BOTTLE_ERB = <<-EOS
bottle do
Expand Down Expand Up @@ -490,6 +489,9 @@ def merge(args:)
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"
Expand Down
4 changes: 3 additions & 1 deletion Library/Homebrew/dev-cmd/bump-revision.rb
Expand Up @@ -3,7 +3,6 @@

require "formula"
require "cli/parser"
require "utils/ast"

module Homebrew
extend T::Sig
Expand Down Expand Up @@ -50,6 +49,9 @@ def bump_revision
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?
Expand Down
45 changes: 2 additions & 43 deletions 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
Expand All @@ -11,50 +12,8 @@ module FormulaAudit
# - `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

Expand Down
67 changes: 51 additions & 16 deletions 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?
[]
Expand All @@ -19,23 +28,35 @@ def body_children(body_node)
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) }
Expand All @@ -46,6 +67,14 @@ def replace_formula_stanza!(formula_contents, name, replacement, type: nil)
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)

Expand All @@ -62,7 +91,7 @@ def add_formula_stanza!(formula_contents, name, value, type: nil)
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|
Expand All @@ -88,7 +117,7 @@ def add_formula_stanza!(formula_contents, name, value, type: nil)
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* +/)
Expand All @@ -97,16 +126,15 @@ def stanza_text(name, value, indent: nil)

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)

Expand All @@ -124,10 +152,9 @@ def process_formula(formula_contents)
[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],
Expand All @@ -142,19 +169,27 @@ def formula_component_before_target?(node, target_name:, target_type: nil)
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)
Expand Down