diff --git a/spec/ameba/formatter/util_spec.cr b/spec/ameba/formatter/util_spec.cr index 9087228f..0ddcfb4b 100644 --- a/spec/ameba/formatter/util_spec.cr +++ b/spec/ameba/formatter/util_spec.cr @@ -69,24 +69,24 @@ module Ameba::Formatter describe "#affected_code" do it "returns nil if there is no such a line number" do - source = Source.new %( + code = <<-EOF a = 1 - ) + EOF location = Crystal::Location.new("filename", 2, 1) - subject.affected_code(source, location).should be_nil + subject.affected_code(code, location).should be_nil end it "returns correct line if it is found" do - source = Source.new %( + code = <<-EOF a = 1 - ) + EOF location = Crystal::Location.new("filename", 1, 1) - subject.deansify(subject.affected_code(source, location)) + subject.deansify(subject.affected_code(code, location)) .should eq "> a = 1\n ^\n" end it "returns correct line if it is found" do - source = Source.new <<-EOF + code = <<-EOF # pre:1 # pre:2 # pre:3 @@ -101,7 +101,7 @@ module Ameba::Formatter EOF location = Crystal::Location.new("filename", 6, 1) - subject.deansify(subject.affected_code(source, location, context_lines: 3)) + subject.deansify(subject.affected_code(code, location, context_lines: 3)) .should eq <<-STR > # pre:3 > # pre:4 diff --git a/spec/ameba/issue_spec.cr b/spec/ameba/issue_spec.cr index b6488552..08188eaf 100644 --- a/spec/ameba/issue_spec.cr +++ b/spec/ameba/issue_spec.cr @@ -3,7 +3,8 @@ require "../spec_helper" module Ameba describe Issue do it "accepts rule and message" do - issue = Issue.new rule: DummyRule.new, + issue = Issue.new code: "", + rule: DummyRule.new, location: nil, end_location: nil, message: "Blah", @@ -15,7 +16,8 @@ module Ameba it "accepts location" do location = Crystal::Location.new("path", 3, 2) - issue = Issue.new rule: DummyRule.new, + issue = Issue.new code: "", + rule: DummyRule.new, location: location, end_location: nil, message: "Blah", @@ -27,7 +29,8 @@ module Ameba it "accepts end_location" do location = Crystal::Location.new("path", 3, 2) - issue = Issue.new rule: DummyRule.new, + issue = Issue.new code: "", + rule: DummyRule.new, location: nil, end_location: location, message: "Blah", @@ -38,7 +41,8 @@ module Ameba end it "accepts status" do - issue = Issue.new rule: DummyRule.new, + issue = Issue.new code: "", + rule: DummyRule.new, location: nil, end_location: nil, message: "", @@ -50,7 +54,8 @@ module Ameba end it "sets status to :enabled by default" do - issue = Issue.new rule: DummyRule.new, + issue = Issue.new code: "", + rule: DummyRule.new, location: nil, end_location: nil, message: "" diff --git a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr index 4f42f9a6..babe9f05 100644 --- a/spec/ameba/rule/layout/trailing_blank_lines_spec.cr +++ b/spec/ameba/rule/layout/trailing_blank_lines_spec.cr @@ -5,28 +5,26 @@ module Ameba::Rule::Layout describe TrailingBlankLines do it "passes if there is a blank line at the end of a source" do - source = Source.new "a = 1\n", normalize: false - subject.catch(source).should be_valid + expect_no_issues subject, "a = 1\n", normalize: false end it "passes if source is empty" do - source = Source.new "" - subject.catch(source).should be_valid + expect_no_issues subject, "" end it "fails if there is no blank lines at the end" do - source = Source.new "no-blankline" - subject.catch(source).should_not be_valid + source = expect_issue subject, "no-blankline # error: Trailing newline missing" + expect_correction source, "no-blankline\n" end it "fails if there more then one blank line at the end of a source" do - source = Source.new "a = 1\n \n", normalize: false - subject.catch(source).should_not be_valid + source = expect_issue subject, "a = 1\n \n # error: Excessive trailing newline detected", normalize: false + expect_no_corrections source end it "fails if last line is not blank" do - source = Source.new "\n\n\n puts 22", normalize: false - subject.catch(source).should_not be_valid + source = expect_issue subject, "\n\n\n puts 22 # error: Trailing newline missing", normalize: false + expect_correction source, "\n\n\n puts 22\n" end context "when unnecessary blank line has been detected" do @@ -36,7 +34,7 @@ module Ameba::Rule::Layout issue = source.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:1" + issue.location.to_s.should eq "source.cr:3:1" issue.end_location.should be_nil issue.message.should eq "Excessive trailing newline detected" end @@ -49,7 +47,7 @@ module Ameba::Rule::Layout issue = source.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:0:1" + issue.location.to_s.should eq "source.cr:1:1" issue.end_location.should be_nil issue.message.should eq "Trailing newline missing" end diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index 87fe95c6..7397603f 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -7,10 +7,14 @@ module Ameba it "transforms large number #{number}" do rule = Rule::Style::LargeNumbers.new - expect_issue rule, <<-CRYSTAL, number: number + source = expect_issue rule, <<-CRYSTAL, number: number number = %{number} # ^{number} error: Large numbers should be written with underscores: #{expected} CRYSTAL + + expect_correction source, <<-CRYSTAL + number = #{expected} + CRYSTAL end end diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 703f39ee..f5ec70c7 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -8,6 +8,13 @@ module Ameba config.update_rule ErrorRule.rule_name, enabled: false config.update_rule PerfRule.rule_name, enabled: false + config.update_rule AtoAA.rule_name, enabled: false + config.update_rule AtoB.rule_name, enabled: false + config.update_rule BtoA.rule_name, enabled: false + config.update_rule BtoC.rule_name, enabled: false + config.update_rule CtoA.rule_name, enabled: false + config.update_rule ClassToModule.rule_name, enabled: false + config.update_rule ModuleToClass.rule_name, enabled: false Runner.new(config) end @@ -54,6 +61,15 @@ module Ameba Runner.new(all_rules, [source], formatter, default_severity).run.success?.should be_true end + it "aborts because of an infinite loop" do + rules = [AtoAA.new] of Rule::Base + source = Source.new "class A; end", "source.cr" + message = "Infinite loop in source.cr caused by Ameba/AtoAA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + end + end + context "exception in rule" do it "raises an exception raised in fiber while running a rule" do rule = RaiseRule.new @@ -180,5 +196,56 @@ module Ameba .run.success?.should be_true end end + + describe "#run with rules autocorrecting each other" do + context "with two conflicting rules" do + context "if there is an offense in an inspected file" do + it "aborts because of an infinite loop" do + rules = [AtoB.new, BtoA.new] + source = Source.new "class A; end", "source.cr" + message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + end + end + end + + context "if there are multiple offenses in an inspected file" do + it "aborts because of an infinite loop" do + rules = [AtoB.new, BtoA.new] + source = Source.new %( + class A; end + class A_A; end + ), "source.cr" + message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + end + end + end + end + + context "with two pairs of conflicting rules" do + it "aborts because of an infinite loop" do + rules = [ClassToModule.new, ModuleToClass.new, AtoB.new, BtoA.new] + source = Source.new "class A_A; end", "source.cr" + message = "Infinite loop in source.cr caused by Ameba/ClassToModule, Ameba/AtoB -> Ameba/ModuleToClass, Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + end + end + end + + context "with three rule cycle" do + it "aborts because of an infinite loop" do + rules = [AtoB.new, BtoC.new, CtoA.new] + source = Source.new "class A; end", "source.cr" + message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoC -> Ameba/CtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + end + end + end + end end end diff --git a/spec/ameba/source/rewriter_spec.cr b/spec/ameba/source/rewriter_spec.cr new file mode 100644 index 00000000..7d97bb9b --- /dev/null +++ b/spec/ameba/source/rewriter_spec.cr @@ -0,0 +1,110 @@ +require "../../spec_helper" + +class Ameba::Source + describe Rewriter do + code = "puts(:hello, :world)" + hello = {5, 11} + comma_space = {11, 13} + world = {13, 19} + + it "can remove" do + rewriter = Rewriter.new(code) + rewriter.remove(*hello) + rewriter.process.should eq "puts(, :world)" + end + + it "can insert before" do + rewriter = Rewriter.new(code) + rewriter.insert_before(*world, "42, ") + rewriter.process.should eq "puts(:hello, 42, :world)" + end + + it "can insert after" do + rewriter = Rewriter.new(code) + rewriter.insert_after(*hello, ", 42") + rewriter.process.should eq "puts(:hello, 42, :world)" + end + + it "can wrap" do + rewriter = Rewriter.new(code) + rewriter.wrap(*hello, '[', ']') + rewriter.process.should eq "puts([:hello], :world)" + end + + it "can replace" do + rewriter = Rewriter.new(code) + rewriter.replace(*hello, ":hi") + rewriter.process.should eq "puts(:hi, :world)" + end + + it "accepts crossing deletions" do + rewriter = Rewriter.new(code) + rewriter.remove(hello[0], comma_space[1]) + rewriter.remove(comma_space[0], world[1]) + rewriter.process.should eq "puts()" + end + + it "accepts multiple actions" do + rewriter = Rewriter.new(code) + rewriter.replace(*comma_space, " => ") + rewriter.wrap(hello[0], world[1], '{', '}') + rewriter.replace(*world, ":everybody") + rewriter.wrap(*world, '[', ']') + rewriter.process.should eq "puts({:hello => [:everybody]})" + end + + it "can wrap the same range" do + rewriter = Rewriter.new(code) + rewriter.wrap(*hello, '(', ')') + rewriter.wrap(*hello, '[', ']') + rewriter.process.should eq "puts([(:hello)], :world)" + end + + it "can insert on empty ranges" do + rewriter = Rewriter.new(code) + rewriter.insert_before(hello[0], '{') + rewriter.replace(hello[0], hello[0], 'x') + rewriter.insert_after(hello[0], '}') + rewriter.insert_before(hello[1], '[') + rewriter.replace(hello[1], hello[1], 'y') + rewriter.insert_after(hello[1], ']') + rewriter.process.should eq "puts({x}:hello[y], :world)" + end + + it "can replace the same range" do + rewriter = Rewriter.new(code) + rewriter.replace(*hello, ":hi") + rewriter.replace(*hello, ":hey") + rewriter.process.should eq "puts(:hey, :world)" + end + + it "can swallow insertions" do + rewriter = Rewriter.new(code) + rewriter.wrap(hello[0] + 1, hello[1], "__", "__") + rewriter.replace(world[0], world[1] - 2, "xx") + rewriter.replace(hello[0], world[1], ":hi") + rewriter.process.should eq "puts(:hi)" + end + + it "rejects out-of-range ranges" do + rewriter = Rewriter.new(code) + expect_raises(IndexError) { rewriter.insert_before(0, 100, "hola") } + end + + it "ignores trivial actions" do + rewriter = Rewriter.new(code) + rewriter.empty?.should be_true + + # This is a trivial wrap + rewriter.wrap(2, 5, "", "") + rewriter.empty?.should be_true + + # This is a trivial deletion + rewriter.remove(2, 2) + rewriter.empty?.should be_true + + rewriter.remove(2, 5) + rewriter.empty?.should be_false + end + end +end diff --git a/spec/ameba/spec/annotated_source_spec.cr b/spec/ameba/spec/annotated_source_spec.cr index 9f801f46..538f054b 100644 --- a/spec/ameba/spec/annotated_source_spec.cr +++ b/spec/ameba/spec/annotated_source_spec.cr @@ -1,6 +1,7 @@ require "../../spec_helper" -private def dummy_issue(message, +private def dummy_issue(code, + message, position : {Int32, Int32}?, end_position : {Int32, Int32}?, path = "") @@ -9,6 +10,7 @@ private def dummy_issue(message, end_location = Crystal::Location.new(path, *end_position) if end_position Ameba::Issue.new( + code: code, rule: Ameba::DummyRule.new, location: location, end_location: end_location, @@ -25,7 +27,7 @@ private def expect_invalid_location(code, expect_raises Exception, exception_message, file, line do Ameba::Spec::AnnotatedSource.new( lines: code.lines, - issues: [dummy_issue("Message", position, end_position, "path")] + issues: [dummy_issue(code, "Message", position, end_position, "path")] ) end end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index c468bed5..aa489fca 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -95,6 +95,133 @@ module Ameba end end + class AtoAA < Rule::Base + include AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) + return unless (name = node_source(node.name, source.lines)) + return unless name.includes?("A") + + issue_for(node.name, message: "A to AA") do |corrector| + corrector.replace(node.name, name.sub("A", "AA")) + end + end + end + + class AtoB < Rule::Base + include AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) + return unless (name = node_source(node.name, source.lines)) + return unless name.includes?("A") + + issue_for(node.name, message: "A to B") do |corrector| + corrector.replace(node.name, name.tr("A", "B")) + end + end + end + + class BtoA < Rule::Base + include AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) + return unless (name = node_source(node.name, source.lines)) + return unless name.includes?("B") + + issue_for(node.name, message: "B to A") do |corrector| + corrector.replace(node.name, name.tr("B", "A")) + end + end + end + + class BtoC < Rule::Base + include AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) + return unless (name = node_source(node.name, source.lines)) + return unless name.includes?("B") + + issue_for(node.name, message: "B to C") do |corrector| + corrector.replace(node.name, name.tr("B", "C")) + end + end + end + + class CtoA < Rule::Base + include AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) + return unless (name = node_source(node.name, source.lines)) + return unless name.includes?("C") + + issue_for(node.name, message: "C to A") do |corrector| + corrector.replace(node.name, name.tr("C", "A")) + end + end + end + + class ClassToModule < Ameba::Rule::Base + include Ameba::AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ClassDef) + return unless (location = node.location) + + end_location = Crystal::Location.new( + location.filename, + location.line_number, + location.column_number + "class".size - 1 + ) + issue_for(location, end_location, message: "class to module") do |corrector| + corrector.replace(location, end_location, "module") + end + end + end + + class ModuleToClass < Ameba::Rule::Base + include Ameba::AST::Util + + properties do + description "This rule is only used to test infinite loop detection" + end + + def test(source, node : Crystal::ModuleDef) + return unless (location = node.location) + + end_location = Crystal::Location.new( + location.filename, + location.line_number, + location.column_number + "module".size - 1 + ) + issue_for(location, end_location, message: "module to class") do |corrector| + corrector.replace(location, end_location, "class") + end + end + end + class DummyFormatter < Formatter::BaseFormatter property started_sources : Array(Source)? property finished_sources : Array(Source)? diff --git a/src/ameba.cr b/src/ameba.cr index f16c62e6..2a34d0ef 100644 --- a/src/ameba.cr +++ b/src/ameba.cr @@ -2,6 +2,7 @@ require "./ameba/*" require "./ameba/ast/**" require "./ameba/rule/**" require "./ameba/formatter/*" +require "./ameba/source/**" # Ameba's entry module. # diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 97e6ba2e..71051724 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -7,7 +7,15 @@ module Ameba::Cli def run(args = ARGV) opts = parse_args args + location_to_explain = opts.location_to_explain + autocorrect = opts.autocorrect? + + if location_to_explain && autocorrect + raise "Invalid usage: Cannot explain an issue and autocorrect at the same time." + end + config = Config.load opts.config, opts.colors? + config.autocorrect = autocorrect if globs = opts.globs config.globs = globs @@ -25,8 +33,8 @@ module Ameba::Cli runner = Ameba.run(config) - if location = opts.location_to_explain - runner.explain(location) + if location_to_explain + runner.explain(location_to_explain) else exit 1 unless runner.success? end @@ -47,6 +55,7 @@ module Ameba::Cli property? all = false property? colors = true property? without_affected_code = false + property? autocorrect = false end def parse_args(args, opts = Opts.new) @@ -89,6 +98,10 @@ module Ameba::Cli opts.all = true end + parser.on("--fix", "Autocorrect issues") do + opts.autocorrect = true + end + parser.on("--gen-config", "Generate a configuration file acting as a TODO list") do opts.formatter = :todo @@ -133,6 +146,7 @@ module Ameba::Cli if name = opts.formatter config.formatter = name end + config.formatter.config[:autocorrect] = opts.autocorrect? config.formatter.config[:without_affected_code] = opts.without_affected_code? end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 381f8725..91fad3cc 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -54,6 +54,9 @@ class Ameba::Config # ``` property excluded : Array(String) + # Returns true if correctable issues should be autocorrected. + property? autocorrect = false + @rule_groups : Hash(String, Array(Rule::Base)) # Creates a new instance of `Ameba::Config` based on YAML parameters. diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr index 121ea1cf..9f998ecd 100644 --- a/src/ameba/formatter/dot_formatter.cr +++ b/src/ameba/formatter/dot_formatter.cr @@ -36,13 +36,21 @@ module Ameba::Formatter next if issue.disabled? next if (location = issue.location).nil? - output.puts location.colorize(:cyan) + output.print location.colorize(:cyan) + if issue.correctable? + if config[:autocorrect]? + output.print " [Corrected]".colorize(:green) + else + output.print " [Correctable]".colorize(:yellow) + end + end + output.puts output.puts \ "[#{issue.rule.severity.symbol}] " \ "#{issue.rule.name}: " \ "#{issue.message}".colorize(:red) - if show_affected_code && (code = affected_code(source, location, issue.end_location)) + if show_affected_code && (code = affected_code(issue)) output << code.colorize(:default) end diff --git a/src/ameba/formatter/explain_formatter.cr b/src/ameba/formatter/explain_formatter.cr index f8fe87c1..33c8b7d5 100644 --- a/src/ameba/formatter/explain_formatter.cr +++ b/src/ameba/formatter/explain_formatter.cr @@ -38,10 +38,7 @@ module Ameba::Formatter private def explain(source, issue) rule = issue.rule - location, end_location = - issue.location, issue.end_location - - return unless location + return unless (location = issue.location) output_title "ISSUE INFO" output_paragraph [ @@ -49,7 +46,7 @@ module Ameba::Formatter location.to_s.colorize(:cyan).to_s, ] - if affected_code = affected_code(source, location, end_location, context_lines: 3) + if affected_code = affected_code(issue, context_lines: 3) output_title "AFFECTED CODE" output_paragraph affected_code end diff --git a/src/ameba/formatter/flycheck_formatter.cr b/src/ameba/formatter/flycheck_formatter.cr index 50d5de6c..63039b82 100644 --- a/src/ameba/formatter/flycheck_formatter.cr +++ b/src/ameba/formatter/flycheck_formatter.cr @@ -5,6 +5,7 @@ module Ameba::Formatter def source_finished(source : Source) source.issues.each do |e| next if e.disabled? + next if e.correctable? && config[:autocorrect]? if loc = e.location @mutex.synchronize do output.printf "%s:%d:%d: %s: [%s] %s\n", diff --git a/src/ameba/formatter/json_formatter.cr b/src/ameba/formatter/json_formatter.cr index 211dabb9..da16fe1f 100644 --- a/src/ameba/formatter/json_formatter.cr +++ b/src/ameba/formatter/json_formatter.cr @@ -77,6 +77,7 @@ module Ameba::Formatter source.issues.each do |e| next if e.disabled? + next if e.correctable? && config[:autocorrect]? json_source.issues << AsJSON::Issue.new(e.rule.name, e.rule.severity.to_s, e.location, e.end_location, e.message) @result.summary.issues_count += 1 end diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr index 47314407..8c8524f9 100644 --- a/src/ameba/formatter/todo_formatter.cr +++ b/src/ameba/formatter/todo_formatter.cr @@ -42,6 +42,7 @@ module Ameba::Formatter Hash(Rule::Base, Array(Issue)).new.tap do |h| issues.each do |issue| next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax) + next if issue.correctable? && config[:autocorrect]? (h[issue.rule] ||= Array(Issue).new) << issue end end diff --git a/src/ameba/formatter/util.cr b/src/ameba/formatter/util.cr index d6c411c8..cbf0907c 100644 --- a/src/ameba/formatter/util.cr +++ b/src/ameba/formatter/util.cr @@ -40,8 +40,14 @@ module Ameba::Formatter {pre_context, post_context} end - def affected_code(source, location, end_location = nil, context_lines = 0, max_length = 120, ellipsis = " ...", prompt = "> ") - lines = source.lines + def affected_code(issue : Issue, context_lines = 0, max_length = 120, ellipsis = " ...", prompt = "> ") + return unless (location = issue.location) + + affected_code(issue.code, location, issue.end_location, context_lines, max_length, ellipsis, prompt) + end + + def affected_code(code, location, end_location = nil, context_lines = 0, max_length = 120, ellipsis = " ...", prompt = "> ") + lines = code.split('\n') # must preserve trailing newline lineno, column = location.line_number, location.column_number diff --git a/src/ameba/issue.cr b/src/ameba/issue.cr index b913eac3..b6fa55e8 100644 --- a/src/ameba/issue.cr +++ b/src/ameba/issue.cr @@ -6,6 +6,9 @@ module Ameba Disabled end + # The source code that triggered this issue. + getter code : String + # A rule that triggers this issue. getter rule : Rule::Base @@ -24,12 +27,20 @@ module Ameba delegate :enabled?, :disabled?, to: status - def initialize(@rule, @location, @end_location, @message, status : Status? = nil) + def initialize(@code, @rule, @location, @end_location, @message, status : Status? = nil, @block : (Source::Corrector ->)? = nil) @status = status || Status::Enabled end def syntax? rule.is_a?(Rule::Lint::Syntax) end + + def correctable? + !@block.nil? + end + + def correct(corrector) + @block.try &.call(corrector) + end end end diff --git a/src/ameba/reportable.cr b/src/ameba/reportable.cr index 4632fa91..ec9e77a5 100644 --- a/src/ameba/reportable.cr +++ b/src/ameba/reportable.cr @@ -5,41 +5,76 @@ module Ameba getter issues = [] of Issue # Adds a new issue to the list of issues. - def add_issue(rule, location, end_location, message, status : Issue::Status? = nil) : Issue + def add_issue(rule, location, end_location, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil) : Issue status ||= Issue::Status::Disabled if location_disabled?(location, rule) - Issue.new(rule, location, end_location, message, status).tap do |issue| + Issue.new(code, rule, location, end_location, message, status, block).tap do |issue| issues << issue end end + # :ditto: + def add_issue(rule, location, end_location, message, status : Issue::Status? = nil, &block : Source::Corrector ->) : Issue + add_issue rule, location, end_location, message, status, block + end + # Adds a new issue for Crystal AST *node*. - def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil) : Issue - add_issue rule, node.location, node.end_location, message, status + def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil) : Issue + add_issue rule, node.location, node.end_location, message, status, block + end + + # :ditto: + def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, &block : Source::Corrector ->) : Issue + add_issue rule, node, message, status, block end # Adds a new issue for Crystal *token*. - def add_issue(rule, token : Crystal::Token, message, status : Issue::Status? = nil) : Issue - add_issue rule, token.location, nil, message, status + def add_issue(rule, token : Crystal::Token, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil) : Issue + add_issue rule, token.location, nil, message, status, block + end + + # :ditto: + def add_issue(rule, token : Crystal::Token, message, status : Issue::Status? = nil, &block : Source::Corrector ->) : Issue + add_issue rule, token, message, status, block end # Adds a new issue for *location* defined by line and column numbers. - def add_issue(rule, location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue + def add_issue(rule, location : {Int32, Int32}, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil) : Issue location = Crystal::Location.new(path, *location) - add_issue rule, location, nil, message, status + add_issue rule, location, nil, message, status, block + end + + # :ditto: + def add_issue(rule, location : {Int32, Int32}, message, status : Issue::Status? = nil, &block : Source::Corrector ->) : Issue + add_issue rule, location, message, status, block end # Adds a new issue for *location* and *end_location* defined by line and column numbers. - def add_issue(rule, location : {Int32, Int32}, end_location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue + def add_issue(rule, + location : {Int32, Int32}, + end_location : {Int32, Int32}, + message, + status : Issue::Status? = nil, + block : (Source::Corrector ->)? = nil) : Issue location = Crystal::Location.new(path, *location) end_location = Crystal::Location.new(path, *end_location) - add_issue rule, location, end_location, message, status + add_issue rule, location, end_location, message, status, block + end + + # :ditto: + def add_issue(rule, + location : {Int32, Int32}, + end_location : {Int32, Int32}, + message, + status : Issue::Status? = nil, + &block : Source::Corrector ->) : Issue + add_issue rule, location, end_location, message, status, block end # Returns `true` if the list of not disabled issues is empty, `false` otherwise. diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 7777167c..0ae7016e 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -112,8 +112,8 @@ module Ameba::Rule name.hash end - macro issue_for(*args) - source.add_issue self, {{*args}} + macro issue_for(*args, **kwargs, &block) + source.add_issue(self, {{*args}}, {{**kwargs}}) {{block}} end protected def self.rule_name diff --git a/src/ameba/rule/layout/trailing_blank_lines.cr b/src/ameba/rule/layout/trailing_blank_lines.cr index a133ebb8..63e5327b 100644 --- a/src/ameba/rule/layout/trailing_blank_lines.cr +++ b/src/ameba/rule/layout/trailing_blank_lines.cr @@ -25,7 +25,13 @@ module Ameba::Rule::Layout last_line_empty = last_source_line.empty? if source_lines_size >= 1 && (source_lines.last(2).join.blank? || !last_line_empty) - issue_for({source_lines_size - 1, 1}, last_line_empty ? MSG : MSG_FINAL_NEWLINE) + if last_line_empty + issue_for({source_lines_size, 1}, MSG) + else + issue_for({source_lines_size, 1}, MSG_FINAL_NEWLINE) do |corrector| + corrector.insert_before({source_lines_size + 1, 1}, '\n') + end + end end end end diff --git a/src/ameba/rule/style/large_numbers.cr b/src/ameba/rule/style/large_numbers.cr index 8608abe3..4c2de7ff 100644 --- a/src/ameba/rule/style/large_numbers.cr +++ b/src/ameba/rule/style/large_numbers.cr @@ -28,7 +28,7 @@ module Ameba::Rule::Style # ``` class LargeNumbers < Base properties do - enabled false + enabled true description "Disallows usage of large numbers without underscore" int_min_digits 5 end @@ -48,7 +48,9 @@ module Ameba::Rule::Style location.line_number, location.column_number + token.raw.size - 1 ) - issue_for location, end_location, MSG % expected + issue_for location, end_location, MSG % expected do |corrector| + corrector.replace(location, end_location, expected) + end end end end diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index 56b06e64..dcc9f0ec 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -1,3 +1,5 @@ +require "digest" + module Ameba # Represents a runner for inspecting sources files. # Holds a list of rules to do inspection based on, @@ -11,6 +13,24 @@ module Ameba # ``` # class Runner + # An error indicating that the inspection loop got stuck correcting + # issues back and forth. + class InfiniteCorrectionLoopError < RuntimeError + def initialize(path, issues_by_iteration, loop_start = -1) + root_cause = + issues_by_iteration[loop_start..-1] + .join(" -> ", &.map(&.rule.name).uniq!.join(", ")) + + message = String.build do |io| + io << "Infinite loop" + io << " in " << path unless path.empty? + io << " caused by " << root_cause + end + + super message + end + end + # A list of rules to do inspection based on. @rules : Array(Rule::Base) @@ -29,6 +49,9 @@ module Ameba # Checks for unneeded disable directives. Always inspects a source last @unneeded_disable_directive_rule : Rule::Base? + # Returns true if correctable issues should be autocorrected. + private getter? autocorrect : Bool + # Instantiates a runner using a `config`. # # ``` @@ -43,13 +66,14 @@ module Ameba @formatter = config.formatter @severity = config.severity @rules = config.rules.select(&.enabled).reject!(&.special?) + @autocorrect = config.autocorrect? @unneeded_disable_directive_rule = config.rules .find &.name.==(Rule::Lint::UnneededDisableDirective.rule_name) end - protected def initialize(@rules, @sources, @formatter, @severity) + protected def initialize(@rules, @sources, @formatter, @severity, @autocorrect = false) end # Performs the inspection. Iterates through all sources and test it using @@ -89,14 +113,40 @@ module Ameba private def run_source(source) @formatter.source_started source - if @syntax_rule.catch(source).valid? + # This variable is a 2D array used to track corrected issues after each + # inspection iteration. This is used to output meaningful infinite loop + # error message. + corrected_issues = [] of Array(Issue) + + # When running with --fix, we need to inspect the source until no more + # corrections are made (because automatic corrections can introduce new + # issues). In the normal case the loop is only executed once. + loop_unless_infinite(source, corrected_issues) do + # We have to reprocess the source to pick up any changes. Since a + # change could (theoretically) introduce syntax errors, we break the + # loop if we find any. + @syntax_rule.test(source) + break unless source.valid? + @rules.each do |rule| next if rule.excluded?(source) rule.test(source) end check_unneeded_directives(source) + break unless autocorrect? && source.correct + + # The issues that couldn't be corrected will be found again so we + # only keep the corrected ones in order to avoid duplicate reporting. + corrected_issues << source.issues.select(&.correctable?) + source.issues.clear end + corrected_issues.flatten.reverse_each do |issue| + source.issues.unshift(issue) + end + + File.write(source.path, source.code) unless corrected_issues.empty? + ensure @formatter.source_finished source end @@ -130,6 +180,46 @@ module Ameba end end + private MAX_ITERATIONS = 200 + + private def loop_unless_infinite(source, corrected_issues) + # Keep track of the state of the source. If a rule modifies the source + # and another rule undoes it producing identical source we have an + # infinite loop. + processed_sources = [] of String + + # It is possible for a rule to keep adding indefinitely to a file, + # making it bigger and bigger. If the inspection loop runs for an + # excessively high number of iterations, this is likely happening. + iterations = 0 + + loop do + check_for_infinite_loop(source, corrected_issues, processed_sources) + + if (iterations += 1) > MAX_ITERATIONS + raise InfiniteCorrectionLoopError.new(source.path, corrected_issues) + end + + yield + end + end + + # Check whether a run created source identical to a previous run, which + # means that we definitely have an infinite loop. + private def check_for_infinite_loop(source, corrected_issues, processed_sources) + checksum = Digest::SHA1.hexdigest(source.code) + + if loop_start = processed_sources.index(checksum) + raise InfiniteCorrectionLoopError.new( + source.path, + corrected_issues, + loop_start: loop_start + ) + end + + processed_sources << checksum + end + private def check_unneeded_directives(source) if (rule = @unneeded_disable_directive_rule) && rule.enabled rule.test(source) diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 68dd2a1c..d6e0b97d 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -22,6 +22,20 @@ module Ameba def initialize(@code, @path = "") end + # Corrects any correctable issues and updates `code`. + # Returns `false` if no issues were corrected. + def correct + corrector = Corrector.new(code) + issues.each(&.correct(corrector)) + corrected_code = corrector.process + return false if code == corrected_code + + @code = corrected_code + @lines = nil + @ast = nil + true + end + # Returns lines of code splitted by new line character. # Since `code` is immutable and can't be changed, this # method caches lines in an instance variable, so calling diff --git a/src/ameba/source/corrector.cr b/src/ameba/source/corrector.cr new file mode 100644 index 00000000..97c46e24 --- /dev/null +++ b/src/ameba/source/corrector.cr @@ -0,0 +1,136 @@ +require "./rewriter" + +class Ameba::Source + # This class takes source code and rewrites it based + # on the different correction actions supplied. + class Corrector + @line_sizes = [] of Int32 + + def initialize(code : String) + code.each_line(chomp: false) do |line| + @line_sizes << line.size + end + @rewriter = Rewriter.new(code) + 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) + 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 + + # 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 `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(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 `insert_before(location, location, content)` + def insert_before(location, content) + @rewriter.insert_before(loc_to_pos(location), content) + end + + # Shortcut for `insert_after(location, location, content)` + def insert_after(location, content) + @rewriter.insert_after(loc_to_pos(location) + 1, 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 + + # 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. + def remove_leading(location, end_location, size) + remove(loc_to_pos(location), loc_to_pos(location) + 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. + def remove_trailing(location, end_location, size) + remove(loc_to_pos(end_location) + 1 - size, loc_to_pos(end_location) + 1) + 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 + else + line, column = location + end + @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 + end + end +end diff --git a/src/ameba/source/rewriter.cr b/src/ameba/source/rewriter.cr new file mode 100644 index 00000000..5eda2d6a --- /dev/null +++ b/src/ameba/source/rewriter.cr @@ -0,0 +1,132 @@ +class Ameba::Source + # This class performs the heavy lifting in the source rewriting process. + # It schedules code updates to be performed in the correct order. + # + # For simple cases, the resulting source will be obvious. + # + # Examples for more complex cases follow. Assume these examples are acting on + # the source `puts(:hello, :world)`. The methods `#wrap`, `#remove`, etc. + # receive a range as the first two arguments; for clarity, examples below use + # English sentences and a string of raw code instead. + # + # ## Overlapping deletions: + # + # * remove `:hello, ` + # * remove `, :world` + # + # The overlapping ranges are merged and `:hello, :world` will be removed. + # + # ## Multiple actions at the same end points: + # + # Results will always be independent of the order they were given. + # Exception: rewriting actions done on exactly the same range (covered next). + # + # Example: + # + # * replace `, ` by ` => ` + # * wrap `:hello, :world` with `{` and `}` + # * replace `:world` with `:everybody` + # * wrap `:world` with `[`, `]` + # + # The resulting string will be `puts({:hello => [:everybody]})` + # and this result is independent of the order the instructions were given in. + # + # ## Multiple wraps on same range: + # + # * wrap `:hello` with `(` and `)` + # * wrap `:hello` with `[` and `]` + # + # The wraps are combined in order given and results would be `puts([(:hello)], :world)`. + # + # ## Multiple replacements on same range: + # + # * replace `:hello` by `:hi`, then + # * replace `:hello` by `:hey` + # + # The replacements are made in the order given, so the latter replacement + # supersedes the former and `:hello` will be replaced by `:hey`. + # + # ## Swallowed insertions: + # + # * wrap `world` by `__`, `__` + # * replace `:hello, :world` with `:hi` + # + # A containing replacement will swallow the contained rewriting actions + # and `:hello, :world` will be replaced by `:hi`. + # + # ## Implementation + # + # The updates are organized in a tree, according to the ranges they act on + # (where children are strictly contained by their parent). + class Rewriter + getter code : String + + def initialize(@code) + @action_root = Rewriter::Action.new(0, code.size) + end + + # Returns true if no (non trivial) update has been recorded + def empty? + @action_root.empty? + end + + # Replaces the code of the given range with *content*. + def replace(begin_pos, end_pos, content) + combine(begin_pos, end_pos, replacement: content.to_s) + end + + # Inserts the given strings before and after the given range. + def wrap(begin_pos, end_pos, insert_before, insert_after) + combine(begin_pos, end_pos, insert_before: insert_before.to_s, insert_after: insert_after.to_s) + end + + # Shortcut for `replace(begin_pos, end_pos, "")` + def remove(begin_pos, end_pos) + replace(begin_pos, end_pos, "") + end + + # Shortcut for `wrap(begin_pos, end_pos, content, nil)` + def insert_before(begin_pos, end_pos, content) + wrap(begin_pos, end_pos, content, nil) + end + + # Shortcut for `wrap(begin_pos, end_pos, nil, content)` + def insert_after(begin_pos, end_pos, content) + wrap(begin_pos, end_pos, nil, content) + end + + # Shortcut for `insert_before(pos, pos, content)` + def insert_before(pos, content) + insert_before(pos, pos, content) + end + + # Shortcut for `insert_after(pos, pos, content)` + def insert_after(pos, content) + insert_after(pos, pos, content) + end + + # Applies all scheduled changes and returns modified source as a new string. + def process + String.build do |io| + last_end = 0 + @action_root.ordered_replacements.each do |begin_pos, end_pos, replacement| + io << code[last_end...begin_pos] << replacement + last_end = end_pos + end + io << code[last_end...code.size] + end + end + + protected def combine(begin_pos, end_pos, **attributes) + check_range_validity(begin_pos, end_pos) + action = Rewriter::Action.new(begin_pos, end_pos, **attributes) + @action_root = @action_root.combine(action) + end + + private def check_range_validity(begin_pos, end_pos) + if begin_pos < 0 || end_pos > code.size + raise IndexError.new("The range #{begin_pos}...#{end_pos} is outside the bounds of the source") + end + end + end +end diff --git a/src/ameba/source/rewriter/action.cr b/src/ameba/source/rewriter/action.cr new file mode 100644 index 00000000..23a009ec --- /dev/null +++ b/src/ameba/source/rewriter/action.cr @@ -0,0 +1,179 @@ +class Ameba::Source::Rewriter + # :nodoc: + # Actions are arranged in a tree and get combined so that: + # - children are strictly contained by their parent + # - siblings all disjoint from one another and ordered + # - only actions with `replacement == nil` may have children + class Action + getter begin_pos : Int32 + getter end_pos : Int32 + getter replacement : String? + getter insert_before : String + getter insert_after : String + protected getter children : Array(Action) + + def initialize(@begin_pos, + @end_pos, + @insert_before = "", + @replacement = nil, + @insert_after = "", + @children = [] of Action) + end + + def combine(action) + return self if action.empty? # Ignore empty action + + if action.begin_pos == @begin_pos && action.end_pos == @end_pos + merge(action) + else + place_in_hierarchy(action) + end + end + + def empty? + replacement = @replacement + @insert_before.empty? && + @insert_after.empty? && + @children.empty? && + (replacement.nil? || (replacement.empty? && @begin_pos == @end_pos)) + end + + def ordered_replacements + replacement = @replacement + reps = [] of {Int32, Int32, String} + reps << {@begin_pos, @begin_pos, @insert_before} unless @insert_before.empty? + reps << {@begin_pos, @end_pos, replacement} if replacement + reps.concat(@children.flat_map(&.ordered_replacements)) + reps << {@end_pos, @end_pos, @insert_after} unless @insert_after.empty? + reps + end + + def insertion? + replacement = @replacement + !@insert_before.empty? || !@insert_after.empty? || (replacement && !replacement.empty?) + end + + protected def with(*, + begin_pos = @begin_pos, + end_pos = @end_pos, + insert_before = @insert_before, + replacement = @replacement, + insert_after = @insert_after, + children = @children) + children = [] of Action if replacement + self.class.new(begin_pos, end_pos, insert_before, replacement, insert_after, children) + end + + protected def place_in_hierarchy(action) + family = analyse_hierarchy(action) + sibling_left, sibling_right = family[:sibling_left], family[:sibling_right] + + if fusible = family[:fusible] + child = family[:child] + child ||= [] of Action + fuse_deletions(action, fusible, sibling_left + child + sibling_right) + else + extra_sibling = + case + when parent = family[:parent] + # action should be a descendant of one of the children + parent.combine(action) + when child = family[:child] + # or it should become the parent of some of the children, + action.with(children: child).combine_children(action.children) + else + # or else it should become an additional child + action + end + self.with(children: sibling_left + [extra_sibling] + sibling_right) + end + end + + # Assumes *more_children* all contained within `@begin_pos...@end_pos` + protected def combine_children(more_children) + more_children.reduce(self) do |parent, new_child| + parent.place_in_hierarchy(new_child) + end + end + + protected def fuse_deletions(action, fusible, other_siblings) + without_fusible = self.with(children: other_siblings) + fusible = [action] + fusible + fused_begin_pos = fusible.min_of(&.begin_pos) + fused_end_pos = fusible.max_of(&.end_pos) + fused_deletion = action.with(begin_pos: fused_begin_pos, end_pos: fused_end_pos) + without_fusible.combine(fused_deletion) + end + + # Similar to `@children.bsearch_index || size` except allows for a starting point + protected def bsearch_child_index(from = 0) + size = @children.size + (from...size).bsearch { |i| yield @children[i] } || size + end + + # Returns the children in a hierarchy with respect to *action*: + # + # - `:sibling_left`, `:sibling_right` (for those that are disjoint from *action*) + # - `:parent` (in case one of our children contains *action*) + # - `:child` (in case *action* strictly contains some of our children) + # - `:fusible` (in case *action* overlaps some children but they can be fused in one deletion) + # + # In case a child has equal range to *action*, it is returned as `:parent` + # + # Reminder: an empty range 1...1 is considered disjoint from 1...10 + protected def analyse_hierarchy(action) # ameba:disable Metrics/CyclomaticComplexity + # left_index is the index of the first child that isn't completely to the left of action + left_index = bsearch_child_index { |child| child.end_pos > action.begin_pos } + # right_index is the index of the first child that is completely on the right of action + start = left_index == 0 ? 0 : left_index - 1 # See "corner case" below for reason of -1 + right_index = bsearch_child_index(start) { |child| child.begin_pos >= action.end_pos } + center = right_index - left_index + case center + when 0 + # All children are disjoint from action, nothing else to do + when -1 + # Corner case: if a child has empty range == action's range + # then it will appear to be both disjoint and to the left of action, + # as well as disjoint and to the right of action. + # Since ranges are equal, we return it as parent + left_index -= 1 # Fix indices, as otherwise this child would be + right_index += 1 # considered as a sibling (both left and right!) + parent = @children[left_index] + else + overlap_left = @children[left_index].begin_pos <=> action.begin_pos + overlap_right = @children[right_index - 1].end_pos <=> action.end_pos + raise "Unable to compare begin pos" if overlap_left.nil? + raise "Unable to compare end pos" if overlap_right.nil? + + # For one child to be the parent of action, we must have: + if center == 1 && overlap_left <= 0 && overlap_right >= 0 + parent = @children[left_index] + else + # Otherwise consider all non disjoint elements (center) to be contained... + contained = @children[left_index...right_index] + fusible = [] of Action + fusible << contained.shift if overlap_left < 0 # ... but check first and last one + fusible << contained.pop if overlap_right > 0 # ... for overlaps + fusible = nil if fusible.empty? + end + end + + { + parent: parent, + sibling_left: @children[0...left_index], + sibling_right: @children[right_index...@children.size], + fusible: fusible, + child: contained, + } + end + + # Assumes *action* has the exact same range and has no children + protected def merge(action) + self.with( + insert_before: "#{action.insert_before}#{insert_before}", + replacement: action.replacement || @replacement, + insert_after: "#{insert_after}#{action.insert_after}", + ).combine_children(action.children) + end + end +end diff --git a/src/ameba/spec/annotated_source.cr b/src/ameba/spec/annotated_source.cr index 26d5271e..76081fe7 100644 --- a/src/ameba/spec/annotated_source.cr +++ b/src/ameba/spec/annotated_source.cr @@ -15,7 +15,8 @@ class Ameba::Spec::AnnotatedSource def self.parse(annotated_code) lines = [] of String annotations = [] of {Int32, String, String} - annotated_code.each_line do |code_line| + code_lines = annotated_code.split('\n') # must preserve trailing newline + code_lines.each do |code_line| if (annotation_match = ANNOTATION_PATTERN_1.match(code_line)) message_index = annotation_match.end prefix = code_line[0...message_index] diff --git a/src/ameba/spec/expect_issue.cr b/src/ameba/spec/expect_issue.cr index fd8d1871..1167b8a3 100644 --- a/src/ameba/spec/expect_issue.cr +++ b/src/ameba/spec/expect_issue.cr @@ -65,7 +65,7 @@ module Ameba::Spec::ExpectIssue raise "Use `report_no_issues` to assert that no issues are found" end - actual_annotations = actual_annotations(rules, code, path, lines) + source, actual_annotations = actual_annotations(rules, code, path, lines) unless actual_annotations == expected_annotations fail <<-MSG, file, line Expected: @@ -77,6 +77,33 @@ module Ameba::Spec::ExpectIssue #{actual_annotations} MSG end + + source + end + + def expect_correction(source, correction, *, file = __FILE__, line = __LINE__) + raise "Use `expect_no_corrections` if the code will not change" unless source.correct + return if correction == source.code + + fail <<-MSG, file, line + Expected correction: + + #{correction} + + Got: + + #{source.code} + MSG + end + + def expect_no_corrections(source, *, file = __FILE__, line = __LINE__) + return unless source.correct + + fail <<-MSG, file, line + Expected no corrections, but got: + + #{source.code} + MSG end def expect_no_issues(rules : Rule::Base | Enumerable(Rule::Base), @@ -87,8 +114,8 @@ module Ameba::Spec::ExpectIssue file = __FILE__, line = __LINE__) code = normalize_code(code) if normalize - lines = code.lines - actual_annotations = actual_annotations(rules, code, path, lines) + lines = code.split('\n') # must preserve trailing newline + _, actual_annotations = actual_annotations(rules, code, path, lines) unless actual_annotations.to_s == code fail <<-MSG, file, line Expected no issues, but got: @@ -105,7 +132,7 @@ module Ameba::Spec::ExpectIssue else rules.catch(source) end - AnnotatedSource.new(lines, source.issues) + {source, AnnotatedSource.new(lines, source.issues)} end private def format_issue(code, **replacements)