Merge pull request #587 from Shopify/ar/nilable-location

Allow nilable Spoom::Location lines and columns attributes

Author: alexcrocha

lib/spoom/location.rb

@@ -15,42 +15,73 @@ class << self
       sig { params(location_string: String).returns(Location) }
       def from_string(location_string)
         file, rest = location_string.split(":", 2)
-        raise LocationError, "Invalid location string: #{location_string}" unless file && rest
+        raise LocationError, "Invalid location string `#{location_string}`: missing file name" unless file
+
+        return new(file) if rest.nil?
 
         start_line, rest = rest.split(":", 2)
-        raise LocationError, "Invalid location string: #{location_string}" unless start_line && rest
+        if rest.nil?
+          start_line, end_line = T.must(start_line).split("-", 2)
+          raise LocationError, "Invalid location string `#{location_string}`: missing end line" unless end_line
+
+          return new(file, start_line: start_line.to_i, end_line: end_line.to_i) if end_line
+        end
 
         start_column, rest = rest.split("-", 2)
-        raise LocationError, "Invalid location string: #{location_string}" unless start_column && rest
+        raise LocationError, "Invalid location string `#{location_string}`: missing end line and column" if rest.nil?
 
         end_line, end_column = rest.split(":", 2)
-        raise LocationError, "Invalid location string: #{location_string}" unless end_line && end_column
-
-        new(file, start_line.to_i, start_column.to_i, end_line.to_i, end_column.to_i)
+        raise LocationError,
+          "Invalid location string `#{location_string}`: missing end column" unless end_line && end_column
+
+        new(
+          file,
+          start_line: start_line.to_i,
+          start_column: start_column.to_i,
+          end_line: end_line.to_i,
+          end_column: end_column.to_i,
+        )
       end
 
       sig { params(file: String, location: Prism::Location).returns(Location) }
       def from_prism(file, location)
-        new(file, location.start_line, location.start_column, location.end_line, location.end_column)
+        new(
+          file,
+          start_line: location.start_line,
+          start_column: location.start_column,
+          end_line: location.end_line,
+          end_column: location.end_column,
+        )
       end
     end
 
     sig { returns(String) }
     attr_reader :file
 
-    sig { returns(Integer) }
+    sig { returns(T.nilable(Integer)) }
     attr_reader :start_line, :start_column, :end_line, :end_column
 
     sig do
       params(
         file: String,
-        start_line: Integer,
-        start_column: Integer,
-        end_line: Integer,
-        end_column: Integer,
+        start_line: T.nilable(Integer),
+        start_column: T.nilable(Integer),
+        end_line: T.nilable(Integer),
+        end_column: T.nilable(Integer),
       ).void
     end
-    def initialize(file, start_line, start_column, end_line, end_column)
+    def initialize(file, start_line: nil, start_column: nil, end_line: nil, end_column: nil)
+      raise LocationError,
+        "Invalid location: end line is required if start line is provided" if start_line && !end_line
+      raise LocationError,
+        "Invalid location: start line is required if end line is provided" if !start_line && end_line
+      raise LocationError,
+        "Invalid location: end column is required if start column is provided" if start_column && !end_column
+      raise LocationError,
+        "Invalid location: start column is required if end column is provided" if !start_column && end_column
+      raise LocationError,
+        "Invalid location: lines are required if columns are provided" if start_column && !start_line
+
       @file = file
       @start_line = start_line
       @start_column = start_column
@@ -61,10 +92,12 @@ def initialize(file, start_line, start_column, end_line, end_column)
     sig { params(other: Location).returns(T::Boolean) }
     def include?(other)
       return false unless @file == other.file
-      return false if @start_line > other.start_line
-      return false if @start_line == other.start_line && @start_column > other.start_column
-      return false if @end_line < other.end_line
-      return false if @end_line == other.end_line && @end_column < other.end_column
+      return false if (@start_line || -Float::INFINITY) > (other.start_line || -Float::INFINITY)
+      return false if @start_line == other.start_line &&
+        (@start_column || -Float::INFINITY) > (other.start_column || -Float::INFINITY)
+      return false if (@end_line || Float::INFINITY) < (other.end_line || Float::INFINITY)
+      return false if @end_line == other.end_line &&
+        (@end_column || Float::INFINITY) < (other.end_column || Float::INFINITY)
 
       true
     end
