crystal-lang · Feb 9, 2017 · Feb 9, 2017
Showing with 88 additions and 19 deletions.

+18 −0 spec/std/option_parser_spec.cr

+14 −4 spec/std/yaml/yaml_spec.cr

+17 −4 src/option_parser.cr

+1 −1 src/yaml.cr

+1 −1 src/yaml/parser.cr

+37 −9 src/yaml/pull_parser.cr
diff --git a/spec/std/option_parser_spec.cr b/spec/std/option_parser_spec.cr
@@ -420,4 +420,22 @@ describe "OptionParser" do
       value2.should eq("value2")
     end
   end
+
+  it "raises if flag doesn't start with dash (#4001)" do
+    OptionParser.parse([] of String) do |opts|
+      expect_raises ArgumentError, %(argument 'flag' ("foo") must start with a dash) do
+        opts.on("foo", "") { }
+      end
+
+      expect_raises ArgumentError, %(argument 'short_flag' ("foo") must start with a dash) do
+        opts.on("foo", "bar", "baz") { }
+      end
+
+      expect_raises ArgumentError, %(argument 'long_flag' ("bar") must start with a dash) do
+        opts.on("-foo", "bar", "baz") { }
+      end
+
+      opts.on("", "-bar", "baz") { }
+    end
+  end
 end
diff --git a/spec/std/yaml/yaml_spec.cr b/spec/std/yaml/yaml_spec.cr
@@ -88,8 +88,8 @@ describe "YAML" do
             YAML
           fail "expected YAML.parse to raise"
         rescue ex : YAML::ParseException
-          ex.line_number.should eq(3)
-          ex.column_number.should eq(3)
+          ex.line_number.should eq(4)
+          ex.column_number.should eq(4)
         end
       end
 
@@ -107,8 +107,18 @@ describe "YAML" do
             end
           end
         rescue ex : YAML::ParseException
-          ex.line_number.should eq(1)
-          ex.column_number.should eq(2)
+          ex.line_number.should eq(2)
+          ex.column_number.should eq(3)
+        end
+      end
+
+      it "has correct message (#4006)" do
+        expect_raises YAML::ParseException, "could not find expected ':' at line 4, column 1, while scanning a simple key at line 3, column 5" do
+          YAML.parse <<-END
+            a:
+              - "b": >
+                c
+            END
         end
       end
 

diff --git a/src/option_parser.cr b/src/option_parser.cr
@@ -93,7 +93,7 @@ class OptionParser
 
   # Establishes a handler for a *flag*.
   #
-  # Flags can (but don't have to) start with a dash. They can also have
+  # Flags must start with a dash or double dash. They can also have
   # an optional argument, which will get passed to the block.
   # Each flag has a description, which will be used for the help message.
   #
@@ -102,15 +102,20 @@ class OptionParser
   # * `-a`, `-B`
   # * `--something-longer`
   # * `-f FILE`, `--file FILE`, `--file=FILE` (these will yield the passed value to the block as a string)
-  def on(flag, description, &block : String ->)
-    append_flag flag.to_s, description
+  def on(flag : String, description : String, &block : String ->)
+    check_starts_with_dash flag, "flag"
+
+    append_flag flag, description
     @handlers << Handler.new(flag, block)
   end
 
   # Establishes a handler for a pair of short and long flags.
   #
   # See the other definition of `on` for examples.
-  def on(short_flag, long_flag, description, &block : String ->)
+  def on(short_flag : String, long_flag : String, description : String, &block : String ->)
+    check_starts_with_dash short_flag, "short_flag", allow_empty: true
+    check_starts_with_dash long_flag, "long_flag"
+
     append_flag "#{short_flag}, #{long_flag}", description
 
     has_argument = /([ =].+)/
@@ -180,6 +185,14 @@ class OptionParser
     parse ARGV
   end
 
+  private def check_starts_with_dash(arg, name, allow_empty = false)
+    return if allow_empty && arg.empty?
+
+    unless arg.starts_with?('-')
+      raise ArgumentError.new("argument '#{name}' (#{arg.inspect}) must start with a dash (-)")
+    end
+  end
+
   private struct ParseTask
     @double_dash_index : Int32?
 

