From 91c8a3f5148893c23b078832ab063ee81e005405 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 20 Oct 2018 03:17:16 +0200 Subject: [PATCH 01/11] Add `read_file` macro method --- spec/compiler/macro/macro_methods_spec.cr | 14 ++++++++++++++ src/compiler/crystal/macros.cr | 11 +++++++++++ src/compiler/crystal/macros/methods.cr | 16 ++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/spec/compiler/macro/macro_methods_spec.cr b/spec/compiler/macro/macro_methods_spec.cr index 67073205e8b6..355bbf82cf25 100644 --- a/spec/compiler/macro/macro_methods_spec.cr +++ b/spec/compiler/macro/macro_methods_spec.cr @@ -1757,4 +1757,18 @@ module Crystal end end end + + describe "read_file" do + it "reads file (exists)" do + run(%q< + {{read_file("#{__DIR__}/../data/build")}} + >, filename = __FILE__).to_string.should eq(File.read("#{__DIR__}/../data/build")) + end + + it "reads file (doesn't exists)" do + run(%q< + {{read_file("#{__DIR__}/../data/build_foo")}} ? 10 : 20 + >, filename = __FILE__).to_i.should eq(20) + end + end end diff --git a/src/compiler/crystal/macros.cr b/src/compiler/crystal/macros.cr index d06e57b4d65d..5f8b975d5564 100644 --- a/src/compiler/crystal/macros.cr +++ b/src/compiler/crystal/macros.cr @@ -67,6 +67,17 @@ module Crystal::Macros def raise(message) : NoReturn end + # Reads a file: if it exists, returns a StringLiteral with its contents; + # otherwise `nil` is returned. + # + # To read a file relative to where the macro is defined, use: + # + # ``` + # read_file("#{__DIR__}/some_file.txt") + # ``` + def read_file(filename) : StringLiteral | NilLiteral + end + # Compiles and execute a Crystal program and returns its output # as a `MacroId`. # diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 36c47e32ee55..56ed8722459e 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -30,6 +30,8 @@ module Crystal interpret_system(node) when "raise" interpret_raise(node) + when "read_file" + interpret_read_file(node) when "run" interpret_run(node) else @@ -142,6 +144,20 @@ module Crystal raise SkipMacroException.new(@str.to_s) end + def interpret_read_file(node) + if node.args.size == 1 + node.args[0].accept self + filename = @last.to_macro_id + if File.exists?(filename) + @last = StringLiteral.new(File.read(filename)) + else + @last = NilLiteral.new + end + else + node.wrong_number_of_arguments "macro call 'read_file'", node.args.size, 1 + end + end + def interpret_system(node) cmd = node.args.map do |arg| arg.accept self From 9200e3865e4a755ce9b237abbfbe0b351a7e319b Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 4 Nov 2018 23:04:27 +0100 Subject: [PATCH 02/11] Refactor run & read_file macros, as per @asterite's review --- src/compiler/crystal/macros/methods.cr | 57 ++++++++++++++------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 56ed8722459e..5592a77c66d6 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -145,17 +145,18 @@ module Crystal end def interpret_read_file(node) - if node.args.size == 1 - node.args[0].accept self - filename = @last.to_macro_id - if File.exists?(filename) - @last = StringLiteral.new(File.read(filename)) - else - @last = NilLiteral.new - end - else + unless node.args.size == 1 node.wrong_number_of_arguments "macro call 'read_file'", node.args.size, 1 end + + node.args[0].accept self + filename = @last.to_macro_id + filename = find_file(filename) { } + if filename + @last = StringLiteral.new(File.read(filename)) + else + @last = NilLiteral.new + end end def interpret_system(node) @@ -179,44 +180,48 @@ module Crystal macro_raise(node, node.args, self) end - def interpret_run(node) - if node.args.size == 0 - node.wrong_number_of_arguments "macro call 'run'", 0, "1+" - end - - node.args.first.accept self - filename = @last.to_macro_id - original_filename = filename - + private def find_file(filename) # Support absolute paths - if filename.starts_with?("/") - filename = "#{filename}.cr" unless filename.ends_with?(".cr") - + if filename.starts_with?('/') if File.exists?(filename) unless File.file?(filename) - node.raise "error executing macro run: '#{filename}' is not a file" + return yield Exception.new "#{filename.inspect} is not a file" end else - node.raise "error executing macro run: can't find file '#{filename}'" + return yield Exception.new "can't find file #{filename.inspect}" end else begin relative_to = @location.try &.original_filename found_filenames = @program.find_in_path(filename, relative_to) rescue ex - node.raise "error executing macro run: #{ex.message}" + return yield ex end unless found_filenames - node.raise "error executing macro run: can't find file '#{filename}'" + return yield Exception.new "can't find file #{filename.inspect}" end if found_filenames.size > 1 - node.raise "error executing macro run: '#{filename}' is a directory" + return yield Exception.new "#{filename.inspect} is a directory" end filename = found_filenames.first end + filename + end + + def interpret_run(node) + if node.args.size == 0 + node.wrong_number_of_arguments "macro call 'run'", 0, "1+" + end + + node.args.first.accept self + original_filename = @last.to_macro_id + + filename = original_filename + filename = "#{filename}.cr" unless filename.ends_with?(".cr") + filename = find_file(filename) { |ex| node.raise "error executing macro 'run': #{ex.message}" } run_args = [] of String node.args.each_with_index do |arg, i| From 0f801d73d077b7efe0d62dce52faf07b37148081 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 4 Nov 2018 23:17:46 +0100 Subject: [PATCH 03/11] Yield error message instead of Exception --- src/compiler/crystal/macros/methods.cr | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 5592a77c66d6..9043467309dd 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -185,25 +185,30 @@ module Crystal if filename.starts_with?('/') if File.exists?(filename) unless File.file?(filename) - return yield Exception.new "#{filename.inspect} is not a file" + yield "#{filename.inspect} is not a file" + return end else - return yield Exception.new "can't find file #{filename.inspect}" + yield "can't find file #{filename.inspect}" + return end else begin relative_to = @location.try &.original_filename found_filenames = @program.find_in_path(filename, relative_to) rescue ex - return yield ex + yield ex.message + return end unless found_filenames - return yield Exception.new "can't find file #{filename.inspect}" + yield "can't find file #{filename.inspect}" + return end if found_filenames.size > 1 - return yield Exception.new "#{filename.inspect} is a directory" + yield "#{filename.inspect} is a directory" + return end filename = found_filenames.first @@ -221,7 +226,9 @@ module Crystal filename = original_filename filename = "#{filename}.cr" unless filename.ends_with?(".cr") - filename = find_file(filename) { |ex| node.raise "error executing macro 'run': #{ex.message}" } + filename = find_file(filename) { |error_message| + node.raise "error executing macro 'run': #{error_message}" + } run_args = [] of String node.args.each_with_index do |arg, i| From 2f378a32e4056b08afa42759181bceafddded626 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 4 Nov 2018 23:42:01 +0100 Subject: [PATCH 04/11] Pass return value of yielded block --- src/compiler/crystal/macros/methods.cr | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 9043467309dd..a2e6d5f8dd12 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -185,30 +185,25 @@ module Crystal if filename.starts_with?('/') if File.exists?(filename) unless File.file?(filename) - yield "#{filename.inspect} is not a file" - return + return yield "#{filename.inspect} is not a file" end else - yield "can't find file #{filename.inspect}" - return + return yield "can't find file #{filename.inspect}" end else begin relative_to = @location.try &.original_filename found_filenames = @program.find_in_path(filename, relative_to) rescue ex - yield ex.message - return + return yield ex.message end unless found_filenames - yield "can't find file #{filename.inspect}" - return + return yield "can't find file #{filename.inspect}" end if found_filenames.size > 1 - yield "#{filename.inspect} is a directory" - return + return yield "#{filename.inspect} is a directory" end filename = found_filenames.first From 82fe3113f66779bb089a0d696255656780329a3f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 6 Nov 2018 15:05:06 +0100 Subject: [PATCH 05/11] Fixes as per @straight-shoota's review --- src/compiler/crystal/macros.cr | 2 +- src/compiler/crystal/macros/methods.cr | 66 +++++++++++++------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/compiler/crystal/macros.cr b/src/compiler/crystal/macros.cr index 5f8b975d5564..e78c3e393514 100644 --- a/src/compiler/crystal/macros.cr +++ b/src/compiler/crystal/macros.cr @@ -67,7 +67,7 @@ module Crystal::Macros def raise(message) : NoReturn end - # Reads a file: if it exists, returns a StringLiteral with its contents; + # Reads a file: if it exists, returns a `StringLiteral` with its contents; # otherwise `nil` is returned. # # To read a file relative to where the macro is defined, use: diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index a2e6d5f8dd12..eb36bc2c8674 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -4,6 +4,37 @@ require "semantic_version" module Crystal class MacroInterpreter + private def find_file(filename) + # Support absolute paths + if filename.starts_with?('/') + if File.exists?(filename) + unless File.file?(filename) + return yield "#{filename.inspect} is not a file" + end + else + return yield "can't find file #{filename.inspect}" + end + else + begin + relative_to = @location.try &.original_filename + found_filenames = @program.find_in_path(filename, relative_to) + rescue ex + return yield ex.message + end + + unless found_filenames + return yield "can't find file #{filename.inspect}" + end + + if found_filenames.size > 1 + return yield "#{filename.inspect} is a directory" + end + + filename = found_filenames.first + end + filename + end + def interpret_top_level_call(node) interpret_top_level_call?(node) || node.raise("undefined macro method: '#{node.name}'") @@ -180,37 +211,6 @@ module Crystal macro_raise(node, node.args, self) end - private def find_file(filename) - # Support absolute paths - if filename.starts_with?('/') - if File.exists?(filename) - unless File.file?(filename) - return yield "#{filename.inspect} is not a file" - end - else - return yield "can't find file #{filename.inspect}" - end - else - begin - relative_to = @location.try &.original_filename - found_filenames = @program.find_in_path(filename, relative_to) - rescue ex - return yield ex.message - end - - unless found_filenames - return yield "can't find file #{filename.inspect}" - end - - if found_filenames.size > 1 - return yield "#{filename.inspect} is a directory" - end - - filename = found_filenames.first - end - filename - end - def interpret_run(node) if node.args.size == 0 node.wrong_number_of_arguments "macro call 'run'", 0, "1+" @@ -221,9 +221,9 @@ module Crystal filename = original_filename filename = "#{filename}.cr" unless filename.ends_with?(".cr") - filename = find_file(filename) { |error_message| + filename = find_file(filename) do |error_message| node.raise "error executing macro 'run': #{error_message}" - } + end run_args = [] of String node.args.each_with_index do |arg, i| From 0820ae88b508b6770f43fce1607bba3014aa988c Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 01:46:11 +0100 Subject: [PATCH 06/11] Don't use relative path resolution in 'read_file' macro --- src/compiler/crystal/macros/methods.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index eb36bc2c8674..24ca4da881f0 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -182,8 +182,7 @@ module Crystal node.args[0].accept self filename = @last.to_macro_id - filename = find_file(filename) { } - if filename + if File.exists?(filename) @last = StringLiteral.new(File.read(filename)) else @last = NilLiteral.new From f13cccba693ba3810ab360de9a7e181d27b4fa90 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 11:56:43 +0100 Subject: [PATCH 07/11] Add NOTE and spec regarding relative paths --- spec/compiler/macro/macro_methods_spec.cr | 32 +++++++++++++++++------ src/compiler/crystal/macros.cr | 2 ++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/spec/compiler/macro/macro_methods_spec.cr b/spec/compiler/macro/macro_methods_spec.cr index 355bbf82cf25..e7d844247024 100644 --- a/spec/compiler/macro/macro_methods_spec.cr +++ b/spec/compiler/macro/macro_methods_spec.cr @@ -1759,16 +1759,32 @@ module Crystal end describe "read_file" do - it "reads file (exists)" do - run(%q< - {{read_file("#{__DIR__}/../data/build")}} - >, filename = __FILE__).to_string.should eq(File.read("#{__DIR__}/../data/build")) + context "with absolute path" do + it "reads file (exists)" do + run(%q< + {{read_file("#{__DIR__}/../data/build")}} + >, filename = __FILE__).to_string.should eq(File.read("#{__DIR__}/../data/build")) + end + + it "reads file (doesn't exists)" do + run(%q< + {{read_file("#{__DIR__}/../data/build_foo")}} ? 10 : 20 + >, filename = __FILE__).to_i.should eq(20) + end end - it "reads file (doesn't exists)" do - run(%q< - {{read_file("#{__DIR__}/../data/build_foo")}} ? 10 : 20 - >, filename = __FILE__).to_i.should eq(20) + context "with relative path" do + it "reads file (exists)" do + run(%q< + {{read_file("spec/compiler/data/build")}} + >, filename = __FILE__).to_string.should eq(File.read("spec/compiler/data/build")) + end + + it "reads file (doesn't exists)" do + run(%q< + {{read_file("spec/compiler/data/build_foo")}} ? 10 : 20 + >, filename = __FILE__).to_i.should eq(20) + end end end end diff --git a/src/compiler/crystal/macros.cr b/src/compiler/crystal/macros.cr index e78c3e393514..4119b9ec68e2 100644 --- a/src/compiler/crystal/macros.cr +++ b/src/compiler/crystal/macros.cr @@ -75,6 +75,8 @@ module Crystal::Macros # ``` # read_file("#{__DIR__}/some_file.txt") # ``` + # + # NOTE: Relative paths are resolved to the current working directory. def read_file(filename) : StringLiteral | NilLiteral end From a2d70e4f56d35bbbb7dc591e3f0a442b40645856 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 14:38:30 +0100 Subject: [PATCH 08/11] Use more apt File.file? check instead of File.exists? --- src/compiler/crystal/macros/methods.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 24ca4da881f0..c837dedcb8d6 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -182,7 +182,7 @@ module Crystal node.args[0].accept self filename = @last.to_macro_id - if File.exists?(filename) + if File.file?(filename) @last = StringLiteral.new(File.read(filename)) else @last = NilLiteral.new From 9aa5f09b15c06bdebc57931e71fc0b4493a7c9cd Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 14:40:12 +0100 Subject: [PATCH 09/11] Refactor filename resolution in `find_file` to be more consistent --- src/compiler/crystal/macros/methods.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index c837dedcb8d6..c638b77d5c82 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -5,6 +5,8 @@ require "semantic_version" module Crystal class MacroInterpreter private def find_file(filename) + filename = "#{filename}.cr" unless filename.ends_with?(".cr") + # Support absolute paths if filename.starts_with?('/') if File.exists?(filename) @@ -219,7 +221,6 @@ module Crystal original_filename = @last.to_macro_id filename = original_filename - filename = "#{filename}.cr" unless filename.ends_with?(".cr") filename = find_file(filename) do |error_message| node.raise "error executing macro 'run': #{error_message}" end From 29adc3534508fc761b3566904d4c6f6e2c758eb4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 19:53:55 +0100 Subject: [PATCH 10/11] Revert `run` macro refactors --- src/compiler/crystal/macros/methods.cr | 67 ++++++++++++-------------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index c638b77d5c82..8069d4cc57b4 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -4,39 +4,6 @@ require "semantic_version" module Crystal class MacroInterpreter - private def find_file(filename) - filename = "#{filename}.cr" unless filename.ends_with?(".cr") - - # Support absolute paths - if filename.starts_with?('/') - if File.exists?(filename) - unless File.file?(filename) - return yield "#{filename.inspect} is not a file" - end - else - return yield "can't find file #{filename.inspect}" - end - else - begin - relative_to = @location.try &.original_filename - found_filenames = @program.find_in_path(filename, relative_to) - rescue ex - return yield ex.message - end - - unless found_filenames - return yield "can't find file #{filename.inspect}" - end - - if found_filenames.size > 1 - return yield "#{filename.inspect} is a directory" - end - - filename = found_filenames.first - end - filename - end - def interpret_top_level_call(node) interpret_top_level_call?(node) || node.raise("undefined macro method: '#{node.name}'") @@ -218,11 +185,37 @@ module Crystal end node.args.first.accept self - original_filename = @last.to_macro_id + filename = @last.to_macro_id + original_filename = filename + + # Support absolute paths + if filename.starts_with?("/") + filename = "#{filename}.cr" unless filename.ends_with?(".cr") - filename = original_filename - filename = find_file(filename) do |error_message| - node.raise "error executing macro 'run': #{error_message}" + if File.exists?(filename) + unless File.file?(filename) + node.raise "error executing macro run: '#{filename}' is not a file" + end + else + node.raise "error executing macro run: can't find file '#{filename}'" + end + else + begin + relative_to = @location.try &.original_filename + found_filenames = @program.find_in_path(filename, relative_to) + rescue ex + node.raise "error executing macro run: #{ex.message}" + end + + unless found_filenames + node.raise "error executing macro run: can't find file '#{filename}'" + end + + if found_filenames.size > 1 + node.raise "error executing macro run: '#{filename}' is a directory" + end + + filename = found_filenames.first end run_args = [] of String From 2ae9e53af5de0f51f67170cca32a2d07f1be5ff9 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 7 Nov 2018 20:59:19 +0100 Subject: [PATCH 11/11] Move `interpret_read_file` definition under lexicographically correct position --- src/compiler/crystal/macros/methods.cr | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/compiler/crystal/macros/methods.cr b/src/compiler/crystal/macros/methods.cr index 8069d4cc57b4..0a3dbfaab64b 100644 --- a/src/compiler/crystal/macros/methods.cr +++ b/src/compiler/crystal/macros/methods.cr @@ -144,20 +144,6 @@ module Crystal raise SkipMacroException.new(@str.to_s) end - def interpret_read_file(node) - unless node.args.size == 1 - node.wrong_number_of_arguments "macro call 'read_file'", node.args.size, 1 - end - - node.args[0].accept self - filename = @last.to_macro_id - if File.file?(filename) - @last = StringLiteral.new(File.read(filename)) - else - @last = NilLiteral.new - end - end - def interpret_system(node) cmd = node.args.map do |arg| arg.accept self @@ -179,6 +165,20 @@ module Crystal macro_raise(node, node.args, self) end + def interpret_read_file(node) + unless node.args.size == 1 + node.wrong_number_of_arguments "macro call 'read_file'", node.args.size, 1 + end + + node.args[0].accept self + filename = @last.to_macro_id + if File.file?(filename) + @last = StringLiteral.new(File.read(filename)) + else + @last = NilLiteral.new + end + end + def interpret_run(node) if node.args.size == 0 node.wrong_number_of_arguments "macro call 'run'", 0, "1+"