INI: Rewrite parser to avoid regular expressions (#5442)

`INI.parse` now parses an INI-formatted string into a `Hash` without using any regular expressions.
crystal-lang · Jan 2, 2018 · 68a7ce8 · 68a7ce8
1 parent 1eeb5c0
commit 68a7ce8
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 11 deletions.
diff --git a/spec/std/ini_spec.cr b/spec/std/ini_spec.cr
@@ -3,23 +3,58 @@ require "ini"
 
 describe "INI" do
   describe "parse from string" do
+    it "fails on malformed section" do
+      expect_raises(INI::ParseException, "unterminated section") do
+        INI.parse("[section")
+      end
+    end
+
+    it "fails on data after section" do
+      expect_raises(INI::ParseException, "data after section") do
+        INI.parse("[section] foo  ")
+      end
+    end
+
+    it "fails on malformed declaration" do
+      expect_raises(INI::ParseException, "expected declaration") do
+        INI.parse("foobar")
+      end
+
+      expect_raises(INI::ParseException, "expected declaration") do
+        INI.parse("foo: bar")
+      end
+    end
+
     it "parses key = value" do
       INI.parse("key = value").should eq({"" => {"key" => "value"}})
     end
 
+    it "parses empty values" do
+      INI.parse("key = ").should eq({"" => {"key" => ""}})
+    end
+
     it "ignores whitespaces" do
       INI.parse("   key   =   value  ").should eq({"" => {"key" => "value"}})
+      INI.parse("  [foo]").should eq({} of String => Hash(String, String))
+    end
+
+    it "ignores comments" do
+      INI.parse("; foo\n# bar\nkey = value").should eq({"" => {"key" => "value"}})
     end
 
     it "parses sections" do
       INI.parse("[section]\na = 1").should eq({"section" => {"a" => "1"}})
     end
 
-    it "empty section" do
-      INI.parse("[section]").should eq({"section" => {} of String => String})
+    it "parses a reopened section" do
+      INI.parse("[foo]\na=1\n[foo]\nb=2").should eq({"foo" => {"a" => "1", "b" => "2"}})
+    end
+
+    it "ignores an empty section" do
+      INI.parse("[section]").should eq({} of String => Hash(String, String))
     end
 
-    it "parse file" do
+    it "parses a file" do
       INI.parse(File.read "#{__DIR__}/data/test_file.ini").should eq({
         "general" => {
           "log_level" => "DEBUG",

diff --git a/src/ini.cr b/src/ini.cr
@@ -1,22 +1,58 @@
 class INI
+  # Exception thrown on an INI parse error.
+  class ParseException < Exception
+    getter line_number : Int32
+    getter column_number : Int32
+
+    def initialize(message, @line_number, @column_number)
+      super "#{message} at #{@line_number}:#{@column_number}"
+    end
+
+    def location
+      {line_number, column_number}
+    end
+  end
+
   # Parses INI-style configuration from the given string.
+  # Raises a `ParseException` on any errors.
   #
   # ```
   # INI.parse("[foo]\na = 1") # => {"foo" => {"a" => "1"}}
   # ```
   def self.parse(str) : Hash(String, Hash(String, String))
-    ini = {} of String => Hash(String, String)
+    ini = Hash(String, Hash(String, String)).new
+    current_section = ini[""] = Hash(String, String).new
+    lineno = 0
 
-    section = ""
     str.each_line do |line|
-      if line =~ /\s*(.*[^\s])\s*=\s*(.*[^\s])/
-        ini[section] ||= {} of String => String if section == ""
-        ini[section][$1] = $2
-      elsif line =~ /\[(.*)\]/
-        section = $1
-        ini[section] = {} of String => String
+      lineno += 1
+      next if line.empty?
+
+      offset = 0
+      line.each_char do |char|
+        break unless char.ascii_whitespace?
+        offset += 1
+      end
+
+      case line[offset]
+      when '#', ';'
+        next
+      when '['
+        end_idx = line.index(']', offset)
+        raise ParseException.new("unterminated section", lineno, line.size) unless end_idx
+        raise ParseException.new("data after section", lineno, end_idx + 1) unless end_idx == line.size - 1
+
+        current_section_name = line[offset + 1...end_idx]
+        current_section = ini[current_section_name] ||= Hash(String, String).new
+      else
+        key, eq, value = line.partition('=')
+        raise ParseException.new("expected declaration", lineno, key.size) if eq != "="
+
+        current_section[key.strip] = value.strip
       end
     end
+
+    ini.delete_if { |_, v| v.empty? }
     ini
   end
 end