diff --git a/src/yaml.cr b/src/yaml.cr
@@ -71,7 +71,7 @@ module YAML
     def initialize(message, line_number, column_number)
       @line_number = line_number.to_i
       @column_number = column_number.to_i
-      super "#{message} at #{@line_number}:#{@column_number}"
+      super(message)
     end
   end
 

diff --git a/src/yaml/parser.cr b/src/yaml/parser.cr
@@ -109,6 +109,6 @@ class YAML::Parser
   end
 
   private def raise(msg)
-    ::raise ParseException.new(msg, @pull_parser.problem_line_number, @pull_parser.problem_column_number)
+    @pull_parser.raise(msg)
   end
 end
diff --git a/src/yaml/pull_parser.cr b/src/yaml/pull_parser.cr
@@ -22,7 +22,7 @@ class YAML::PullParser
     end
 
     read_next
-    parse_exception "Expected STREAM_START" unless kind == LibYAML::EventType::STREAM_START
+    raise "Expected STREAM_START" unless kind == LibYAML::EventType::STREAM_START
   end
 
   def self.new(content)
@@ -76,6 +76,14 @@ class YAML::PullParser
   def read_next
     LibYAML.yaml_event_delete(pointerof(@event))
     LibYAML.yaml_parser_parse(@parser, pointerof(@event))
+    if problem = problem?
+      if context = context?
+        msg = "#{String.new(problem)} at line #{problem_line_number}, column #{problem_column_number}, #{String.new(context)} at line #{context_line_number}, column #{context_column_number}"
+      else
+        msg = "#{String.new(problem)} at line #{problem_line_number}, column #{problem_column_number}"
+      end
+      raise msg
+    end
     kind
   end
 
@@ -174,7 +182,7 @@ class YAML::PullParser
     when EventKind::SEQUENCE_START, EventKind::MAPPING_START
       String.build { |io| read_raw(io) }
     else
-      parse_exception "unexpected kind: #{kind}"
+      raise "unexpected kind: #{kind}"
     end
   end
 
@@ -208,7 +216,7 @@ class YAML::PullParser
       io << "}"
       read_next
     else
-      parse_exception "unexpected kind: #{kind}"
+      raise "unexpected kind: #{kind}"
     end
   end
 
@@ -243,21 +251,41 @@ class YAML::PullParser
   end
 
   def problem_line_number
-    problem? ? problem_mark.line : line_number
+    # YAML starts counting from 0, we want to count from 1
+    (problem? ? problem_mark?.line : line_number) + 1
   end
 
   def problem_column_number
-    problem? ? problem_mark.column : column_number
+    # YAML starts counting from 0, we want to count from 1
+    (problem? ? problem_mark?.column : column_number) + 1
   end
 
-  def problem_mark
+  private def problem_mark?
     @parser.value.problem_mark
   end
 
   private def problem?
     @parser.value.problem
   end
 
+  private def context?
+    @parser.value.context
+  end
+
+  private def context_mark?
+    @parser.value.context_mark
+  end
+
+  private def context_line_number
+    # YAML starts counting from 0, we want to count from 1
+    context_mark?.line + 1
+  end
+
+  private def context_column_number
+    # YAML starts counting from 0, we want to count from 1
+    context_mark?.column + 1
+  end
+
   def finalize
     return if @closed
 
@@ -271,14 +299,14 @@ class YAML::PullParser
   end
 
   private def expect_kind(kind)
-    parse_exception "expected #{kind} but was #{self.kind}" unless kind == self.kind
+    raise "expected #{kind} but was #{self.kind}" unless kind == self.kind
   end
 
   private def read_anchor(anchor)
     anchor ? String.new(anchor) : nil
   end
 
-  private def parse_exception(msg)
-    raise ParseException.new(msg, problem_line_number, problem_column_number)
+  def raise(msg : String)
+    ::raise ParseException.new(msg, problem_line_number, problem_column_number)
   end
 end