@@ -73,13 +106,34 @@ def include?(other)
     def <=>(other)
       return unless Location === other
 
-      [file, start_line, start_column, end_line, end_column] <=>
-        [other.file, other.start_line, other.start_column, other.end_line, other.end_column]
+      comparison_array_self = [
+        @file,
+        @start_line || -Float::INFINITY,
+        @start_column || -Float::INFINITY,
+        @end_line || Float::INFINITY,
+        @end_column || Float::INFINITY,
+      ]
+
+      comparison_array_other = [
+        other.file,
+        other.start_line || -Float::INFINITY,
+        other.start_column || -Float::INFINITY,
+        other.end_line || Float::INFINITY,
+        other.end_column || Float::INFINITY,
+      ]
+
+      comparison_array_self <=> comparison_array_other
     end
 
     sig { returns(String) }
     def to_s
-      "#{@file}:#{@start_line}:#{@start_column}-#{@end_line}:#{@end_column}"
+      if @start_line && @start_column
+        "#{@file}:#{@start_line}:#{@start_column}-#{@end_line}:#{@end_column}"
+      elsif @start_line
+        "#{@file}:#{@start_line}-#{@end_line}"
+      else
+        @file
+      end
     end
   end
 end

test/spoom/location_test.rb

@@ -7,12 +7,25 @@ module Spoom
   module Sorbet
     class LocationTest < Minitest::Test
       def test_from_string
-        location = Location.from_string("foo.rb:1:2-3:4")
-        assert_equal("foo.rb", location.file)
-        assert_equal(1, location.start_line)
-        assert_equal(2, location.start_column)
-        assert_equal(3, location.end_line)
-        assert_equal(4, location.end_column)
+        location1 = Location.from_string("foo.rb:1:2-3:4")
+        assert_equal("foo.rb", location1.file)
+        assert_equal(1, location1.start_line)
+        assert_equal(2, location1.start_column)
+        assert_equal(3, location1.end_line)
+        assert_equal(4, location1.end_column)
+
+        location2 = Location.from_string("foo.rb:1-3")
+        assert_equal(1, location2.start_line)
+        assert_equal(3, location2.end_line)
+        assert_nil(location2.start_column)
+        assert_nil(location2.end_column)
+
+        location3 = Location.from_string("foo.rb")
+        assert_equal("foo.rb", location3.file)
+        assert_nil(location3.start_line)
+        assert_nil(location3.start_column)
+        assert_nil(location3.end_line)
+        assert_nil(location3.end_column)
       end
 
       def test_raises_if_location_string_has_missing_components
@@ -27,65 +40,112 @@ def test_raises_if_location_string_has_missing_components
         assert_raises(Location::LocationError) do
           Location.from_string("foo.rb:1")
         end
+      end
+
+      def test_raises_if_initialize_has_missing_attributes
+        assert_raises(Location::LocationError) do
+          Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3)
+        end
+
+        assert_raises(Location::LocationError) do
+          Location.new("foo.rb", start_line: 1, start_column: 2, end_column: 3)
+        end
+
+        assert_raises(Location::LocationError) do
+          Location.new("foo.rb", start_line: 1, end_column: 2, end_line: 3)
+        end
 
         assert_raises(Location::LocationError) do
-          Location.from_string("foo.rb")
+          Location.new("foo.rb", start_column: 1, end_column: 2, end_line: 3)
+        end
+
+        assert_raises(Location::LocationError) do
+          Location.new("foo.rb", start_line: 1, start_column: 2)
+        end
+
+        assert_raises(Location::LocationError) do
+          Location.new("foo.rb", start_line: 1)
         end
       end
 
       def test_include
