From a64ec034d5b91260f0de041d639be837677e3362 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: Fri, 23 Dec 2022 03:46:48 -0800 Subject: [PATCH] Allow skipping autocorrect from within autocorrect block --- spec/spec_helper.cr | 25 ++++++-- src/ameba/issue.cr | 6 +- src/ameba/rule/lint/comparison_to_boolean.cr | 21 ++++--- src/ameba/rule/lint/not_nil_after_no_bang.cr | 11 ++-- src/ameba/rule/style/is_a_nil.cr | 5 +- .../style/parentheses_around_condition.cr | 12 +++- src/ameba/rule/style/redundant_next.cr | 12 ++-- src/ameba/rule/style/redundant_return.cr | 12 ++-- src/ameba/rule/style/unless_else.cr | 15 ++--- src/ameba/source/corrector.cr | 57 ++----------------- 10 files changed, 78 insertions(+), 98 deletions(-) diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 4a18746ba..0f50ed2dd 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -109,7 +109,10 @@ module Ameba return unless name.includes?("A") issue_for(node.name, message: "A to AA") do |corrector| - corrector.replace(node.name, name.sub("A", "AA")) + next unless location = node.name.location + next unless end_location = node.name.end_location + + corrector.replace(location, end_location, name.sub("A", "AA")) end end end @@ -126,7 +129,10 @@ module Ameba return unless name.includes?("A") issue_for(node.name, message: "A to B") do |corrector| - corrector.replace(node.name, name.tr("A", "B")) + next unless location = node.name.location + next unless end_location = node.name.end_location + + corrector.replace(location, end_location, name.tr("A", "B")) end end end @@ -143,7 +149,10 @@ module Ameba return unless name.includes?("B") issue_for(node.name, message: "B to A") do |corrector| - corrector.replace(node.name, name.tr("B", "A")) + next unless location = node.name.location + next unless end_location = node.name.end_location + + corrector.replace(location, end_location, name.tr("B", "A")) end end end @@ -160,7 +169,10 @@ module Ameba return unless name.includes?("B") issue_for(node.name, message: "B to C") do |corrector| - corrector.replace(node.name, name.tr("B", "C")) + next unless location = node.name.location + next unless end_location = node.name.end_location + + corrector.replace(location, end_location, name.tr("B", "C")) end end end @@ -177,7 +189,10 @@ module Ameba return unless name.includes?("C") issue_for(node.name, message: "C to A") do |corrector| - corrector.replace(node.name, name.tr("C", "A")) + next unless location = node.name.location + next unless end_location = node.name.end_location + + corrector.replace(location, end_location, name.tr("C", "A")) end end end diff --git a/src/ameba/issue.cr b/src/ameba/issue.cr index b6fa55e8e..0734ea915 100644 --- a/src/ameba/issue.cr +++ b/src/ameba/issue.cr @@ -35,8 +35,10 @@ module Ameba rule.is_a?(Rule::Lint::Syntax) end - def correctable? - !@block.nil? + getter? correctable : Bool do + corrector = Source::Corrector.new(code) + correct(corrector) + !corrector.empty? end def correct(corrector) diff --git a/src/ameba/rule/lint/comparison_to_boolean.cr b/src/ameba/rule/lint/comparison_to_boolean.cr index cc9eda9ee..f39887c83 100644 --- a/src/ameba/rule/lint/comparison_to_boolean.cr +++ b/src/ameba/rule/lint/comparison_to_boolean.cr @@ -43,18 +43,21 @@ module Ameba::Rule::Lint end return unless bool && exp - return unless exp_code = node_source(exp, source.lines) - not = - case node.name - when "==", "===" then !bool.value # foo == false - when "!=" then bool.value # foo != true - end + issue_for node, MSG do |corrector| + next unless location = node.location + next unless end_location = node.end_location + next unless exp_code = node_source(exp, source.lines) - exp_code = "!#{exp_code}" if not + not = + case node.name + when "==", "===" then !bool.value # foo == false + when "!=" then bool.value # foo != true + end - issue_for node, MSG do |corrector| - corrector.replace(node, exp_code) + exp_code = "!#{exp_code}" if not + + corrector.replace(location, end_location, exp_code) end end end diff --git a/src/ameba/rule/lint/not_nil_after_no_bang.cr b/src/ameba/rule/lint/not_nil_after_no_bang.cr index e0dab6e00..0aa7e2c3c 100644 --- a/src/ameba/rule/lint/not_nil_after_no_bang.cr +++ b/src/ameba/rule/lint/not_nil_after_no_bang.cr @@ -47,14 +47,17 @@ module Ameba::Rule::Lint return unless obj.name.in?(obj.block ? BLOCK_CALL_NAMES : CALL_NAMES) return unless name_location = obj.name_location - return unless name_location_end = name_end_location(obj) - return unless end_location = name_end_location(node) + return unless name_end_location = name_end_location(node) msg = MSG % {obj.name, obj.name} - issue_for name_location, end_location, msg do |corrector| + issue_for name_location, name_end_location, msg do |corrector| + next unless location = node.location + next unless end_location = node.end_location + next unless name_location_end = name_end_location(obj) + corrector.insert_after(name_location_end, '!') - corrector.remove_trailing(node, {{ ".not_nil!".size }}) + corrector.remove_trailing(location, end_location, {{ ".not_nil!".size }}) end end end diff --git a/src/ameba/rule/style/is_a_nil.cr b/src/ameba/rule/style/is_a_nil.cr index 06681261d..adeb87b75 100644 --- a/src/ameba/rule/style/is_a_nil.cr +++ b/src/ameba/rule/style/is_a_nil.cr @@ -35,7 +35,10 @@ module Ameba::Rule::Style return unless path_named?(const, "Nil") issue_for const, MSG do |corrector| - corrector.replace(node, "#{node.obj}.nil?") + next unless location = node.location + next unless end_location = node.end_location + + corrector.replace(location, end_location, "#{node.obj}.nil?") end end end diff --git a/src/ameba/rule/style/parentheses_around_condition.cr b/src/ameba/rule/style/parentheses_around_condition.cr index a19dbabf7..009e72a24 100644 --- a/src/ameba/rule/style/parentheses_around_condition.cr +++ b/src/ameba/rule/style/parentheses_around_condition.cr @@ -57,7 +57,10 @@ module Ameba::Rule::Style if cond.is_a?(Crystal::Assign) && allow_safe_assignment? issue_for cond, MSG_MISSING do |corrector| - corrector.wrap(cond, '(', ')') + next unless location = cond.location + next unless end_location = cond.end_location + + corrector.wrap(location, end_location, '(', ')') end return end @@ -73,8 +76,11 @@ module Ameba::Rule::Style return unless strip_parentheses?(exp, is_ternary) issue_for cond, MSG_REDUNDANT do |corrector| - corrector.remove_trailing(cond, 1) - corrector.remove_leading(cond, 1) + next unless location = cond.location + next unless end_location = cond.end_location + + corrector.remove_trailing(location, end_location, 1) + corrector.remove_leading(location, end_location, 1) end end end diff --git a/src/ameba/rule/style/redundant_next.cr b/src/ameba/rule/style/redundant_next.cr index ce81d080f..9da71bc53 100644 --- a/src/ameba/rule/style/redundant_next.cr +++ b/src/ameba/rule/style/redundant_next.cr @@ -116,12 +116,12 @@ module Ameba::Rule::Style return if allow_multi_next? && node.exp.is_a?(Crystal::TupleLiteral) return if allow_empty_next? && (node.exp.nil? || node.exp.try(&.nop?)) - if exp_code = control_exp_code(node, source.lines) - issue_for node, MSG do |corrector| - corrector.replace(node, exp_code) - end - else - issue_for node, MSG + issue_for node, MSG do |corrector| + next unless location = node.location + next unless end_location = node.end_location + next unless exp_code = control_exp_code(node, source.lines) + + corrector.replace(location, end_location, exp_code) end end end diff --git a/src/ameba/rule/style/redundant_return.cr b/src/ameba/rule/style/redundant_return.cr index 234fd097e..a63179add 100644 --- a/src/ameba/rule/style/redundant_return.cr +++ b/src/ameba/rule/style/redundant_return.cr @@ -113,12 +113,12 @@ module Ameba::Rule::Style return if allow_multi_return? && node.exp.is_a?(Crystal::TupleLiteral) return if allow_empty_return? && (node.exp.nil? || node.exp.try(&.nop?)) - if exp_code = control_exp_code(node, source.lines) - issue_for node, MSG do |corrector| - corrector.replace(node, exp_code) - end - else - issue_for node, MSG + issue_for node, MSG do |corrector| + next unless location = node.location + next unless end_location = node.end_location + next unless exp_code = control_exp_code(node, source.lines) + + corrector.replace(location, end_location, exp_code) end end end diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index 42f9cfb9a..61975e677 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -52,17 +52,12 @@ module Ameba::Rule::Style def test(source, node : Crystal::Unless) 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| + next unless location = node.location + next unless cond_end_location = node.cond.end_location + next unless else_location = node.else_location + next unless end_location = node.end_location + keyword_begin_pos = source.pos(location) keyword_end_pos = keyword_begin_pos + {{ "unless".size }} keyword_range = keyword_begin_pos...keyword_end_pos diff --git a/src/ameba/source/corrector.cr b/src/ameba/source/corrector.cr index 33e722d23..a03a8e116 100644 --- a/src/ameba/source/corrector.cr +++ b/src/ameba/source/corrector.cr @@ -13,6 +13,11 @@ class Ameba::Source @rewriter = Rewriter.new(code) end + # Returns `true` if no (non trivial) update has been recorded + def empty? + @rewriter.empty? + end + # Replaces the code of the given range with *content*. def replace(location, end_location, content) @rewriter.replace(loc_to_pos(location), loc_to_pos(end_location) + 1, content) @@ -140,58 +145,6 @@ class Ameba::Source @line_sizes[0...line - 1].sum + (column - 1) end - # Replaces the code of the given node with *content*. - def replace(node : Crystal::ASTNode, content) - replace(location(node), end_location(node), content) - end - - # Inserts the given strings before and after the given node. - def wrap(node : Crystal::ASTNode, insert_before, insert_after) - wrap(location(node), end_location(node), insert_before, insert_after) - end - - # Shortcut for `replace(node, "")` - def remove(node : Crystal::ASTNode) - remove(location(node), end_location(node)) - end - - # Shortcut for `wrap(node, content, nil)` - def insert_before(node : Crystal::ASTNode, content) - insert_before(location(node), content) - end - - # Shortcut for `wrap(node, nil, content)` - def insert_after(node : Crystal::ASTNode, content) - insert_after(end_location(node), content) - end - - # Removes *size* characters prior to the given node. - def remove_preceding(node : Crystal::ASTNode, size) - remove_preceding(location(node), end_location(node), size) - end - - # Removes *size* characters from the beginning of the given node. - # If *size* is greater than the size of the node, the removed region can - # overrun the end of the node. - def remove_leading(node : Crystal::ASTNode, size) - remove_leading(location(node), end_location(node), size) - end - - # Removes *size* characters from the end of the given node. - # If *size* is greater than the size of the node, the removed region can - # overrun the beginning of the node. - def remove_trailing(node : Crystal::ASTNode, size) - remove_trailing(location(node), end_location(node), size) - end - - private def location(node : Crystal::ASTNode) - node.location || raise "Missing location" - end - - private def end_location(node : Crystal::ASTNode) - node.end_location || raise "Missing end location" - end - # Applies all scheduled changes and returns modified source as a new string. def process @rewriter.process