iv-org-mirror
/
shard-ameba
mirror of https://gitea.invidious.io/iv-org/shard-ameba.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #248 from FnControlOption/autocorrect 

Add autocorrect
This commit is contained in:
Sijawusz Pur Rahnama 2021-11-01 20:24:43 +01:00 committed by GitHub
parent 3e1df339ae 78071722c4
commit 7cb0c15747
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
29 changed files with 1040 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
spec/ameba/formatter/util_spec.cr
View File

@ -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

15
spec/ameba/issue_spec.cr
View File

@ -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: ""

22
spec/ameba/rule/layout/trailing_blank_lines_spec.cr
View File

@ -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

6
spec/ameba/rule/style/large_numbers_spec.cr
View File

@ -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

67
spec/ameba/runner_spec.cr
View File

@ -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

110
spec/ameba/source/rewriter_spec.cr Normal file
View File

@ -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

6
spec/ameba/spec/annotated_source_spec.cr
View File

@ -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

127
spec/spec_helper.cr
View File

@ -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)?

1
src/ameba.cr
View File

@ -2,6 +2,7 @@ require "./ameba/*"
require "./ameba/ast/**"
require "./ameba/rule/**"
require "./ameba/formatter/*"
require "./ameba/source/**"
# Ameba's entry module.
#

18
src/ameba/cli/cmd.cr
View File

@ -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

3
src/ameba/config.cr
View File

@ -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.

12
src/ameba/formatter/dot_formatter.cr
View File

@ -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

7
src/ameba/formatter/explain_formatter.cr
View File

@ -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

1
src/ameba/formatter/flycheck_formatter.cr
View File

@ -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",

1
src/ameba/formatter/json_formatter.cr
View File

@ -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

1
src/ameba/formatter/todo_formatter.cr
View File

@ -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

10
src/ameba/formatter/util.cr
View File

@ -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

13
src/ameba/issue.cr
View File

@ -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

55
src/ameba/reportable.cr
View File

@ -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.

4
src/ameba/rule/base.cr
View File

@ -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

8
src/ameba/rule/layout/trailing_blank_lines.cr
View File

@ -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

6
src/ameba/rule/style/large_numbers.cr
View File

@ -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

94
src/ameba/runner.cr
View File

@ -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)

14
src/ameba/source.cr
View File

@ -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

136
src/ameba/source/corrector.cr Normal file
View File

@ -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

132
src/ameba/source/rewriter.cr Normal file
View File

@ -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

179
src/ameba/source/rewriter/action.cr Normal file
View File

@ -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

3
src/ameba/spec/annotated_source.cr
View File

@ -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]

35
src/ameba/spec/expect_issue.cr
View File

@ -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)