From 487d7b9923b6e3a7cb4274f5db4dcd00ac256698 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Wed, 3 Jan 2018 20:46:22 +0100 Subject: [PATCH 1/5] Remove `bin/crystal` usage for init tool specs --- spec/compiler/crystal/tools/init_spec.cr | 9 +++---- src/compiler/crystal/tools/init.cr | 30 ++++++++++++------------ 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/spec/compiler/crystal/tools/init_spec.cr b/spec/compiler/crystal/tools/init_spec.cr index fa70296481f1..eb7b717d3b73 100644 --- a/spec/compiler/crystal/tools/init_spec.cr +++ b/spec/compiler/crystal/tools/init_spec.cr @@ -6,17 +6,14 @@ require "spec" require "yaml" PROJECT_ROOT_DIR = "#{__DIR__}/../../../.." -BIN_CRYSTAL = File.expand_path("#{PROJECT_ROOT_DIR}/bin/crystal") private def exec_init(project_name, project_dir = nil, type = "lib") args = ["init", type, project_name] args << project_dir if project_dir - process = Process.new(BIN_CRYSTAL, args, shell: true, error: Process::Redirect::Pipe) - stderr = process.error.gets_to_end - status = process.wait - $? = status - stderr + err_io = IO::Memory.new + Crystal::Init.run(args, stderr: err_io) + err_io.to_s end # Creates a temporary directory, cd to it and run the block inside it. diff --git a/src/compiler/crystal/tools/init.cr b/src/compiler/crystal/tools/init.cr index 151325324fa1..b58c36e4fd23 100644 --- a/src/compiler/crystal/tools/init.cr +++ b/src/compiler/crystal/tools/init.cr @@ -7,7 +7,7 @@ module Crystal module Init WHICH_GIT_COMMAND = "which git >/dev/null" - def self.run(args) + def self.run(args, *, stderr = STDERR) config = Config.new OptionParser.parse(args) do |opts| @@ -31,9 +31,9 @@ module Crystal end opts.unknown_args do |args, after_dash| - config.skeleton_type = fetch_skeleton_type(opts, args) - config.name = fetch_name(opts, args) - config.dir = fetch_directory(args, config.name) + config.skeleton_type = fetch_skeleton_type(opts, args, stderr) + config.name = fetch_name(opts, args, stderr) + config.dir = fetch_directory(args, config.name, stderr) end end @@ -67,33 +67,33 @@ module Crystal github_user || "your-github-user" end - def self.fetch_name(opts, args) - fetch_required_parameter(opts, args, "NAME") + def self.fetch_name(opts, args, stderr) + fetch_required_parameter(opts, args, "NAME", stderr) end - def self.fetch_directory(args, project_name) + def self.fetch_directory(args, project_name, stderr) directory = args.empty? ? project_name : args.shift if Dir.exists?(directory) || File.exists?(directory) - STDERR.puts "file or directory #{directory} already exists" + stderr.puts "file or directory #{directory} already exists" exit 1 end directory end - def self.fetch_skeleton_type(opts, args) - skeleton_type = fetch_required_parameter(opts, args, "TYPE") + def self.fetch_skeleton_type(opts, args, stderr) + skeleton_type = fetch_required_parameter(opts, args, "TYPE", stderr) unless {"lib", "app"}.includes?(skeleton_type) - STDERR.puts "invalid TYPE value: #{skeleton_type}" - STDERR.puts opts + stderr.puts "invalid TYPE value: #{skeleton_type}" + stderr.puts opts exit 1 end skeleton_type end - def self.fetch_required_parameter(opts, args, name) + def self.fetch_required_parameter(opts, args, name, stderr) if args.empty? - STDERR.puts "#{name} is missing" - STDERR.puts opts + stderr.puts "#{name} is missing" + stderr.puts opts exit 1 end args.shift From 2b4807694ccbf2a49a9c6c2d50974cb4c69d796a Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Wed, 3 Jan 2018 21:39:23 +0100 Subject: [PATCH 2/5] Make Crystal::Init a class --- src/compiler/crystal/tools/init.cr | 46 ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/compiler/crystal/tools/init.cr b/src/compiler/crystal/tools/init.cr index b58c36e4fd23..a8f03e6284c7 100644 --- a/src/compiler/crystal/tools/init.cr +++ b/src/compiler/crystal/tools/init.cr @@ -4,10 +4,18 @@ require "ecr/macros" require "option_parser" module Crystal - module Init + class Init WHICH_GIT_COMMAND = "which git >/dev/null" def self.run(args, *, stderr = STDERR) + config = new(stderr).parse_args(args) + InitProject.new(config).run + end + + def initialize(@stderr : IO) + end + + def parse_args(args) config = Config.new OptionParser.parse(args) do |opts| @@ -31,19 +39,19 @@ module Crystal end opts.unknown_args do |args, after_dash| - config.skeleton_type = fetch_skeleton_type(opts, args, stderr) - config.name = fetch_name(opts, args, stderr) - config.dir = fetch_directory(args, config.name, stderr) + config.skeleton_type = fetch_skeleton_type(opts, args) + config.name = fetch_name(opts, args) + config.dir = fetch_directory(args, config.name) end end config.author = fetch_author config.email = fetch_email config.github_name = fetch_github_name - InitProject.new(config).run + config end - def self.fetch_author + def fetch_author if system(WHICH_GIT_COMMAND) user_name = `git config --get user.name`.strip user_name = nil if user_name.empty? @@ -51,7 +59,7 @@ module Crystal user_name || "your-name-here" end - def self.fetch_email + def fetch_email if system(WHICH_GIT_COMMAND) user_email = `git config --get user.email`.strip user_email = nil if user_email.empty? @@ -59,7 +67,7 @@ module Crystal user_email || "your-email-here" end - def self.fetch_github_name + def fetch_github_name if system(WHICH_GIT_COMMAND) github_user = `git config --get github.user`.strip github_user = nil if github_user.empty? @@ -67,33 +75,33 @@ module Crystal github_user || "your-github-user" end - def self.fetch_name(opts, args, stderr) - fetch_required_parameter(opts, args, "NAME", stderr) + def fetch_name(opts, args) + fetch_required_parameter(opts, args, "NAME") end - def self.fetch_directory(args, project_name, stderr) + def fetch_directory(args, project_name) directory = args.empty? ? project_name : args.shift if Dir.exists?(directory) || File.exists?(directory) - stderr.puts "file or directory #{directory} already exists" + @stderr.puts "file or directory #{directory} already exists" exit 1 end directory end - def self.fetch_skeleton_type(opts, args, stderr) - skeleton_type = fetch_required_parameter(opts, args, "TYPE", stderr) + def fetch_skeleton_type(opts, args) + skeleton_type = fetch_required_parameter(opts, args, "TYPE") unless {"lib", "app"}.includes?(skeleton_type) - stderr.puts "invalid TYPE value: #{skeleton_type}" - stderr.puts opts + @stderr.puts "invalid TYPE value: #{skeleton_type}" + @stderr.puts opts exit 1 end skeleton_type end - def self.fetch_required_parameter(opts, args, name, stderr) + def fetch_required_parameter(opts, args, name) if args.empty? - stderr.puts "#{name} is missing" - stderr.puts opts + @stderr.puts "#{name} is missing" + @stderr.puts opts exit 1 end args.shift From 498db67901e191032c0f470f00701b5a8edbc3c2 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Wed, 3 Jan 2018 22:34:51 +0100 Subject: [PATCH 3/5] Revert to module, use exceptions to report errors --- spec/compiler/crystal/tools/init_spec.cr | 23 ++++++++----- src/compiler/crystal/command.cr | 7 +++- src/compiler/crystal/tools/init.cr | 43 ++++++++++++------------ 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/spec/compiler/crystal/tools/init_spec.cr b/spec/compiler/crystal/tools/init_spec.cr index eb7b717d3b73..b01bfa6a5166 100644 --- a/spec/compiler/crystal/tools/init_spec.cr +++ b/spec/compiler/crystal/tools/init_spec.cr @@ -8,12 +8,12 @@ require "yaml" PROJECT_ROOT_DIR = "#{__DIR__}/../../../.." private def exec_init(project_name, project_dir = nil, type = "lib") - args = ["init", type, project_name] + args = [type, project_name] args << project_dir if project_dir - err_io = IO::Memory.new - Crystal::Init.run(args, stderr: err_io) - err_io.to_s + config = Crystal::Init.parse_args(args) + config.silent = true + Crystal::Init::InitProject.new(config).run end # Creates a temporary directory, cd to it and run the block inside it. @@ -190,11 +190,16 @@ end within_temporary_directory do existing_dir = "existing-dir" Dir.mkdir(existing_dir) - exec_init(existing_dir).should contain("file or directory #{existing_dir} already exists") + + expect_raises(Crystal::Init::Error, "file or directory #{existing_dir} already exists") do + exec_init(existing_dir) + end existing_file = "existing-file" File.touch(existing_file) - exec_init(existing_file).should contain("file or directory #{existing_file} already exists") + expect_raises(Crystal::Init::Error, "file or directory #{existing_file} already exists") do + exec_init(existing_file) + end end end @@ -205,9 +210,11 @@ end Dir.mkdir(project_name) - exec_init(project_name, project_dir).should_not contain("file or directory #{project_name} already exists") + exec_init(project_name, project_dir) - exec_init(project_name, project_dir).should contain("file or directory #{project_dir} already exists") + expect_raises(Crystal::Init::Error, "file or directory #{project_dir} already exists") do + exec_init(project_name, project_dir) + end end end end diff --git a/src/compiler/crystal/command.cr b/src/compiler/crystal/command.cr index c46cf0174347..f08cce6c8454 100644 --- a/src/compiler/crystal/command.cr +++ b/src/compiler/crystal/command.cr @@ -157,7 +157,12 @@ class Crystal::Command end private def init - Init.run(options) + begin + Init.run(options) + rescue ex : Init::Error + STDERR.puts ex + exit 1 + end end private def build diff --git a/src/compiler/crystal/tools/init.cr b/src/compiler/crystal/tools/init.cr index a8f03e6284c7..d2bd2782fd93 100644 --- a/src/compiler/crystal/tools/init.cr +++ b/src/compiler/crystal/tools/init.cr @@ -4,18 +4,24 @@ require "ecr/macros" require "option_parser" module Crystal - class Init + module Init WHICH_GIT_COMMAND = "which git >/dev/null" - def self.run(args, *, stderr = STDERR) - config = new(stderr).parse_args(args) - InitProject.new(config).run + class Error < ::Exception end - def initialize(@stderr : IO) + class ArgError < Error + def self.new(message, opts : OptionParser) + new("#{message}\n#{opts}\n") + end + end + + def self.run(args) + config = parse_args(args) + InitProject.new(config).run end - def parse_args(args) + def self.parse_args(args) config = Config.new OptionParser.parse(args) do |opts| @@ -51,7 +57,7 @@ module Crystal config end - def fetch_author + def self.fetch_author if system(WHICH_GIT_COMMAND) user_name = `git config --get user.name`.strip user_name = nil if user_name.empty? @@ -59,7 +65,7 @@ module Crystal user_name || "your-name-here" end - def fetch_email + def self.fetch_email if system(WHICH_GIT_COMMAND) user_email = `git config --get user.email`.strip user_email = nil if user_email.empty? @@ -67,7 +73,7 @@ module Crystal user_email || "your-email-here" end - def fetch_github_name + def self.fetch_github_name if system(WHICH_GIT_COMMAND) github_user = `git config --get github.user`.strip github_user = nil if github_user.empty? @@ -75,34 +81,29 @@ module Crystal github_user || "your-github-user" end - def fetch_name(opts, args) + def self.fetch_name(opts, args) fetch_required_parameter(opts, args, "NAME") end - def fetch_directory(args, project_name) + def self.fetch_directory(args, project_name) directory = args.empty? ? project_name : args.shift if Dir.exists?(directory) || File.exists?(directory) - @stderr.puts "file or directory #{directory} already exists" - exit 1 + raise Error.new "file or directory #{directory} already exists" end directory end - def fetch_skeleton_type(opts, args) + def self.fetch_skeleton_type(opts, args) skeleton_type = fetch_required_parameter(opts, args, "TYPE") unless {"lib", "app"}.includes?(skeleton_type) - @stderr.puts "invalid TYPE value: #{skeleton_type}" - @stderr.puts opts - exit 1 + raise ArgError.new "invalid TYPE value: #{skeleton_type}", opts end skeleton_type end - def fetch_required_parameter(opts, args, name) + def self.fetch_required_parameter(opts, args, name) if args.empty? - @stderr.puts "#{name} is missing" - @stderr.puts opts - exit 1 + raise ArgError.new "#{name} is missing", opts end args.shift end From 3996447957e438aefb8e23b062db3befdffb519d Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Thu, 4 Jan 2018 01:56:40 +0100 Subject: [PATCH 4/5] Make exceptions start with uppercase letter --- spec/compiler/crystal/tools/init_spec.cr | 6 +++--- src/compiler/crystal/tools/init.cr | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/compiler/crystal/tools/init_spec.cr b/spec/compiler/crystal/tools/init_spec.cr index b01bfa6a5166..40e32651f077 100644 --- a/spec/compiler/crystal/tools/init_spec.cr +++ b/spec/compiler/crystal/tools/init_spec.cr @@ -191,13 +191,13 @@ end existing_dir = "existing-dir" Dir.mkdir(existing_dir) - expect_raises(Crystal::Init::Error, "file or directory #{existing_dir} already exists") do + expect_raises(Crystal::Init::Error, "File or directory #{existing_dir} already exists") do exec_init(existing_dir) end existing_file = "existing-file" File.touch(existing_file) - expect_raises(Crystal::Init::Error, "file or directory #{existing_file} already exists") do + expect_raises(Crystal::Init::Error, "File or directory #{existing_file} already exists") do exec_init(existing_file) end end @@ -212,7 +212,7 @@ end exec_init(project_name, project_dir) - expect_raises(Crystal::Init::Error, "file or directory #{project_dir} already exists") do + expect_raises(Crystal::Init::Error, "File or directory #{project_dir} already exists") do exec_init(project_name, project_dir) end end diff --git a/src/compiler/crystal/tools/init.cr b/src/compiler/crystal/tools/init.cr index d2bd2782fd93..846c09aedf88 100644 --- a/src/compiler/crystal/tools/init.cr +++ b/src/compiler/crystal/tools/init.cr @@ -88,7 +88,7 @@ module Crystal def self.fetch_directory(args, project_name) directory = args.empty? ? project_name : args.shift if Dir.exists?(directory) || File.exists?(directory) - raise Error.new "file or directory #{directory} already exists" + raise Error.new "File or directory #{directory} already exists" end directory end @@ -96,14 +96,14 @@ module Crystal def self.fetch_skeleton_type(opts, args) skeleton_type = fetch_required_parameter(opts, args, "TYPE") unless {"lib", "app"}.includes?(skeleton_type) - raise ArgError.new "invalid TYPE value: #{skeleton_type}", opts + raise ArgError.new "Invalid TYPE value: #{skeleton_type}", opts end skeleton_type end def self.fetch_required_parameter(opts, args, name) if args.empty? - raise ArgError.new "#{name} is missing", opts + raise ArgError.new "Argument #{name} is missing", opts end args.shift end From 3b417b5a707f2f4325d101308469b240693d4615 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Fri, 5 Jan 2018 17:12:58 +0100 Subject: [PATCH 5/5] Use only 1 exception class --- src/compiler/crystal/tools/init.cr | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/compiler/crystal/tools/init.cr b/src/compiler/crystal/tools/init.cr index 846c09aedf88..0238f77160dd 100644 --- a/src/compiler/crystal/tools/init.cr +++ b/src/compiler/crystal/tools/init.cr @@ -8,9 +8,6 @@ module Crystal WHICH_GIT_COMMAND = "which git >/dev/null" class Error < ::Exception - end - - class ArgError < Error def self.new(message, opts : OptionParser) new("#{message}\n#{opts}\n") end @@ -96,14 +93,14 @@ module Crystal def self.fetch_skeleton_type(opts, args) skeleton_type = fetch_required_parameter(opts, args, "TYPE") unless {"lib", "app"}.includes?(skeleton_type) - raise ArgError.new "Invalid TYPE value: #{skeleton_type}", opts + raise Error.new "Invalid TYPE value: #{skeleton_type}", opts end skeleton_type end def self.fetch_required_parameter(opts, args, name) if args.empty? - raise ArgError.new "Argument #{name} is missing", opts + raise Error.new "Argument #{name} is missing", opts end args.shift end