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

Also check sorbet/rbi/todo.rbi for duplicated shims when running the check-shims command #992

Merged
merged 5 commits into from
Jun 17, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,8 @@ Options:
# Default: sorbet/rbi/shims
[--annotations-rbi-dir=ANNOTATIONS_RBI_DIR] # Path to annotations RBIs
# Default: sorbet/rbi/annotations
[--todo-rbi-file=TODO_RBI_FILE] # Path to the generated todo RBI file
# Default: sorbet/rbi/todo.rbi
[--payload], [--no-payload] # Check shims against Sorbet's payload
# Default: true
-c, [--config=<config file path>] # Path to the Tapioca configuration file
Expand Down Expand Up @@ -792,6 +794,7 @@ check_shims:
dsl_rbi_dir: sorbet/rbi/dsl
shim_rbi_dir: sorbet/rbi/shims
annotations_rbi_dir: sorbet/rbi/annotations
todo_rbi_file: sorbet/rbi/todo.rbi
payload: true
annotations:
sources:
Expand Down
2 changes: 2 additions & 0 deletions lib/tapioca/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,15 @@ def gem(*gems)
option :dsl_rbi_dir, type: :string, desc: "Path to DSL RBIs", default: DEFAULT_DSL_DIR
option :shim_rbi_dir, type: :string, desc: "Path to shim RBIs", default: DEFAULT_SHIM_DIR
option :annotations_rbi_dir, type: :string, desc: "Path to annotations RBIs", default: DEFAULT_ANNOTATIONS_DIR
option :todo_rbi_file, type: :string, desc: "Path to the generated todo RBI file", default: DEFAULT_TODO_FILE
option :payload, type: :boolean, desc: "Check shims against Sorbet's payload", default: true
def check_shims
command = Commands::CheckShims.new(
gem_rbi_dir: options[:gem_rbi_dir],
dsl_rbi_dir: options[:dsl_rbi_dir],
shim_rbi_dir: options[:shim_rbi_dir],
annotations_rbi_dir: options[:annotations_rbi_dir],
todo_rbi_file: options[:todo_rbi_file],
payload: options[:payload]
)
command.execute
Expand Down
11 changes: 7 additions & 4 deletions lib/tapioca/commands/check_shims.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,25 @@ class CheckShims < Command
dsl_rbi_dir: String,
annotations_rbi_dir: String,
shim_rbi_dir: String,
todo_rbi_file: String,
payload: T::Boolean
).void
end
def initialize(gem_rbi_dir:, dsl_rbi_dir:, annotations_rbi_dir:, shim_rbi_dir:, payload:)
def initialize(gem_rbi_dir:, dsl_rbi_dir:, annotations_rbi_dir:, shim_rbi_dir:, todo_rbi_file:, payload:)
super()
@gem_rbi_dir = gem_rbi_dir
@dsl_rbi_dir = dsl_rbi_dir
@annotations_rbi_dir = annotations_rbi_dir
@shim_rbi_dir = shim_rbi_dir
@todo_rbi_file = todo_rbi_file
@payload = payload
end

sig { override.void }
def execute
index = RBI::Index.new

if !Dir.exist?(@shim_rbi_dir) || Dir.empty?(@shim_rbi_dir)
if (!Dir.exist?(@shim_rbi_dir) || Dir.empty?(@shim_rbi_dir)) && !File.exist?(@todo_rbi_file)
say("No shim RBIs to check", :green)
exit(0)
end
Expand Down Expand Up @@ -59,12 +61,13 @@ def execute
end
end

index_rbi(index, "todo", @todo_rbi_file)
index_rbis(index, "shim", @shim_rbi_dir)
index_rbis(index, "gem", @gem_rbi_dir)
index_rbis(index, "dsl", @dsl_rbi_dir)
index_rbis(index, "annotation", @annotations_rbi_dir)

duplicates = duplicated_nodes_from_index(index, @shim_rbi_dir)
duplicates = duplicated_nodes_from_index(index, shim_rbi_dir: @shim_rbi_dir, todo_rbi_file: @todo_rbi_file)
unless duplicates.empty?
duplicates.each do |key, nodes|
say_error("\nDuplicated RBI for #{key}:", :red)
Expand All @@ -76,7 +79,7 @@ def execute
say_error(" * #{loc_string}", :red)
end
end
say_error("\nPlease remove the duplicated definitions from the #{@shim_rbi_dir} directory.", :red)
say_error("\nPlease remove the duplicated definitions from #{@shim_rbi_dir} and #{@todo_rbi_file}", :red)
exit(1)
end

Expand Down
72 changes: 50 additions & 22 deletions lib/tapioca/helpers/rbi_files_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ def index_payload(index, dir)
say(" Done", :green)
end

sig { params(index: RBI::Index, kind: String, file: String).void }
def index_rbi(index, kind, file)
return unless File.exist?(file)

say("Loading #{kind} RBIs from #{file}... ")
parse_and_index_file(index, file)
say(" Done", :green)
end

