Merge pull request #325 from FnControlOption/unless_else

Add autocorrect for `Style/UnlessElse`

spec/ameba/rule/style/unless_else_spec.cr

@@ -13,7 +13,7 @@ module Ameba::Rule::Style
     end
 
     it "fails if unless has else" do
-      expect_issue subject, <<-CRYSTAL
+      source = expect_issue subject, <<-CRYSTAL
         unless something
         # ^^^^^^^^^^^^^^ error: Favour if over unless with else
           :one
@@ -21,6 +21,14 @@ module Ameba::Rule::Style
           :two
         end
         CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        if something
+          :two
+        else
+          :one
+        end
+        CRYSTAL
     end
   end
 end

spec/ameba/source_spec.cr

@@ -4,46 +4,70 @@ module Ameba
   describe Source do
     describe ".new" do
       it "allows to create a source by code and path" do
-        s = Source.new("code", "path")
-        s.path.should eq "path"
-        s.code.should eq "code"
-        s.lines.should eq ["code"]
+        source = Source.new "code", "path"
+        source.path.should eq "path"
+        source.code.should eq "code"
+        source.lines.should eq ["code"]
       end
     end
 
     describe "#fullpath" do
       it "returns a relative path of the source" do
-        s = Source.new "", "./source_spec.cr"
-        s.fullpath.should contain "source_spec.cr"
+        source = Source.new "", "./source_spec.cr"
+        source.fullpath.should contain "source_spec.cr"
       end
 
       it "returns fullpath if path is blank" do
-        s = Source.new "", ""
-        s.fullpath.should_not be_nil
+        source = Source.new "", ""
+        source.fullpath.should_not be_nil
       end
     end
 
     describe "#spec?" do
       it "returns true if the source is a spec file" do
-        s = Source.new "", "./source_spec.cr"
-        s.spec?.should be_true
+        source = Source.new "", "./source_spec.cr"
+        source.spec?.should be_true
       end
 
       it "returns false if the source is not a spec file" do
-        s = Source.new "", "./source.cr"
-        s.spec?.should be_false
+        source = Source.new "", "./source.cr"
+        source.spec?.should be_false
       end
     end
 
     describe "#matches_path?" do
       it "returns true if source's path is matched" do
-        s = Source.new "", "source.cr"
-        s.matches_path?("source.cr").should be_true
+        source = Source.new "", "source.cr"
+        source.matches_path?("source.cr").should be_true
       end
 
       it "returns false if source's path is not matched" do
-        s = Source.new "", "source.cr"
-        s.matches_path?("new_source.cr").should be_false
+        source = Source.new "", "source.cr"
+        source.matches_path?("new_source.cr").should be_false
+      end
+    end
+
+    describe "#pos" do
+      it "works" do
+        source = Source.new <<-CRYSTAL
+          foo
+          bar
+          fizz
+          buzz
+          CRYSTAL
+
+        location = Crystal::Location.new("", 2, 1)
+        end_location = Crystal::Location.new("", 3, 4)
+
+        range = Range.new(
+          source.pos(location),
+          source.pos(end_location, end: true),
+          exclusive: true
+        )
+        source.code[range].should eq <<-CRYSTAL
+          bar
+          fizz
+          CRYSTAL
       end
     end
   end

src/ameba/rule/style/unless_else.cr

@@ -50,7 +50,36 @@ module Ameba::Rule::Style
     MSG = "Favour if over unless with else"
 
     def test(source, node : Crystal::Unless)
-      issue_for node, MSG unless node.else.nop?
+      return if node.else.nop?
+
+      location = node.location
+      cond_end_location = node.cond.end_location
+      else_location = node.else_location
+      end_location = node.end_location
+
+      unless location && cond_end_location && else_location && end_location
+        issue_for node, MSG
+        return
+      end
+
+      issue_for node, MSG do |corrector|
+        keyword_begin_pos = source.pos(location)
+        keyword_end_pos = keyword_begin_pos + {{ "unless".size }}
+        keyword_range = keyword_begin_pos...keyword_end_pos
+
+        cond_end_pos = source.pos(cond_end_location, end: true)
+        else_begin_pos = source.pos(else_location)
+        body_range = cond_end_pos...else_begin_pos
+
+        else_end_pos = else_begin_pos + {{ "else".size }}
+        end_end_pos = source.pos(end_location, end: true)
+        end_begin_pos = end_end_pos - {{ "end".size }}
+        else_range = else_end_pos...end_begin_pos
+
+        corrector.replace(keyword_range, "if")
+        corrector.replace(body_range, source.code[else_range])
+        corrector.replace(else_range, source.code[body_range])
+      end
     end
   end
 end

src/ameba/source.cr

@@ -76,5 +76,13 @@ module Ameba
     def matches_path?(filepath)
       path.in?(filepath, File.expand_path(filepath))
     end
