From d7795c0d7d5bd6f3009907309883a68ab485bb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?= <70830482+FnControlOption@users.noreply.github.com> Date: Mon, 19 Dec 2022 06:27:20 -0800 Subject: [PATCH] Add autocorrect for `Style/UnlessElse` --- spec/ameba/rule/style/unless_else_spec.cr | 10 +++- spec/ameba/source_spec.cr | 17 ++++++ src/ameba/rule/style/unless_else.cr | 25 ++++++++- src/ameba/source.cr | 8 +++ src/ameba/source/corrector.cr | 64 +++++++++++++++++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) diff --git a/spec/ameba/rule/style/unless_else_spec.cr b/spec/ameba/rule/style/unless_else_spec.cr index cf1c065d..f79575a2 100644 --- a/spec/ameba/rule/style/unless_else_spec.cr +++ b/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 it "reports rule, pos and message" do diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index fd4781fa..e50b499b 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -46,5 +46,22 @@ module Ameba s.matches_path?("new_source.cr").should be_false end end + + describe "#pos" do + it "works" do + s = Source.new <<-EOS + foo + bar + fizz + buzz + EOS + loc = Crystal::Location.new("", 2, 1) + end_loc = Crystal::Location.new("", 3, 4) + s.code[s.pos(loc)...s.pos(end_loc, end: true)].should eq <<-EOS + bar + fizz + EOS + end + end end end diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index cdb4b966..20fa0e08 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -50,7 +50,30 @@ 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? + return unless location = node.location + return unless cond_end_location = node.cond.end_location + return unless else_location = node.else_location + return unless end_location = node.end_location + + 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 diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 38ce9b82..03aa3ac3 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -75,5 +75,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 diff --git a/src/ameba/source/corrector.cr b/src/ameba/source/corrector.cr index 646594be..33e722d2 100644 --- a/src/ameba/source/corrector.cr +++ b/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