sig { params(index: RBI::Index, kind: String, dir: String).void }
def index_rbis(index, kind, dir)
return unless Dir.exist?(dir) && !Dir.empty?(dir)
Expand All @@ -29,13 +38,19 @@ def index_rbis(index, kind, dir)
say(" Done", :green)
end

sig { params(index: RBI::Index, shim_rbi_dir: String).returns(T::Hash[String, T::Array[RBI::Node]]) }
def duplicated_nodes_from_index(index, shim_rbi_dir)
sig do
params(
index: RBI::Index,
shim_rbi_dir: String,
todo_rbi_file: String
).returns(T::Hash[String, T::Array[RBI::Node]])
end
def duplicated_nodes_from_index(index, shim_rbi_dir:, todo_rbi_file:)
duplicates = {}
say("Looking for duplicates... ")
index.keys.each do |key|
nodes = index[key]
next unless shims_have_duplicates?(nodes, shim_rbi_dir)
next unless shims_or_todos_have_duplicates?(nodes, shim_rbi_dir: shim_rbi_dir, todo_rbi_file: todo_rbi_file)

duplicates[key] = nodes
end
Expand Down Expand Up @@ -137,31 +152,39 @@ def validate_rbi_files(command:, gem_dir:, dsl_dir:, auto_strictness:, gems: [],

sig { params(index: RBI::Index, files: T::Array[String]).void }
def parse_and_index_files(index, files)
trees = files.map do |file|
RBI::Parser.parse_file(file)
rescue RBI::ParseError => e
say_error("\nWarning: #{e} (#{e.location})", :yellow)
end.compact
index.visit_all(trees)
files.each do |file|
parse_and_index_file(index, file)
end
end

sig { params(index: RBI::Index, file: String).void }
def parse_and_index_file(index, file)
tree = RBI::Parser.parse_file(file)
index.visit(tree)
rescue RBI::ParseError => e
say_error("\nWarning: #{e} (#{e.location})", :yellow)
end

sig { params(nodes: T::Array[RBI::Node], shim_rbi_dir: String).returns(T::Boolean) }
def shims_have_duplicates?(nodes, shim_rbi_dir)
sig { params(nodes: T::Array[RBI::Node], shim_rbi_dir: String, todo_rbi_file: String).returns(T::Boolean) }
def shims_or_todos_have_duplicates?(nodes, shim_rbi_dir:, todo_rbi_file:)
return false if nodes.size == 1

shims = extract_shims(nodes, shim_rbi_dir)
return false if shims.empty?
shims_or_todos = extract_shims_and_todos(nodes, shim_rbi_dir: shim_rbi_dir, todo_rbi_file: todo_rbi_file)
return false if shims_or_todos.empty?

props = extract_methods_and_attrs(shims)
shims_or_todos_empty_scopes = extract_empty_scopes(shims_or_todos)
return true unless shims_or_todos_empty_scopes.empty?

props = extract_methods_and_attrs(shims_or_todos)
return false if props.empty?

shims_with_sigs = extract_nodes_with_sigs(props)
shims_with_sigs.each do |shim|
shim_sigs = shim.sigs
shims_or_todos_with_sigs = extract_nodes_with_sigs(props)
shims_or_todos_with_sigs.each do |shim_or_todo|
shims_or_todos_sigs = shim_or_todo.sigs

extract_methods_and_attrs(nodes).each do |node|
next if node == shim
return true if shim_sigs.all? { |sig| node.sigs.include?(sig) }
next if node == shim_or_todo
return true if shims_or_todos_sigs.all? { |sig| node.sigs.include?(sig) }
end

return false
Expand All @@ -170,13 +193,18 @@ def shims_have_duplicates?(nodes, shim_rbi_dir)
true
end

sig { params(nodes: T::Array[RBI::Node], shim_rbi_dir: String).returns(T::Array[RBI::Node]) }
def extract_shims(nodes, shim_rbi_dir)
sig { params(nodes: T::Array[RBI::Node], shim_rbi_dir: String, todo_rbi_file: String).returns(T::Array[RBI::Node]) }
def extract_shims_and_todos(nodes, shim_rbi_dir:, todo_rbi_file:)
nodes.select do |node|
node.loc&.file&.start_with?(shim_rbi_dir)
node.loc&.file&.start_with?(shim_rbi_dir) || node.loc&.file == todo_rbi_file
end
end

sig { params(nodes: T::Array[RBI::Node]).returns(T::Array[RBI::Scope]) }
def extract_empty_scopes(nodes)
T.cast(nodes.select { |node| node.is_a?(RBI::Scope) && node.empty? }, T::Array[RBI::Scope])
end

sig { params(nodes: T::Array[RBI::Node]).returns(T::Array[T.any(RBI::Method, RBI::Attr)]) }
def extract_methods_and_attrs(nodes)
T.cast(nodes.select do |node|
Expand Down
3 changes: 0 additions & 3 deletions sorbet/rbi/shims/error_highlight.rbi

This file was deleted.

1 change: 0 additions & 1 deletion sorbet/rbi/shims/rdoc.rbi
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# typed: true

module RDoc; end
class RDoc::Task; end
Loading