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

Add crystal tool dependencies #13613

Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions src/compiler/crystal/command.cr
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class Crystal::Command
expand show macro expansion for given location
format format project, directories and/or files
hierarchy show type hierarchy
dependencies show file dependency tree
implementations show implementations for given call in location
types show type of main variables
--help, -h show this help
Expand Down Expand Up @@ -181,6 +182,9 @@ class Crystal::Command
when "hierarchy".starts_with?(tool)
options.shift
hierarchy
when "dependencies".starts_with?(tool)
options.shift
dependencies
when "implementations".starts_with?(tool)
options.shift
implementations
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/crystal/compiler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ module Crystal
# Whether to link statically
property? static = false

property dependency_printer : DependencyPrinter? = nil

# Program that was created for the last compilation.
property! program : Program

Expand Down
16 changes: 16 additions & 0 deletions src/compiler/crystal/program.cr
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,22 @@ module Crystal
recorded_requires << RecordedRequire.new(filename, relative_to)
end

def run_requires(node : Require, filenames) : Nil
dependency_printer = compiler.try(&.dependency_printer)

filenames.each do |filename|
unseen_file = requires.add?(filename)

dependency_printer.try(&.enter_file(filename, unseen_file))

if unseen_file
yield filename
end

dependency_printer.try(&.leave_file)
end
end

# Finds *filename* in the configured CRYSTAL_PATH for this program,
# relative to *relative_to*.
def find_in_path(filename, relative_to = nil) : Array(String)?
Expand Down
41 changes: 23 additions & 18 deletions src/compiler/crystal/semantic/semantic_visitor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,11 @@ abstract class Crystal::SemanticVisitor < Crystal::Visitor

if filenames
nodes = Array(ASTNode).new(filenames.size)
filenames.each do |filename|
if @program.requires.add?(filename)
parser = @program.new_parser(File.read(filename))
parser.filename = filename
parser.wants_doc = @program.wants_doc?
begin
parsed_nodes = parser.parse
parsed_nodes = @program.normalize(parsed_nodes, inside_exp: inside_exp?)
# We must type the node immediately, in case a file requires another
# *before* one of the files in `filenames`
parsed_nodes.accept self
rescue ex : CodeError
node.raise "while requiring \"#{node.string}\"", ex
rescue ex
raise Error.new "while requiring \"#{node.string}\"", ex
end
nodes << FileNode.new(parsed_nodes, filename)
end

@program.run_requires(node, filenames) do |filename|
nodes << require_file(node, filename)
end

expanded = Expressions.from(nodes)
else
expanded = Nop.new
Expand All @@ -98,6 +84,25 @@ abstract class Crystal::SemanticVisitor < Crystal::Visitor
false
end

private def require_file(node : Require, filename : String)
parser = @program.new_parser(File.read(filename))
parser.filename = filename
parser.wants_doc = @program.wants_doc?
begin
parsed_nodes = parser.parse
parsed_nodes = @program.normalize(parsed_nodes, inside_exp: inside_exp?)
# We must type the node immediately, in case a file requires another
# *before* one of the files in `filenames`
parsed_nodes.accept self
rescue ex : CodeError
node.raise "while requiring \"#{node.string}\"", ex
rescue ex
raise Error.new "while requiring \"#{node.string}\"", ex
end

FileNode.new(parsed_nodes, filename)
end

def visit(node : ClassDef)
check_outside_exp node, "declare class"
pushing_type(node.resolved_type) do
Expand Down
60 changes: 60 additions & 0 deletions src/compiler/crystal/tools/dependencies.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
require "set"
require "colorize"
require "../syntax/ast"

class Crystal::Command
private def dependencies
dependency_printer = nil
option_parser = parse_with_crystal_opts do |opts|
opts.on("-f FORMAT", "--format FORMAT", "Format the output. Available options: flat, tree (default)") do |format|
case format
when "flat"
dependency_printer = DependencyPrinter.new(STDOUT, flat: true)
when "tree"
dependency_printer = DependencyPrinter.new(STDOUT, flat: false)
else
abort "Invalid format: #{format}"
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
end
end
end

config = create_compiler "tool dependencies", no_codegen: true
config.compiler.no_codegen = true

config.compiler.dependency_printer = dependency_printer || DependencyPrinter.new(STDOUT)
config.compile
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be expressed as an extra parameter of Crystal::Command#compile_no_codegen? In that case the command-line argument handling would be done inside #create_compiler itself, similar to hierarchy

Copy link
Member Author

@straight-shoota straight-shoota Jun 30, 2023

Choose a reason for hiding this comment

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

What would be the benefit? It seems to me this is a pretty simple setup. Yes, it's some duplicate code. But that's not bad. Combining everthing in one place adds quite a bit of complexity.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't run this branch yet, but I believe this means the tool doesn't have access to any of the common command-line options that other tools have (-D, --prelude etc.)

Copy link
Contributor

@HertzDevil HertzDevil Jun 30, 2023

Choose a reason for hiding this comment

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

There is indeed a problem with this approach: crystal tool dependencies still shows the full help rather than this help, but the option parser only ever accepts this -f, and not the actually shown options like -h. In particular this -f tree|flat conflicts with the other -f text|json.

Copy link
Contributor

Choose a reason for hiding this comment

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

If crystal tool hierarchy accepts -D and --prelude, so should this tool; this is not merely a matter of code duplication

Copy link
Member Author

Choose a reason for hiding this comment

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

The actual common piece that should be reused for a single option parser instance is create_compiler. Utilizing compile_no_codegen would add unnecessary complexity for no real gain.

end
end

module Crystal
class DependencyPrinter
@indent = 0

def initialize(@io : IO, @flat : Bool = false)
end

def enter_file(filename : String, unseen : Bool)
unless unseen
@indent += 1
return
end
print_indent
print_file(filename)

@indent += 1
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
end

def leave_file
@indent -= 1
end

private def print_indent
return if @flat
@io.print " " * @indent.clamp(0..)
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
end

private def print_file(filename)
@io.puts ::Path[filename].relative_to?(Dir.current) || filename
end
end
end