Let ameba explain the issue at the specified location (#86)

This commit is contained in:
Vitalii Elenhaupt 2018-12-27 23:34:10 +02:00 committed by GitHub
parent 4e19571fb3
commit c91da1aa08
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 359 additions and 22 deletions

View file

@ -126,6 +126,19 @@ $ ameba --except Lint/Syntax # runs all rules except Lint/Syntax
$ ameba --except Style,Lint # runs all rules except rules in Style and Lint groups
```
### Explanation
Ameba allows you to dig deeper into an issue, by showing you details about the issue
and the reasoning by it being reported.
To be convenient, you can just copy-paste the `PATH:line:column` string from the
report and paste behind the `ameba` command to check it out.
```
$ ameba crystal/command/format.cr:26:83 # show explanation for the issue
$ ameba --explain crystal/command/format.cr:26:83 # same thing
```
### Inline disabling
One or more rules or one or more group of rules can be disabled using inline directives:

View file

@ -78,11 +78,57 @@ module Ameba::Cli
c.config.should eq ""
end
describe "-e/--explain" do
it "configures file/line/column" do
c = Cli.parse_args %w(--explain src/file.cr:3:5)
c.location_to_explain.should_not be_nil
location_to_explain = c.location_to_explain.not_nil!
location_to_explain[:file].should eq "src/file.cr"
location_to_explain[:line].should eq 3
location_to_explain[:column].should eq 5
end
it "raises an error if location is not valid" do
expect_raises(Exception, "location should have PATH:line:column") do
Cli.parse_args %w(--explain src/file.cr:3)
end
end
it "raises an error if line number is not valid" do
expect_raises(Exception, "location should have PATH:line:column") do
Cli.parse_args %w(--explain src/file.cr:a:3)
end
end
it "raises an error if column number is not valid" do
expect_raises(Exception, "location should have PATH:line:column") do
Cli.parse_args %w(--explain src/file.cr:3:&)
end
end
it "raises an error if line/column are missing" do
expect_raises(Exception, "location should have PATH:line:column") do
Cli.parse_args %w(--explain src/file.cr)
end
end
end
it "accepts unknown args as files" do
c = Cli.parse_args %w(source1.cr source2.cr)
c.files.should eq %w(source1.cr source2.cr)
end
it "accepts one unknown arg as explain location if it has correct format" do
c = Cli.parse_args %w(source.cr:3:22)
c.location_to_explain.should_not be_nil
location_to_explain = c.location_to_explain.not_nil!
location_to_explain[:file].should eq "source.cr"
location_to_explain[:line].should eq 3
location_to_explain[:column].should eq 22
end
it "allows args to be blank" do
c = Cli.parse_args [] of String
c.formatter.should be_nil

View file

@ -0,0 +1,70 @@
require "../../spec_helper"
module Ameba
def explanation(source)
output = IO::Memory.new
ErrorRule.new.catch(source)
location = {file: "source.cr", line: 1, column: 1}
Formatter::ExplainFormatter.new(output, location).finished([source])
output.to_s
end
describe Formatter::ExplainFormatter do
describe "#location" do
it "returns crystal location" do
location = Formatter::ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8}).location
location.is_a?(Crystal::Location).should be_true
location.filename.should eq "compiler.cr"
location.line_number.should eq 3
location.column_number.should eq 8
end
end
describe "#output" do
it "returns io" do
output = Formatter::ExplainFormatter
.new(STDOUT, {file: "compiler.cr", line: 3, column: 8}).output
output.should eq STDOUT
end
end
describe "#finished" do
it "writes issue info" do
source = Source.new "a = 42", "source.cr"
output = explanation(source)
output.should contain "ISSUE INFO"
output.should contain "This rule always adds an error"
output.should contain "source.cr:1:1"
end
it "writes affected code" do
source = Source.new "a = 42", "source.cr"
output = explanation(source)
output.should contain "AFFECTED CODE"
output.should contain "a = 42"
end
it "writes rule info" do
source = Source.new "a = 42", "source.cr"
output = explanation(source)
output.should contain "RULE INFO"
output.should contain "Ameba/ErrorRule"
output.should contain "Always adds an error at 1:1"
end
it "writes detailed description" do
source = Source.new "a = 42", "source.cr"
output = explanation(source)
output.should contain "DETAILED DESCRIPTION"
output.should contain "TO BE DONE..."
end
it "writes nothing if location not found" do
source = Source.new "a = 42", "another_source.cr"
explanation(source).should be_empty
end
end
end
end

View file

@ -0,0 +1,29 @@
require "../../spec_helper"
module Ameba::Formatter
class Subject
include Util
end
subject = Subject.new
describe Util do
describe "#affected_code" do
it "returns nil if there is no such a line number" do
source = Source.new %(
a = 1
)
location = Crystal::Location.new("filename", 2, 1)
subject.affected_code(source, location).should be_nil
end
it "returns correct line if it is found" do
source = Source.new %(
a = 1
)
location = Crystal::Location.new("filename", 1, 1)
subject.affected_code(source, location).should eq "> a = 1\n \e[33m^\e[0m"
end
end
end
end

View file

@ -90,6 +90,41 @@ module Ameba
end
end
describe "#explain" do
io = IO::Memory.new
it "writes nothing if sources are valid" do
io.clear
runner = runner(formatter: formatter).run
runner.explain({file: "source.cr", line: 1, column: 2}, io)
io.to_s.should be_empty
end
it "writes the explanation if sources are not valid and location found" do
io.clear
rules = [ErrorRule.new] of Rule::Base
source = Source.new %(
a = 1
), "source.cr"
runner = Runner.new(rules, [source], formatter).run
runner.explain({file: "source.cr", line: 1, column: 1}, io)
io.to_s.should_not be_empty
end
it "writes nothing if sources are not valid and location is not found" do
io.clear
rules = [ErrorRule.new] of Rule::Base
source = Source.new %(
a = 1
), "source.cr"
runner = Runner.new(rules, [source], formatter).run
runner.explain({file: "source.cr", line: 1, column: 2}, io)
io.to_s.should be_empty
end
end
describe "#success?" do
it "returns true if runner has not been run" do
runner.success?.should be_true

View file

@ -50,6 +50,10 @@ module Ameba
end
struct ErrorRule < Rule::Base
properties do
description : String = "Always adds an error at 1:1"
end
def test(source)
issue_for({1, 1}, "This rule always adds an error")
end

View file

@ -13,7 +13,13 @@ module Ameba::Cli
configure_formatter(config, opts)
configure_rules(config, opts)
exit 1 unless Ameba.run(config).success?
runner = Ameba.run(config)
if location = opts.location_to_explain
runner.explain(location)
else
exit 1 unless runner.success?
end
rescue e
puts "Error: #{e.message}"
exit 255
@ -25,6 +31,7 @@ module Ameba::Cli
property files : Array(String)?
property only : Array(String)?
property except : Array(String)?
property location_to_explain : NamedTuple(file: String, line: Int32, column: Int32)?
property? all = false
property? colors = true
property? without_affected_code = false
@ -37,7 +44,13 @@ module Ameba::Cli
parser.on("-v", "--version", "Print version") { print_version }
parser.on("-h", "--help", "Show this help") { show_help parser }
parser.on("-s", "--silent", "Disable output") { opts.formatter = :silent }
parser.unknown_args { |f| opts.files = f if f.any? }
parser.unknown_args do |f|
if f.size == 1 && f.first =~ /.+:\d+:\d+/
configure_explain_opts(f.first, opts)
else
opts.files = f if f.any?
end
end
parser.on("-c", "--config PATH",
"Specify a configuration file") do |path|
@ -69,6 +82,11 @@ module Ameba::Cli
opts.config = ""
end
parser.on("-e", "--explain PATH:line:column",
"Explain an issue at a specified location") do |loc|
configure_explain_opts(loc, opts)
end
parser.on("--without-affected-code",
"Stop showing affected code while using a default formatter") do
opts.without_affected_code = true
@ -100,6 +118,22 @@ module Ameba::Cli
config.formatter.config[:without_affected_code] = opts.without_affected_code?
end
private def configure_explain_opts(loc, opts)
location_to_explain = parse_explain_location(loc)
opts.location_to_explain = location_to_explain
opts.files = [location_to_explain[:file]]
opts.formatter = :silent
end
private def parse_explain_location(arg)
location = arg.split(":", remove_empty: true).map &.strip
raise ArgumentError.new unless location.size === 3
file, line, column = location
{file: file, line: line.to_i, column: column.to_i}
rescue
raise "location should have PATH:line:column format"
end
private def print_version
puts VERSION
exit 0

View file

@ -1,3 +1,5 @@
require "./util"
# A module that utilizes Ameba's formatters.
module Ameba::Formatter
# A base formatter for all formatters. It uses `output` IO

View file

@ -1,7 +1,11 @@
require "./util"
module Ameba::Formatter
# A formatter that shows a progress of inspection in a terminal using dots.
# It is similar to Crystal's dot formatter for specs.
class DotFormatter < BaseFormatter
include Util
@started_at : Time?
# Reports a message when inspection is started.
@ -87,25 +91,5 @@ module Ameba::Formatter
"#{total} inspected, #{failures} failure#{s}.\n".colorize color
end
private def affected_code(source, location, max_length = 100, placeholder = " ...", prompt = "> ")
line, column = location.line_number, location.column_number
affected_line = source.lines[line - 1]?
return unless affected_line
if affected_line.size > max_length && column < max_length
affected_line = affected_line[0, max_length - placeholder.size - 1] + placeholder
end
stripped = affected_line.lstrip
position = column - (affected_line.size - stripped.size) + prompt.size
String.build do |str|
str << prompt << stripped << "\n"
str << " " * (position - 1)
str << "^".colorize(:yellow)
end
end
end
end

View file

@ -0,0 +1,82 @@
require "./util"
module Ameba::Formatter
# A formatter that shows the detailed explanation of the issue at
# a specific location.
class ExplainFormatter
LINE_BREAK = "\n"
PREFIX = "| ".colorize(:yellow)
include Util
getter output : IO::FileDescriptor | IO::Memory
getter location : Crystal::Location
# Creates a new instance of ExplainFormatter.
# Accepts *output* which indicates the io where the explaination will be wrtitten to.
# Second argument is *location* which indicates the location to explain.
#
# ```
# ExplainFormatter.new output,
# {file: path, line: line_number, column: column_number}
# ```
#
def initialize(@output, loc)
@location = Crystal::Location.new(loc[:file], loc[:line], loc[:column])
end
# Reports the explainations at the *@location*.
def finished(sources)
source = sources.find { |s| s.path == @location.filename }
return unless source
source.issues.each do |issue|
if (location = issue.location) &&
location.line_number == @location.line_number &&
location.column_number == @location.column_number
explain(source, issue)
end
end
end
private def explain(source, issue)
rule = issue.rule
output_title "ISSUE INFO"
output_paragraph [
issue.message.colorize(:red).to_s,
@location.to_s.colorize(:cyan).to_s,
]
if affected_code = affected_code(source, @location)
output_title "AFFECTED CODE"
output_paragraph affected_code
end
if rule.responds_to?(:description)
output_title "RULE INFO"
output_paragraph [rule.name, rule.description]
end
output_title "DETAILED DESCRIPTION"
output_paragraph(rule.class.parsed_doc || "TO BE DONE...")
end
private def output_title(title)
output << PREFIX << title.colorize(:yellow) << LINE_BREAK
output << PREFIX << LINE_BREAK
end
private def output_paragraph(paragraph : String)
output_paragraph(paragraph.split(LINE_BREAK))
end
private def output_paragraph(paragraph : Array(String))
paragraph.each do |line|
output << PREFIX << PREFIX << line << LINE_BREAK
end
output << PREFIX << LINE_BREAK
end
end
end

View file

@ -0,0 +1,23 @@
module Ameba::Formatter
module Util
def affected_code(source, location, max_length = 100, placeholder = " ...", prompt = "> ")
line, column = location.line_number, location.column_number
affected_line = source.lines[line - 1]?
return if affected_line.nil? || affected_line.strip.empty?
if affected_line.size > max_length && column < max_length
affected_line = affected_line[0, max_length - placeholder.size - 1] + placeholder
end
stripped = affected_line.lstrip
position = column - (affected_line.size - stripped.size) + prompt.size
String.build do |str|
str << prompt << stripped << "\n"
str << " " * (position - 1)
str << "^".colorize(:yellow)
end
end
end
end

View file

@ -82,6 +82,21 @@ module Ameba
@formatter.finished @sources
end
# Explains an issue at a specified *location*.
#
# Runner should perform inspection before doing the explain.
# This is necessary to be able to find the issue at a specified location.
#
# ```
# runner = Ameba::Runner.new config
# runner.run
# runner.explain({file: file, line: l, column: c})
# ```
#
def explain(location, output = STDOUT)
Formatter::ExplainFormatter.new(output, location).finished @sources
end
# Indicates whether the last inspection successful or not.
# It returns true if no issues in sources found, false otherwise.
#