-        location1 = Location.new("foo.rb", 1, 2, 3, 4)
-        location2 = Location.new("foo.rb", 1, 2, 3, 4)
+        location1 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
+        location2 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
         assert(location1.include?(location2))
 
-        location3 = Location.new("foo.rb", 1, 2, 3, 5)
+        location3 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 5)
         refute(location1.include?(location3))
         assert(location3.include?(location1))
 
-        location4 = Location.new("foo.rb", 1, 2, 4, 4)
+        location4 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 4, end_column: 4)
         refute(location1.include?(location4))
         assert(location4.include?(location1))
 
-        location5 = Location.new("foo.rb", 1, 3, 3, 4)
+        location5 = Location.new("foo.rb", start_line: 1, start_column: 3, end_line: 3, end_column: 4)
         assert(location1.include?(location5))
         refute(location5.include?(location1))
 
-        location6 = Location.new("foo.rb", 2, 2, 3, 4)
+        location6 = Location.new("foo.rb", start_line: 2, start_column: 2, end_line: 3, end_column: 4)
         assert(location1.include?(location6))
         refute(location6.include?(location1))
 
-        location7 = Location.new("bar.rb", 1, 2, 3, 4)
+        location7 = Location.new("bar.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
         refute(location1.include?(location7))
         refute(location7.include?(location1))
+
+        location8 = Location.new("foo.rb")
+        location9 = Location.new("foo.rb")
+        assert(location8.include?(location9))
+        assert(location9.include?(location8))
+
+        assert(location8.include?(location1))
+        refute(location1.include?(location8))
+
+        location10 = Location.new("foo.rb", start_line: 1, end_line: 3)
+        assert(location10.include?(location1))
+        refute(location1.include?(location10))
       end
 
       def test_comparison
-        location1 = Location.new("foo.rb", 1, 2, 3, 4)
-        location2 = Location.new("foo.rb", 1, 2, 3, 4)
+        location1 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
+        location2 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
         assert_equal(0, location1 <=> location2)
 
-        location3 = Location.new("foo.rb", 1, 2, 3, 5)
+        location3 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 5)
         assert_equal(-1, location1 <=> location3)
 
-        location4 = Location.new("foo.rb", 1, 2, 4, 4)
+        location4 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 4, end_column: 4)
         assert_equal(-1, location1 <=> location4)
 
-        location5 = Location.new("foo.rb", 1, 3, 3, 4)
+        location5 = Location.new("foo.rb", start_line: 1, start_column: 3, end_line: 3, end_column: 4)
         assert_equal(-1, location1 <=> location5)
 
-        location6 = Location.new("foo.rb", 11, 2, 3, 4)
+        location6 = Location.new("foo.rb", start_line: 11, start_column: 2, end_line: 3, end_column: 4)
         assert_equal(-1, location1 <=> location6)
 
-        location7 = Location.new("bar.rb", 1, 2, 3, 4)
+        location7 = Location.new("bar.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
         assert_equal(1, location1 <=> location7)
 
         not_a_location = 42
         assert_nil(location1 <=> not_a_location)
+
+        location8 = Location.new("foo.rb")
+        location9 = Location.new("foo.rb")
+        assert_equal(0, location8 <=> location9)
+
+        location10 = Location.new("foo.rb", start_line: 1, end_line: 3)
+        assert_equal(-1, location8 <=> location10)
       end
 
       def test_to_s
-        location = Location.new("foo.rb", 1, 2, 3, 4)
-        assert_equal("foo.rb:1:2-3:4", location.to_s)
+        location1 = Location.new("foo.rb", start_line: 1, start_column: 2, end_line: 3, end_column: 4)
+        assert_equal("foo.rb:1:2-3:4", location1.to_s)
+
+        location2 = Location.new("foo.rb", start_line: 1, end_line: 3)
+        assert_equal("foo.rb:1-3", location2.to_s)
+
+        location3 = Location.new("foo.rb")
+        assert_equal("foo.rb", location3.to_s)
       end
     end
   end