+
+    # Converts an AST location to a string position.
+    def pos(location : Crystal::Location, end end_pos = false) : Int32
+      line, column = location.line_number, location.column_number
+      pos = lines[0...line - 1].sum(&.size) + line + column - 2
+      pos += 1 if end_pos
+      pos
+    end
   end
 end

src/ameba/source/corrector.cr

@@ -18,41 +18,92 @@ class Ameba::Source
       @rewriter.replace(loc_to_pos(location), loc_to_pos(end_location) + 1, content)
     end
 
+    # :ditto:
+    def replace(range : Range(Int32, Int32), content)
+      begin_pos, end_pos = range.begin, range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.replace(begin_pos, end_pos, content)
+    end
+
     # Inserts the given strings before and after the given range.
     def wrap(location, end_location, insert_before, insert_after)
       @rewriter.wrap(loc_to_pos(location), loc_to_pos(end_location) + 1, insert_before, insert_after)
     end
 
+    # :ditto:
+    def wrap(range : Range(Int32, Int32), insert_before, insert_after)
+      begin_pos, end_pos = range.begin, range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.wrap(begin_pos, end_pos, insert_before, insert_after)
+    end
+
     # Shortcut for `replace(location, end_location, "")`
     def remove(location, end_location)
       @rewriter.remove(loc_to_pos(location), loc_to_pos(end_location) + 1)
     end
 
+    # Shortcut for `replace(range, "")`
+    def remove(range : Range(Int32, Int32))
+      begin_pos, end_pos = range.begin, range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.remove(begin_pos, end_pos)
+    end
+
     # Shortcut for `wrap(location, end_location, content, nil)`
     def insert_before(location, end_location, content)
       @rewriter.insert_before(loc_to_pos(location), loc_to_pos(end_location) + 1, content)
     end
 
+    # Shortcut for `wrap(range, content, nil)`
+    def insert_before(range : Range(Int32, Int32), content)
+      begin_pos, end_pos = range.begin, range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.insert_before(begin_pos, end_pos, content)
+    end
+
     # Shortcut for `wrap(location, end_location, nil, content)`
     def insert_after(location, end_location, content)
       @rewriter.insert_after(loc_to_pos(location), loc_to_pos(end_location) + 1, content)
     end
 
+    # Shortcut for `wrap(range, nil, content)`
+    def insert_after(range : Range(Int32, Int32), content)
+      begin_pos, end_pos = range.begin, range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.insert_after(begin_pos, end_pos, content)
+    end
+
     # Shortcut for `insert_before(location, location, content)`
     def insert_before(location, content)
       @rewriter.insert_before(loc_to_pos(location), content)
     end
 
+    # Shortcut for `insert_before(pos.., content)`
+    def insert_before(pos : Int32, content)
+      @rewriter.insert_before(pos, content)
+    end
+
     # Shortcut for `insert_after(location, location, content)`
     def insert_after(location, content)
       @rewriter.insert_after(loc_to_pos(location) + 1, content)
     end
 
+    # Shortcut for `insert_after(...pos, content)`
+    def insert_after(pos : Int32, content)
+      @rewriter.insert_after(pos, content)
+    end
+
     # Removes *size* characters prior to the source range.
     def remove_preceding(location, end_location, size)
       @rewriter.remove(loc_to_pos(location) - size, loc_to_pos(location))
     end
 
+    # :ditto:
+    def remove_preceding(range : Range(Int32, Int32), size)
+      begin_pos = range.begin
+      @rewriter.remove(begin_pos - size, begin_pos)
+    end
+
     # Removes *size* characters from the beginning of the given range.
     # If *size* is greater than the size of the range, the removed region can
     # overrun the end of the range.
@@ -60,6 +111,12 @@ class Ameba::Source
       @rewriter.remove(loc_to_pos(location), loc_to_pos(location) + size)
     end
 
+    # :ditto:
+    def remove_leading(range : Range(Int32, Int32), size)
+      begin_pos = range.begin
+      @rewriter.remove(begin_pos, begin_pos + size)
+    end
+
     # Removes *size* characters from the end of the given range.
     # If *size* is greater than the size of the range, the removed region can
     # overrun the beginning of the range.
@@ -67,6 +124,13 @@ class Ameba::Source
       @rewriter.remove(loc_to_pos(end_location) + 1 - size, loc_to_pos(end_location) + 1)
     end
 
+    # :ditto:
+    def remove_trailing(range : Range(Int32, Int32), size)
+      end_pos = range.end
+      end_pos -= 1 unless range.excludes_end?
+      @rewriter.remove(end_pos - size, end_pos)
+    end
+
     private def loc_to_pos(location : Crystal::Location | {Int32, Int32})
       if location.is_a?(Crystal::Location)
         line, column = location.line_number, location.column_number