From f8d14d42221420efb63e180bc2b29d2365a797af Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Mon, 11 Jun 2018 00:15:12 +0300 Subject: [PATCH] Rename Error to Issue --- .../formatter/disabled_formatter_spec.cr | 9 +-- spec/ameba/formatter/dot_formatter_spec.cr | 15 ++-- .../formatter/flycheck_formatter_spec.cr | 8 +-- spec/ameba/formatter/json_formatter_spec.cr | 24 +++---- spec/ameba/formatter/todo_formatter_spec.cr | 4 +- spec/ameba/inline_comments_spec.cr | 22 +++--- spec/ameba/issue_spec.cr | 50 +++++++++++++ spec/ameba/reportable_spec.cr | 40 +++++++++++ spec/ameba/rule/comparison_to_boolean_spec.cr | 16 ++--- spec/ameba/rule/constant_names_spec.cr | 10 +-- spec/ameba/rule/debugger_statement_spec.cr | 8 +-- spec/ameba/rule/empty_ensure_spec.cr | 8 +-- spec/ameba/rule/empty_expression_spec.cr | 8 +-- spec/ameba/rule/hash_duplicated_key_spec.cr | 12 ++-- spec/ameba/rule/large_numbers_spec.cr | 10 +-- spec/ameba/rule/line_length_spec.cr | 8 +-- spec/ameba/rule/literal_in_condition_spec.cr | 10 +-- .../rule/literal_in_interpolation_spec.cr | 8 +-- spec/ameba/rule/method_names_spec.cr | 10 +-- .../rule/negated_conditions_in_unless_spec.cr | 8 +-- spec/ameba/rule/percent_arrays_spec.cr | 16 ++--- spec/ameba/rule/predicate_name_spec.cr | 8 +-- spec/ameba/rule/rand_zero_spec.cr | 8 +-- spec/ameba/rule/redundant_begin_spec.cr | 8 +-- spec/ameba/rule/shadowed_argument_spec.cr | 8 +-- spec/ameba/rule/shadowed_exception_spec.cr | 10 +-- .../rule/shadowing_local_outer_var_spec.cr | 14 ++-- spec/ameba/rule/syntax_spec.cr | 8 +-- spec/ameba/rule/trailing_blank_lines_spec.cr | 8 +-- spec/ameba/rule/trailing_whitespace_spec.cr | 8 +-- spec/ameba/rule/type_names_spec.cr | 10 +-- spec/ameba/rule/unless_else_spec.cr | 10 +-- .../rule/unneded_disable_directive_spec.cr | 32 +++++---- spec/ameba/rule/unused_argument_spec.cr | 22 +++--- spec/ameba/rule/useless_assign_spec.cr | 72 +++++++++---------- .../rule/useless_condition_in_when_spec.cr | 8 +-- spec/ameba/rule/variable_names_spec.cr | 10 +-- spec/ameba/rule/while_true_spec.cr | 6 +- spec/ameba/runner_spec.cr | 8 +-- spec/ameba/source_spec.cr | 13 ---- spec/spec_helper.cr | 6 +- src/ameba/formatter/disabled_formatter.cr | 2 +- src/ameba/formatter/dot_formatter.cr | 10 +-- src/ameba/formatter/flycheck_formatter.cr | 2 +- src/ameba/formatter/json_formatter.cr | 20 +++--- src/ameba/formatter/todo_formatter.cr | 30 ++++---- src/ameba/issue.cr | 22 ++++++ src/ameba/reportable.cr | 34 +++++++++ src/ameba/rule/base.cr | 10 ++- src/ameba/rule/comparison_to_boolean.cr | 2 +- src/ameba/rule/constant_names.cr | 2 +- src/ameba/rule/debugger_statement.cr | 2 +- src/ameba/rule/empty_ensure.cr | 2 +- src/ameba/rule/empty_expression.cr | 4 +- src/ameba/rule/hash_duplicated_key.cr | 2 +- src/ameba/rule/large_numbers.cr | 2 +- src/ameba/rule/line_length.cr | 2 +- src/ameba/rule/literal_in_condition.cr | 2 +- src/ameba/rule/literal_in_interpolation.cr | 2 +- src/ameba/rule/method_names.cr | 2 +- .../rule/negated_conditions_in_unless.cr | 2 +- src/ameba/rule/percent_arrays.cr | 14 ++-- src/ameba/rule/predicate_name.cr | 2 +- src/ameba/rule/rand_zero.cr | 2 +- src/ameba/rule/redundant_begin.cr | 2 +- src/ameba/rule/shadowed_argument.cr | 2 +- src/ameba/rule/shadowed_exception.cr | 2 +- src/ameba/rule/shadowing_local_outer_var.cr | 2 +- src/ameba/rule/syntax.cr | 2 +- src/ameba/rule/trailing_blank_lines.cr | 2 +- src/ameba/rule/trailing_whitespace.cr | 2 +- src/ameba/rule/type_names.cr | 2 +- src/ameba/rule/unless_else.cr | 2 +- src/ameba/rule/unneeded_disable_directive.cr | 18 ++--- src/ameba/rule/unused_argument.cr | 2 +- src/ameba/rule/useless_assign.cr | 2 +- src/ameba/rule/useless_condition_in_when.cr | 2 +- src/ameba/rule/variable_names.cr | 2 +- src/ameba/rule/while_true.cr | 2 +- src/ameba/runner.cr | 2 +- src/ameba/source.cr | 56 +-------------- 81 files changed, 475 insertions(+), 384 deletions(-) create mode 100644 spec/ameba/issue_spec.cr create mode 100644 spec/ameba/reportable_spec.cr create mode 100644 src/ameba/issue.cr create mode 100644 src/ameba/reportable.cr diff --git a/spec/ameba/formatter/disabled_formatter_spec.cr b/spec/ameba/formatter/disabled_formatter_spec.cr index 052d336c..c44f1a50 100644 --- a/spec/ameba/formatter/disabled_formatter_spec.cr +++ b/spec/ameba/formatter/disabled_formatter_spec.cr @@ -16,8 +16,8 @@ module Ameba::Formatter path = "source.cr" s = Source.new("", path).tap do |source| - source.error(ErrorRule.new, 1, 2, "ErrorRule", :disabled) - source.error(NamedRule.new, 2, 2, "NamedRule", :disabled) + source.add_issue(ErrorRule.new, {1, 2}, message: "ErrorRule", status: :disabled) + source.add_issue(NamedRule.new, location: {2, 2}, message: "NamedRule", status: :disabled) end subject.finished [s] log = output.to_s @@ -30,8 +30,9 @@ module Ameba::Formatter it "does not write not-disabled rules" do s = Source.new("", "source.cr").tap do |source| - source.error(ErrorRule.new, 1, 2, "ErrorRule") - source.error(NamedRule.new, 2, 2, "NamedRule", :disabled) + source.add_issue(ErrorRule.new, {1, 2}, "ErrorRule") + source.add_issue(NamedRule.new, location: {2, 2}, + message: "NamedRule", status: :disabled) end subject.finished [s] output.to_s.should_not contain ErrorRule.name diff --git a/spec/ameba/formatter/dot_formatter_spec.cr b/spec/ameba/formatter/dot_formatter_spec.cr index eac49dcf..b49700fb 100644 --- a/spec/ameba/formatter/dot_formatter_spec.cr +++ b/spec/ameba/formatter/dot_formatter_spec.cr @@ -20,7 +20,7 @@ module Ameba::Formatter it "writes invalid source" do s = Source.new "" - s.error DummyRule.new, nil, "message" + s.add_issue DummyRule.new, Crystal::Nop.new, "message" subject.source_finished s output.to_s.should contain "F" end @@ -37,11 +37,11 @@ module Ameba::Formatter output.to_s.should contain "Finished in" end - context "when errors found" do - it "writes each error" do + context "when issues found" do + it "writes each issue" do s = Source.new("").tap do |source| - source.error(DummyRule.new, 1, 1, "DummyRuleError") - source.error(NamedRule.new, 1, 2, "NamedRuleError") + source.add_issue(DummyRule.new, {1, 1}, "DummyRuleError") + source.add_issue(NamedRule.new, {1, 2}, "NamedRuleError") end subject.finished [s] log = output.to_s @@ -50,9 +50,10 @@ module Ameba::Formatter log.should contain "NamedRuleError" end - it "does not write disabled errors" do + it "does not write disabled issues" do s = Source.new "" - s.error(DummyRule.new, 1, 1, "DummyRuleError", :disabled) + s.add_issue(DummyRule.new, location: {1, 1}, + message: "DummyRuleError", status: :disabled) subject.finished [s] output.to_s.should contain "1 inspected, 0 failures." end diff --git a/spec/ameba/formatter/flycheck_formatter_spec.cr b/spec/ameba/formatter/flycheck_formatter_spec.cr index 2b4785ce..7d7ec44c 100644 --- a/spec/ameba/formatter/flycheck_formatter_spec.cr +++ b/spec/ameba/formatter/flycheck_formatter_spec.cr @@ -16,9 +16,9 @@ module Ameba::Formatter end context "when problems found" do - it "reports an error" do + it "reports an issue" do s = Source.new "a = 1", "source.cr" - s.error DummyRule.new, 1, 2, "message" + s.add_issue DummyRule.new, {1, 2}, "message" subject = flycheck subject.source_finished s subject.output.to_s.should eq( @@ -28,7 +28,7 @@ module Ameba::Formatter it "properly reports multi-line message" do s = Source.new "a = 1", "source.cr" - s.error DummyRule.new, 1, 2, "multi\nline" + s.add_issue DummyRule.new, {1, 2}, "multi\nline" subject = flycheck subject.source_finished s subject.output.to_s.should eq( @@ -38,7 +38,7 @@ module Ameba::Formatter it "reports nothing if location was not set" do s = Source.new "a = 1", "source.cr" - s.error DummyRule.new, nil, "message" + s.add_issue DummyRule.new, Crystal::Nop.new, "message" subject = flycheck subject.source_finished s subject.output.to_s.should eq "" diff --git a/spec/ameba/formatter/json_formatter_spec.cr b/spec/ameba/formatter/json_formatter_spec.cr index e21d56ad..287feb42 100644 --- a/spec/ameba/formatter/json_formatter_spec.cr +++ b/spec/ameba/formatter/json_formatter_spec.cr @@ -31,26 +31,26 @@ module Ameba it "shows rule name" do s = Source.new "" - s.error DummyRule.new, 1, 2, "message1" + s.add_issue DummyRule.new, {1, 2}, "message1" result = get_result [s] - result["sources"][0]["errors"][0]["rule_name"].should eq DummyRule.name + result["sources"][0]["issues"][0]["rule_name"].should eq DummyRule.name end it "shows a message" do s = Source.new "" - s.error DummyRule.new, 1, 2, "message" + s.add_issue DummyRule.new, {1, 2}, "message" result = get_result [s] - result["sources"][0]["errors"][0]["message"].should eq "message" + result["sources"][0]["issues"][0]["message"].should eq "message" end - it "shows error location" do + it "shows issue location" do s = Source.new "" - s.error DummyRule.new, 1, 2, "message" + s.add_issue DummyRule.new, {1, 2}, "message" result = get_result [s] - location = result["sources"][0]["errors"][0]["location"] + location = result["sources"][0]["issues"][0]["location"] location["line"].should eq 1 location["column"].should eq 2 end @@ -62,16 +62,16 @@ module Ameba result["summary"]["target_sources_count"].should eq 2 end - it "shows errors count" do + it "shows issues count" do s1 = Source.new "" - s1.error DummyRule.new, 1, 2, "message1" - s1.error DummyRule.new, 1, 2, "message2" + s1.add_issue DummyRule.new, {1, 2}, "message1" + s1.add_issue DummyRule.new, {1, 2}, "message2" s2 = Source.new "" - s2.error DummyRule.new, 1, 2, "message3" + s2.add_issue DummyRule.new, {1, 2}, "message3" result = get_result [s1, s2] - result["summary"]["errors_count"].should eq 3 + result["summary"]["issues_count"].should eq 3 end end end diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index 631272f9..0ed10575 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -6,7 +6,7 @@ module Ameba formatter = Formatter::TODOFormatter.new IO::Memory.new, file s = Source.new "a = 1", "source.cr" - s.error DummyRule.new, 1, 2, "message" + s.add_issue DummyRule.new, {1, 2}, "message" formatter.finished [s] file.to_s @@ -57,7 +57,7 @@ module Ameba formatter = Formatter::TODOFormatter.new IO::Memory.new, file s = Source.new "def invalid_syntax" - s.error Rule::Syntax.new, 1, 2, "message" + s.add_issue Rule::Syntax.new, {1, 2}, "message" formatter.finished [s] content = file.to_s diff --git a/spec/ameba/inline_comments_spec.cr b/spec/ameba/inline_comments_spec.cr index e0bd843e..95971c75 100644 --- a/spec/ameba/inline_comments_spec.cr +++ b/spec/ameba/inline_comments_spec.cr @@ -7,7 +7,7 @@ module Ameba # ameba:disable #{NamedRule.name} Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "Error!") + s.add_issue(NamedRule.new, location: {3, 12}, message: "Error!") s.should be_valid end @@ -15,7 +15,7 @@ module Ameba s = Source.new %Q( Time.epoch(1483859302) # ameba:disable #{NamedRule.name} ) - s.error(NamedRule.new, 2, 12, "Error!") + s.add_issue(NamedRule.new, location: {2, 12}, message: "Error!") s.should be_valid end @@ -24,7 +24,7 @@ module Ameba # ameba:disable WrongName Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "Error!") + s.add_issue(NamedRule.new, location: {3, 12}, message: "Error!") s.should_not be_valid end @@ -33,7 +33,7 @@ module Ameba # ameba:disable SomeRule LargeNumbers #{NamedRule.name} SomeOtherRule Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should be_valid end @@ -42,7 +42,7 @@ module Ameba # ameba:disable SomeRule, LargeNumbers, #{NamedRule.name}, SomeOtherRule Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should be_valid end @@ -51,7 +51,7 @@ module Ameba # ameba:disable SomeRule, SomeOtherRule LargeNumbers Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should_not be_valid end @@ -61,7 +61,7 @@ module Ameba # Time.epoch(1483859302) ) - s.error(NamedRule.new, 4, 12, "") + s.add_issue(NamedRule.new, location: {4, 12}, message: "") s.should_not be_valid end @@ -71,7 +71,7 @@ module Ameba Time.epoch(1483859302) end ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should_not be_valid end @@ -80,7 +80,7 @@ module Ameba "ameba:disable #{NamedRule.name}" Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should_not be_valid end @@ -89,7 +89,7 @@ module Ameba # # ameba:disable #{NamedRule.name} Time.epoch(1483859302) ) - s.error(NamedRule.new, 3, 12, "") + s.add_issue(NamedRule.new, location: {3, 12}, message: "") s.should_not be_valid end @@ -97,7 +97,7 @@ module Ameba s = Source.new %Q( a = 1 # Disable it: # ameba:disable #{NamedRule.name} ) - s.error(NamedRule.new, 2, 12, "") + s.add_issue(NamedRule.new, location: {2, 12}, message: "") s.should_not be_valid end end diff --git a/spec/ameba/issue_spec.cr b/spec/ameba/issue_spec.cr new file mode 100644 index 00000000..652f5247 --- /dev/null +++ b/spec/ameba/issue_spec.cr @@ -0,0 +1,50 @@ +require "../spec_helper" + +module Ameba + describe Issue do + it "accepts rule and message" do + issue = Issue.new rule: DummyRule.new, + location: nil, + end_location: nil, + message: "Blah", + status: nil + + issue.rule.should_not be_nil + issue.message.should eq "Blah" + end + + it "accepts location" do + location = Crystal::Location.new("path", 3, 2) + issue = Issue.new rule: DummyRule.new, + location: location, + end_location: nil, + message: "Blah", + status: nil + + issue.location.to_s.should eq location.to_s + issue.end_location.should eq nil + end + + it "accepts end_location" do + location = Crystal::Location.new("path", 3, 2) + issue = Issue.new rule: DummyRule.new, + location: nil, + end_location: location, + message: "Blah", + status: nil + + issue.location.should eq nil + issue.end_location.to_s.should eq location.to_s + end + + it "accepts status" do + issue = Issue.new rule: DummyRule.new, + location: nil, + end_location: nil, + message: "", + status: :enabled + + issue.status.should eq :enabled + end + end +end diff --git a/spec/ameba/reportable_spec.cr b/spec/ameba/reportable_spec.cr new file mode 100644 index 00000000..8d85a55a --- /dev/null +++ b/spec/ameba/reportable_spec.cr @@ -0,0 +1,40 @@ +require "../spec_helper" + +module Ameba + describe Reportable do + describe "#add_issue" do + it "adds a new issue for node" do + s = Source.new "", "source.cr" + s.add_issue(DummyRule.new, Crystal::Nop.new, "Error!") + + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "" + issue.message.should eq "Error!" + end + + it "adds a new issue by line and column number" do + s = Source.new "", "source.cr" + s.add_issue(DummyRule.new, {23, 2}, "Error!") + + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:23:2" + issue.message.should eq "Error!" + end + end + + describe "#valid?" do + it "returns true if no issues added" do + s = Source.new "", "source.cr" + s.should be_valid + end + + it "returns false if there are issues added" do + s = Source.new "", "source.cr" + s.add_issue DummyRule.new, {22, 2}, "ERROR!" + s.should_not be_valid + end + end + end +end diff --git a/spec/ameba/rule/comparison_to_boolean_spec.cr b/spec/ameba/rule/comparison_to_boolean_spec.cr index c73a6ff7..9c14c360 100644 --- a/spec/ameba/rule/comparison_to_boolean_spec.cr +++ b/spec/ameba/rule/comparison_to_boolean_spec.cr @@ -66,10 +66,10 @@ module Ameba::Rule source = Source.new "a != true", "source.cr" subject.catch(source) - error = source.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "Comparison to a boolean is pointless" + issue = source.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "Comparison to a boolean is pointless" end end @@ -103,10 +103,10 @@ module Ameba::Rule source = Source.new "true != a", "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "Comparison to a boolean is pointless" + issue = source.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "Comparison to a boolean is pointless" end end end diff --git a/spec/ameba/rule/constant_names_spec.cr b/spec/ameba/rule/constant_names_spec.cr index 778d7b00..3a0529f6 100644 --- a/spec/ameba/rule/constant_names_spec.cr +++ b/spec/ameba/rule/constant_names_spec.cr @@ -7,7 +7,7 @@ module Ameba it "reports constant name #{expected}" do s = Source.new code Rule::ConstantNames.new.catch(s).should_not be_valid - s.errors.first.message.should contain expected + s.issues.first.message.should contain expected end end @@ -40,10 +40,10 @@ module Ameba Const = 1 ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Constant name should be screaming-cased: CONST, not Const" ) end diff --git a/spec/ameba/rule/debugger_statement_spec.cr b/spec/ameba/rule/debugger_statement_spec.cr index bdd19753..358a3819 100644 --- a/spec/ameba/rule/debugger_statement_spec.cr +++ b/spec/ameba/rule/debugger_statement_spec.cr @@ -35,10 +35,10 @@ module Ameba::Rule s = Source.new "debugger", "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "Possible forgotten debugger statement detected" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "Possible forgotten debugger statement detected" end end end diff --git a/spec/ameba/rule/empty_ensure_spec.cr b/spec/ameba/rule/empty_ensure_spec.cr index 0d4c4e4b..a63eedca 100644 --- a/spec/ameba/rule/empty_ensure_spec.cr +++ b/spec/ameba/rule/empty_ensure_spec.cr @@ -58,11 +58,11 @@ module Ameba::Rule end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first + issue = s.issues.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:11" - error.message.should eq "Empty `ensure` block detected" + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:11" + issue.message.should eq "Empty `ensure` block detected" end end end diff --git a/spec/ameba/rule/empty_expression_spec.cr b/spec/ameba/rule/empty_expression_spec.cr index 05b1e60a..a1aa41d8 100644 --- a/spec/ameba/rule/empty_expression_spec.cr +++ b/spec/ameba/rule/empty_expression_spec.cr @@ -101,10 +101,10 @@ module Ameba end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:12" - error.message.should eq "Avoid empty expressions" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:12" + issue.message.should eq "Avoid empty expressions" end end end diff --git a/spec/ameba/rule/hash_duplicated_key_spec.cr b/spec/ameba/rule/hash_duplicated_key_spec.cr index 4f03e1bd..5e078513 100644 --- a/spec/ameba/rule/hash_duplicated_key_spec.cr +++ b/spec/ameba/rule/hash_duplicated_key_spec.cr @@ -32,10 +32,10 @@ module Ameba::Rule h = {"a" => 1, "a" => 2} ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:13" - error.message.should eq %(Duplicated keys in hash literal: "a") + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:13" + issue.message.should eq %(Duplicated keys in hash literal: "a") end it "reports multiple duplicated keys" do @@ -43,8 +43,8 @@ module Ameba::Rule h = {"key1" => 1, "key1" => 2, "key2" => 3, "key2" => 4} ) subject.catch(s).should_not be_valid - error = s.errors.first - error.message.should eq %(Duplicated keys in hash literal: "key1", "key2") + issue = s.issues.first + issue.message.should eq %(Duplicated keys in hash literal: "key1", "key2") end end end diff --git a/spec/ameba/rule/large_numbers_spec.cr b/spec/ameba/rule/large_numbers_spec.cr index 89c619ad..35d979b0 100644 --- a/spec/ameba/rule/large_numbers_spec.cr +++ b/spec/ameba/rule/large_numbers_spec.cr @@ -7,7 +7,7 @@ module Ameba it "transforms large number #{number}" do s = Source.new number Rule::LargeNumbers.new.catch(s).should_not be_valid - s.errors.first.message.should contain expected + s.issues.first.message.should contain expected end end @@ -115,10 +115,10 @@ module Ameba 1200000 ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:10" - error.message.should match /1_200_000/ + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:10" + issue.message.should match /1_200_000/ end context "properties" do diff --git a/spec/ameba/rule/line_length_spec.cr b/spec/ameba/rule/line_length_spec.cr index 4c6ae563..2b75efef 100644 --- a/spec/ameba/rule/line_length_spec.cr +++ b/spec/ameba/rule/line_length_spec.cr @@ -24,10 +24,10 @@ module Ameba::Rule source = Source.new long_line, "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.rule.should eq subject - error.location.to_s.should eq "source.cr:1:81" - error.message.should eq "Line too long" + issue = source.issues.first + issue.rule.should eq subject + issue.location.to_s.should eq "source.cr:1:81" + issue.message.should eq "Line too long" end context "properties" do diff --git a/spec/ameba/rule/literal_in_condition_spec.cr b/spec/ameba/rule/literal_in_condition_spec.cr index 8c209e29..839aa9af 100644 --- a/spec/ameba/rule/literal_in_condition_spec.cr +++ b/spec/ameba/rule/literal_in_condition_spec.cr @@ -62,11 +62,11 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq "Literal value found in conditional" + s.issues.size.should eq 1 + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq "Literal value found in conditional" end end end diff --git a/spec/ameba/rule/literal_in_interpolation_spec.cr b/spec/ameba/rule/literal_in_interpolation_spec.cr index afc7a19b..11d388e8 100644 --- a/spec/ameba/rule/literal_in_interpolation_spec.cr +++ b/spec/ameba/rule/literal_in_interpolation_spec.cr @@ -32,10 +32,10 @@ module Ameba::Rule s = Source.new %q("#{4}"), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "Literal value found in interpolation" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "Literal value found in interpolation" end end end diff --git a/spec/ameba/rule/method_names_spec.cr b/spec/ameba/rule/method_names_spec.cr index 030a21bc..447eeb75 100644 --- a/spec/ameba/rule/method_names_spec.cr +++ b/spec/ameba/rule/method_names_spec.cr @@ -7,7 +7,7 @@ module Ameba it "reports method name #{expected}" do s = Source.new code Rule::MethodNames.new.catch(s).should_not be_valid - s.errors.first.message.should contain expected + s.issues.first.message.should contain expected end end @@ -44,10 +44,10 @@ module Ameba end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Method name should be underscore-cased: bad_name, not bad_Name" ) end diff --git a/spec/ameba/rule/negated_conditions_in_unless_spec.cr b/spec/ameba/rule/negated_conditions_in_unless_spec.cr index 39495797..fb8841e4 100644 --- a/spec/ameba/rule/negated_conditions_in_unless_spec.cr +++ b/spec/ameba/rule/negated_conditions_in_unless_spec.cr @@ -59,10 +59,10 @@ module Ameba::Rule s = Source.new ":nok unless !s.empty?", "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "Avoid negated conditions in unless blocks" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "Avoid negated conditions in unless blocks" end end end diff --git a/spec/ameba/rule/percent_arrays_spec.cr b/spec/ameba/rule/percent_arrays_spec.cr index 9b3ffc97..d9247115 100644 --- a/spec/ameba/rule/percent_arrays_spec.cr +++ b/spec/ameba/rule/percent_arrays_spec.cr @@ -44,10 +44,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Symbols `,:` may be unwanted in %i array literals" ) end @@ -58,10 +58,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Symbols `,\"` may be unwanted in %w array literals" ) end diff --git a/spec/ameba/rule/predicate_name_spec.cr b/spec/ameba/rule/predicate_name_spec.cr index 169c30dd..150729d0 100644 --- a/spec/ameba/rule/predicate_name_spec.cr +++ b/spec/ameba/rule/predicate_name_spec.cr @@ -38,10 +38,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:11" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:11" + issue.message.should eq( "Favour method name 'picture?' over 'has_picture?'") end diff --git a/spec/ameba/rule/rand_zero_spec.cr b/spec/ameba/rule/rand_zero_spec.cr index cb886cb4..c3ee4c40 100644 --- a/spec/ameba/rule/rand_zero_spec.cr +++ b/spec/ameba/rule/rand_zero_spec.cr @@ -26,11 +26,11 @@ module Ameba::Rule it "reports rule, location and a message" do s = Source.new "rand(1)", "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first + issue = s.issues.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:1" - error.message.should eq "rand(1) always returns 0" + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.message.should eq "rand(1) always returns 0" end end end diff --git a/spec/ameba/rule/redundant_begin_spec.cr b/spec/ameba/rule/redundant_begin_spec.cr index 7bdad9a9..4f383777 100644 --- a/spec/ameba/rule/redundant_begin_spec.cr +++ b/spec/ameba/rule/redundant_begin_spec.cr @@ -204,10 +204,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq "Redundant `begin` block detected" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq "Redundant `begin` block detected" end end end diff --git a/spec/ameba/rule/shadowed_argument_spec.cr b/spec/ameba/rule/shadowed_argument_spec.cr index 5e88dc23..121a7eab 100644 --- a/spec/ameba/rule/shadowed_argument_spec.cr +++ b/spec/ameba/rule/shadowed_argument_spec.cr @@ -156,10 +156,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:11" - error.message.should eq "Argument `bar` is assigned before it is used" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:11" + issue.message.should eq "Argument `bar` is assigned before it is used" end end end diff --git a/spec/ameba/rule/shadowed_exception_spec.cr b/spec/ameba/rule/shadowed_exception_spec.cr index 366743b9..42b23404 100644 --- a/spec/ameba/rule/shadowed_exception_spec.cr +++ b/spec/ameba/rule/shadowed_exception_spec.cr @@ -3,7 +3,7 @@ require "../../spec_helper" private def check_shadowed(source, exceptions) s = Ameba::Source.new source Ameba::Rule::ShadowedException.new.catch(s).should_not be_valid - s.errors.first.message.should contain exceptions.join(", ") + s.issues.first.message.should contain exceptions.join(", ") end module Ameba::Rule @@ -163,11 +163,11 @@ module Ameba::Rule end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first + issue = s.issues.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:11" - error.message.should eq( + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:11" + issue.message.should eq( "Exception handler has shadowed exceptions: IndexError" ) end diff --git a/spec/ameba/rule/shadowing_local_outer_var_spec.cr b/spec/ameba/rule/shadowing_local_outer_var_spec.cr index 75e19f35..5eb83396 100644 --- a/spec/ameba/rule/shadowing_local_outer_var_spec.cr +++ b/spec/ameba/rule/shadowing_local_outer_var_spec.cr @@ -68,7 +68,7 @@ module Ameba::Rule ) subject.catch(source).should_not be_valid - source.errors.size.should eq 2 + source.issues.size.should eq 2 end it "reports if a splat block argument shadows local var" do @@ -89,7 +89,7 @@ module Ameba::Rule end ) subject.catch(source).should_not be_valid - source.errors.first.message.should eq "Shadowing outer local variable `block`" + source.issues.first.message.should eq "Shadowing outer local variable `block`" end it "reports if there are multiple args and one shadows local var" do @@ -100,7 +100,7 @@ module Ameba::Rule end ) subject.catch(source).should_not be_valid - source.errors.first.message.should eq "Shadowing outer local variable `foo`" + source.issues.first.message.should eq "Shadowing outer local variable `foo`" end it "doesn't report if an outer var is reassigned in a block" do @@ -131,10 +131,10 @@ module Ameba::Rule ), "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:20" - error.message.should eq "Shadowing outer local variable `foo`" + issue = source.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:20" + issue.message.should eq "Shadowing outer local variable `foo`" end end end diff --git a/spec/ameba/rule/syntax_spec.cr b/spec/ameba/rule/syntax_spec.cr index 79b2d8a9..f234e6f8 100644 --- a/spec/ameba/rule/syntax_spec.cr +++ b/spec/ameba/rule/syntax_spec.cr @@ -27,11 +27,11 @@ module Ameba::Rule it "reports rule, location and message" do s = Source.new "def hello end", "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first + issue = s.issues.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:1:11" - error.message.should eq "unexpected token: end (expected ';' or newline)" + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:11" + issue.message.should eq "unexpected token: end (expected ';' or newline)" end end end diff --git a/spec/ameba/rule/trailing_blank_lines_spec.cr b/spec/ameba/rule/trailing_blank_lines_spec.cr index b24849c3..ca3cf13c 100644 --- a/spec/ameba/rule/trailing_blank_lines_spec.cr +++ b/spec/ameba/rule/trailing_blank_lines_spec.cr @@ -28,10 +28,10 @@ module Ameba::Rule source = Source.new "a = 1\n\n ", "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:1" - error.message.should eq "Blank lines detected at the end of the file" + issue = source.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:1" + issue.message.should eq "Blank lines detected at the end of the file" end end end diff --git a/spec/ameba/rule/trailing_whitespace_spec.cr b/spec/ameba/rule/trailing_whitespace_spec.cr index a736cedb..260a65d7 100644 --- a/spec/ameba/rule/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/trailing_whitespace_spec.cr @@ -18,10 +18,10 @@ module Ameba::Rule source = Source.new "a = 1\n b = 2 ", "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:7" - error.message.should eq "Trailing whitespace detected" + issue = source.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:7" + issue.message.should eq "Trailing whitespace detected" end end end diff --git a/spec/ameba/rule/type_names_spec.cr b/spec/ameba/rule/type_names_spec.cr index 3dc18312..e7b55510 100644 --- a/spec/ameba/rule/type_names_spec.cr +++ b/spec/ameba/rule/type_names_spec.cr @@ -7,7 +7,7 @@ module Ameba it "reports type name #{expected}" do s = Source.new code Rule::TypeNames.new.catch(s).should_not be_valid - s.errors.first.message.should contain expected + s.issues.first.message.should contain expected end end @@ -49,10 +49,10 @@ module Ameba end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Type name should be camelcased: MyClass, but it was My_class" ) end diff --git a/spec/ameba/rule/unless_else_spec.cr b/spec/ameba/rule/unless_else_spec.cr index 49898642..98466a69 100644 --- a/spec/ameba/rule/unless_else_spec.cr +++ b/spec/ameba/rule/unless_else_spec.cr @@ -34,11 +34,11 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.should_not be_nil - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq "Favour if over unless with else" + issue = s.issues.first + issue.should_not be_nil + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq "Favour if over unless with else" end end end diff --git a/spec/ameba/rule/unneded_disable_directive_spec.cr b/spec/ameba/rule/unneded_disable_directive_spec.cr index 0c50784c..2efbbcad 100644 --- a/spec/ameba/rule/unneded_disable_directive_spec.cr +++ b/spec/ameba/rule/unneded_disable_directive_spec.cr @@ -23,7 +23,8 @@ module Ameba::Rule # ameba:disable #{NamedRule.name} a = 1 ) - s.error NamedRule.new, 3, 9, "Useless assignment", :disabled + s.add_issue NamedRule.new, location: {3, 9}, + message: "Useless assignment", status: :disabled subject.catch(s).should be_valid end @@ -31,7 +32,8 @@ module Ameba::Rule s = Source.new %Q( a = 1 # ameba:disable #{NamedRule.name} ) - s.error NamedRule.new, 2, 1, "Alarm!", :disabled + s.add_issue NamedRule.new, location: {2, 1}, + message: "Alarm!", status: :disabled subject.catch(s).should be_valid end @@ -40,7 +42,8 @@ module Ameba::Rule # # ameba:disable #{NamedRule.name} a = 1 ) - s.error NamedRule.new, 3, 1, "Alarm!", :disabled + s.add_issue NamedRule.new, location: {3, 1}, + message: "Alarm!", status: :disabled subject.catch(s).should be_valid end @@ -50,7 +53,7 @@ module Ameba::Rule a = 1 ) subject.catch(s).should_not be_valid - s.errors.first.message.should eq( + s.issues.first.message.should eq( "Unnecessary disabling of #{NamedRule.name}" ) end @@ -58,7 +61,7 @@ module Ameba::Rule it "fails if there is inline unneeded directive" do s = Source.new %Q(a = 1 # ameba:disable #{NamedRule.name}) subject.catch(s).should_not be_valid - s.errors.first.message.should eq( + s.issues.first.message.should eq( "Unnecessary disabling of #{NamedRule.name}" ) end @@ -69,9 +72,9 @@ module Ameba::Rule a = 1 # ameba:disable Rule3 ), "source.cr" subject.catch(s).should_not be_valid - s.errors.size.should eq 2 - s.errors.first.message.should contain "Rule1, Rule2" - s.errors.last.message.should contain "Rule3" + s.issues.size.should eq 2 + s.issues.first.message.should contain "Rule1, Rule2" + s.issues.last.message.should contain "Rule3" end it "fails if there is disabled UnneededDisableDirective" do @@ -79,20 +82,21 @@ module Ameba::Rule # ameba:disable #{UnneededDisableDirective.rule_name} a = 1 ), "source.cr" - s.error UnneededDisableDirective.new, 3, 1, "Alarm!", :disabled + s.add_issue UnneededDisableDirective.new, location: {3, 1}, + message: "Alarm!", status: :disabled subject.catch(s).should_not be_valid end - it "reports error, location and message" do + it "reports issue, location and message" do s = Source.new %Q( # ameba:disable Rule1, Rule2 a = 1 ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq "Unnecessary disabling of Rule1, Rule2" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq "Unnecessary disabling of Rule1, Rule2" end end end diff --git a/spec/ameba/rule/unused_argument_spec.cr b/spec/ameba/rule/unused_argument_spec.cr index 405eb480..944e6ab7 100644 --- a/spec/ameba/rule/unused_argument_spec.cr +++ b/spec/ameba/rule/unused_argument_spec.cr @@ -27,7 +27,7 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.first.message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ + s.issues.first.message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ "as an argument name to indicate that it won't be used." end @@ -38,7 +38,7 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.first.message.should eq "Unused argument `i`. If it's necessary, use `_` " \ + s.issues.first.message.should eq "Unused argument `i`. If it's necessary, use `_` " \ "as an argument name to indicate that it won't be used." end @@ -49,7 +49,7 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ + s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ "as an argument name to indicate that it won't be used." end @@ -60,11 +60,11 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors[0].message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ + s.issues[0].message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ "as an argument name to indicate that it won't be used." - s.errors[1].message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ + s.issues[1].message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ "as an argument name to indicate that it won't be used." - s.errors[2].message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ + s.issues[2].message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ "as an argument name to indicate that it won't be used." end @@ -164,7 +164,7 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ + s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ "as an argument name to indicate that it won't be used." end @@ -174,11 +174,11 @@ module Ameba::Rule end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ + issue = s.issues.first + issue.rule.should_not be_nil + issue.message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ "as an argument name to indicate that it won't be used." - error.location.to_s.should eq "source.cr:2:22" + issue.location.to_s.should eq "source.cr:2:22" end end diff --git a/spec/ameba/rule/useless_assign_spec.cr b/spec/ameba/rule/useless_assign_spec.cr index e86493d3..5c9d1f79 100644 --- a/spec/ameba/rule/useless_assign_spec.cr +++ b/spec/ameba/rule/useless_assign_spec.cr @@ -75,10 +75,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:3:11" - error.message.should eq "Useless assignment to variable `a`" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:3:11" + issue.message.should eq "Useless assignment to variable `a`" end it "does not report useless assignment of instance var" do @@ -137,7 +137,7 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.first.location.to_s.should eq ":3:11" + s.issues.first.location.to_s.should eq ":3:11" end it "reports if variable reassigned and not used" do @@ -314,10 +314,10 @@ module Ameba::Rule ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.last - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:5:13" - error.message.should eq "Useless assignment to variable `a`" + issue = s.issues.last + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:5:13" + issue.message.should eq "Useless assignment to variable `a`" end end @@ -340,9 +340,9 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - error = s.errors.first - error.location.to_s.should eq ":3:16" - error.message.should eq "Useless assignment to variable `b`" + issue = s.issues.first + issue.location.to_s.should eq ":3:16" + issue.message.should eq "Useless assignment to variable `b`" end it "reports if both assigns are reassigned and useless" do @@ -363,13 +363,13 @@ module Ameba::Rule ) subject.catch(s).should_not be_valid - error = s.errors.first - error.location.to_s.should eq ":3:13" - error.message.should eq "Useless assignment to variable `a`" + issue = s.issues.first + issue.location.to_s.should eq ":3:13" + issue.message.should eq "Useless assignment to variable `a`" - error = s.errors.last - error.location.to_s.should eq ":3:16" - error.message.should eq "Useless assignment to variable `b`" + issue = s.issues.last + issue.location.to_s.should eq ":3:16" + issue.message.should eq "Useless assignment to variable `b`" end end @@ -380,9 +380,9 @@ module Ameba::Rule a = 2 ) subject.catch(s).should_not be_valid - s.errors.size.should eq 2 - s.errors.first.location.to_s.should eq ":2:11" - s.errors.last.location.to_s.should eq ":3:11" + s.issues.size.should eq 2 + s.issues.first.location.to_s.should eq ":2:11" + s.issues.last.location.to_s.should eq ":3:11" end it "doesn't report if assignments are referenced" do @@ -476,8 +476,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":5:17" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":5:17" end it "does not report of assignments are referenced in all branches" do @@ -545,8 +545,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":5:17" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":5:17" end end @@ -578,9 +578,9 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 2 - s.errors.first.location.to_s.should eq ":5:17" - s.errors.last.location.to_s.should eq ":7:17" + s.issues.size.should eq 2 + s.issues.first.location.to_s.should eq ":5:17" + s.issues.last.location.to_s.should eq ":7:17" end it "doesn't report if assignment is referenced in cond" do @@ -615,8 +615,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":3:27" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":3:27" end end @@ -642,8 +642,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":4:17" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":4:17" end it "does not report if assignment is referenced in a loop" do @@ -737,8 +737,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":4:17" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":4:17" end end @@ -785,8 +785,8 @@ module Ameba::Rule end ) subject.catch(s).should_not be_valid - s.errors.size.should eq 1 - s.errors.first.location.to_s.should eq ":4:15" + s.issues.size.should eq 1 + s.issues.first.location.to_s.should eq ":4:15" end end end diff --git a/spec/ameba/rule/useless_condition_in_when_spec.cr b/spec/ameba/rule/useless_condition_in_when_spec.cr index d504793c..4c800934 100644 --- a/spec/ameba/rule/useless_condition_in_when_spec.cr +++ b/spec/ameba/rule/useless_condition_in_when_spec.cr @@ -36,10 +36,10 @@ module Ameba::Rule end ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:6:23" - error.message.should eq "Useless condition in when detected" + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:6:23" + issue.message.should eq "Useless condition in when detected" end end end diff --git a/spec/ameba/rule/variable_names_spec.cr b/spec/ameba/rule/variable_names_spec.cr index 193816e8..58b3fe62 100644 --- a/spec/ameba/rule/variable_names_spec.cr +++ b/spec/ameba/rule/variable_names_spec.cr @@ -7,7 +7,7 @@ module Ameba it "reports method name #{expected}" do s = Source.new code Rule::VariableNames.new.catch(s).should_not be_valid - s.errors.first.message.should contain expected + s.issues.first.message.should contain expected end end @@ -50,10 +50,10 @@ module Ameba badName = "Yeah" ), "source.cr" subject.catch(s).should_not be_valid - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:2:9" - error.message.should eq( + issue = s.issues.first + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:2:9" + issue.message.should eq( "Var name should be underscore-cased: bad_name, not badName" ) end diff --git a/spec/ameba/rule/while_true_spec.cr b/spec/ameba/rule/while_true_spec.cr index b23b2503..f3a32aaa 100644 --- a/spec/ameba/rule/while_true_spec.cr +++ b/spec/ameba/rule/while_true_spec.cr @@ -34,9 +34,9 @@ module Ameba::Rule source = Source.new invalid_source, "source.cr" subject.catch(source).should_not be_valid - error = source.errors.first - error.location.to_s.should eq "source.cr:2:1" - error.message.should eq "While statement using true literal as condition" + issue = source.issues.first + issue.location.to_s.should eq "source.cr:2:1" + issue.message.should eq "While statement using true literal as condition" end end end diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 880f00a3..1f4bf0fa 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -59,7 +59,7 @@ module Ameba Runner.new(rules, [source], formatter).run source.should_not be_valid - source.errors.first.rule.name.should eq Rule::Syntax.rule_name + source.issues.first.rule.name.should eq Rule::Syntax.rule_name end it "does not run other rules" do @@ -72,12 +72,12 @@ module Ameba Runner.new(rules, [source], formatter).run source.should_not be_valid - source.errors.size.should eq 1 + source.issues.size.should eq 1 end end context "unneeded disables" do - it "reports an error if such disable exists" do + it "reports an issue if such disable exists" do rules = [Rule::UnneededDisableDirective.new] of Rule::Base source = Source.new %( a = 1 # ameba:disable LineLength @@ -85,7 +85,7 @@ module Ameba Runner.new(rules, [source], formatter).run source.should_not be_valid - source.errors.first.rule.name.should eq Rule::UnneededDisableDirective.rule_name + source.issues.first.rule.name.should eq Rule::UnneededDisableDirective.rule_name end end end diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index 830c3f5f..cd8af875 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -11,19 +11,6 @@ module Ameba end end - describe "#error" do - it "adds and error" do - s = Source.new "", "source.cr" - s.error(DummyRule.new, 23, 2, "Error!") - s.should_not be_valid - - error = s.errors.first - error.rule.should_not be_nil - error.location.to_s.should eq "source.cr:23:2" - error.message.should eq "Error!" - end - end - describe "#fullpath" do it "returns a relative path of the source" do s = Source.new "", "./source_spec.cr" diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index b91e5e5d..a1e96dd7 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -26,7 +26,7 @@ module Ameba struct ErrorRule < Rule::Base def test(source) - source.error self, 1, 1, "This rule always adds an error" + issue_for({1, 1}, "This rule always adds an error") end end @@ -71,8 +71,8 @@ module Ameba def failure_message(source) String.build do |str| - str << "Source expected to be valid, but there are errors:\n\n" - source.errors.reject(&.disabled?).each do |e| + str << "Source expected to be valid, but there are issues: \n\n" + source.issues.reject(&.disabled?).each do |e| str << " * #{e.rule.name}: #{e.message}\n" end end diff --git a/src/ameba/formatter/disabled_formatter.cr b/src/ameba/formatter/disabled_formatter.cr index 8c4e03f6..3957032d 100644 --- a/src/ameba/formatter/disabled_formatter.cr +++ b/src/ameba/formatter/disabled_formatter.cr @@ -5,7 +5,7 @@ module Ameba::Formatter output << "Disabled rules using inline directives: \n\n" sources.each do |source| - source.errors.select(&.disabled?).each do |e| + source.issues.select(&.disabled?).each do |e| if loc = e.location output << "#{source.path}:#{loc.line_number}".colorize(:cyan) output << " #{e.rule.name}\n" diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr index 88b3edb6..f3f97028 100644 --- a/src/ameba/formatter/dot_formatter.cr +++ b/src/ameba/formatter/dot_formatter.cr @@ -24,10 +24,10 @@ module Ameba::Formatter failed_sources = sources.reject &.valid? failed_sources.each do |source| - source.errors.each do |error| - next if error.disabled? - output << "#{error.location}\n".colorize(:cyan) - output << "#{error.rule.name}: #{error.message}\n\n".colorize(:red) + source.issues.each do |issue| + next if issue.disabled? + output << "#{issue.location}\n".colorize(:cyan) + output << "#{issue.rule.name}: #{issue.message}\n\n".colorize(:red) end end @@ -71,7 +71,7 @@ module Ameba::Formatter private def final_message(sources, failed_sources) total = sources.size - failures = failed_sources.map { |f| f.errors.size }.sum + failures = failed_sources.map { |f| f.issues.size }.sum color = failures == 0 ? :green : :red s = failures != 1 ? "s" : "" diff --git a/src/ameba/formatter/flycheck_formatter.cr b/src/ameba/formatter/flycheck_formatter.cr index 21e96547..47e0e388 100644 --- a/src/ameba/formatter/flycheck_formatter.cr +++ b/src/ameba/formatter/flycheck_formatter.cr @@ -1,7 +1,7 @@ module Ameba::Formatter class FlycheckFormatter < BaseFormatter def source_finished(source : Source) - source.errors.each do |e| + source.issues.each do |e| next if e.disabled? if loc = e.location 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 a1b8e8a6..9e982cdc 100644 --- a/src/ameba/formatter/json_formatter.cr +++ b/src/ameba/formatter/json_formatter.cr @@ -13,7 +13,7 @@ module Ameba::Formatter # }, # "sources": [ # { - # "errors": [ + # "issues": [ # { # "location": { # "column": 7, @@ -43,7 +43,7 @@ module Ameba::Formatter # }, # ], # "summary": { - # "errors_count": 3, + # "issues_count": 3, # "target_sources_count": 1, # }, # } @@ -61,10 +61,10 @@ module Ameba::Formatter def source_finished(source : Source) json_source = AsJSON::Source.new source.path - source.errors.each do |e| + source.issues.each do |e| next if e.disabled? - json_source.errors << AsJSON::Error.new(e.rule.name, e.location, e.message) - @result.summary.errors_count += 1 + json_source.issues << AsJSON::Issue.new(e.rule.name, e.location, e.message) + @result.summary.issues_count += 1 end @result.sources << json_source @@ -87,13 +87,13 @@ module Ameba::Formatter record Source, path : String, - errors = [] of Error do + issues = [] of Issue do def to_json(json) - {path: path, errors: errors}.to_json(json) + {path: path, issues: issues}.to_json(json) end end - record Error, + record Issue, rule_name : String, location : Crystal::Location?, message : String do @@ -120,12 +120,12 @@ module Ameba::Formatter class Summary property target_sources_count = 0 - property errors_count = 0 + property issues_count = 0 def to_json(json) json.object do json.field :target_sources_count, target_sources_count - json.field :errors_count, errors_count + json.field :issues_count, issues_count end end end diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr index b0f0bfa8..d0bb22dc 100644 --- a/src/ameba/formatter/todo_formatter.cr +++ b/src/ameba/formatter/todo_formatter.cr @@ -1,6 +1,6 @@ module Ameba::Formatter # A formatter that creates a todo config. - # Basically, it takes all errors reported and disables corresponding rules + # Basically, it takes all issues reported and disables corresponding rules # or excludes failed sources from these rules. class TODOFormatter < DotFormatter @io : IO::FileDescriptor | IO::Memory @@ -10,30 +10,30 @@ module Ameba::Formatter def finished(sources) super - errors = sources.map(&.errors).flatten - generate_todo_config errors if errors.any? + issues = sources.map(&.issues).flatten + generate_todo_config issues if issues.any? if (io = @io).is_a?(File) @output << "Created #{io.path}\n" end end - private def generate_todo_config(errors) + private def generate_todo_config(issues) @io << header - rule_errors_map(errors).each do |rule, rule_errors| - @io << "\n# Problems found: #{rule_errors.size}" + rule_issues_map(issues).each do |rule, rule_issues| + @io << "\n# Problems found: #{rule_issues.size}" @io << "\n# Run `ameba --only #{rule.name}` for details" - @io << rule_todo(rule, rule_errors).gsub("---", "") + @io << rule_todo(rule, rule_issues).gsub("---", "") end ensure @io.flush end - private def rule_errors_map(errors) - Hash(Rule::Base, Array(Source::Error)).new.tap do |h| - errors.each do |error| - next if error.disabled? || error.rule.is_a? Rule::Syntax - h[error.rule] ||= Array(Source::Error).new - h[error.rule] << error + private def rule_issues_map(issues) + Hash(Rule::Base, Array(Issue)).new.tap do |h| + issues.each do |issue| + next if issue.disabled? || issue.rule.is_a? Rule::Syntax + h[issue.rule] ||= Array(Issue).new + h[issue.rule] << issue end end end @@ -48,9 +48,9 @@ module Ameba::Formatter HEADER end - private def rule_todo(rule, errors) + private def rule_todo(rule, issues) rule.excluded = - errors.map(&.location.try &.filename.try &.to_s) + issues.map(&.location.try &.filename.try &.to_s) .compact .uniq! diff --git a/src/ameba/issue.cr b/src/ameba/issue.cr new file mode 100644 index 00000000..3dcb7873 --- /dev/null +++ b/src/ameba/issue.cr @@ -0,0 +1,22 @@ +module Ameba + # Represents an issue reported by Ameba. + record Issue, + # A rule that triggers this issue. + rule : Rule::Base, + + # Location of the issue. + location : Crystal::Location?, + + # End location of the issue. + end_location : Crystal::Location?, + + # Issue message. + message : String, + + # Issue status. + status : Symbol? do + def disabled? + status == :disabled + end + end +end diff --git a/src/ameba/reportable.cr b/src/ameba/reportable.cr new file mode 100644 index 00000000..8cd13491 --- /dev/null +++ b/src/ameba/reportable.cr @@ -0,0 +1,34 @@ +module Ameba + # Represents a module used to report issues. + module Reportable + # List of reported issues. + getter issues = [] of Issue + + # Adds a new issue to the list of issues. + def add_issue(rule, location, end_location, message, status = nil) + status ||= :disabled if location_disabled?(location, rule.name) + issues << Issue.new rule, location, end_location, message, status + end + + # Adds a new issue for AST *node*. + def add_issue(rule, node, message, **args) + add_issue rule, node.location, node.end_location, message, **args + end + + # Adds a new issue for Crystal *token*. + def add_issue(rule, token, message, **args) + add_issue rule, token.location, nil, message, **args + end + + # Adds a new issue for *location* defined by line and column numbers. + def add_issue(rule, location : Tuple(Int32, Int32), message, **args) + location = Crystal::Location.new path, *location + add_issue rule, location, nil, message, **args + end + + # Returns true if the list of not disabled issues is empty, false otherwise. + def valid? + issues.reject(&.disabled?).empty? + end + end +end diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 8a8d9404..11055796 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -13,7 +13,7 @@ module Ameba::Rule # struct MyRule < Ameba::Rule::Base # def test(source) # if invalid?(source) - # source.error self, location, "Something wrong." + # issue_for line, column, "Something wrong." # end # end # @@ -25,13 +25,13 @@ module Ameba::Rule # # Enforces rules to implement an abstract `#test` method which # is designed to test the source passed in. If source has issues - # that are tested by this rule, it should add an error. + # that are tested by this rule, it should add an issue. # abstract struct Base include Config::RuleConfig # This method is designed to test the source passed in. If source has issues - # that are tested by this rule, it should add an error. + # that are tested by this rule, it should add an issue. abstract def test(source : Source) def test(source : Source, node : Crystal::ASTNode, *opts) @@ -91,6 +91,10 @@ module Ameba::Rule SPECIAL.includes? name end + macro issue_for(*args) + source.add_issue self, {{*args}} + end + protected def self.rule_name name.gsub("Ameba::Rule::", "") end diff --git a/src/ameba/rule/comparison_to_boolean.cr b/src/ameba/rule/comparison_to_boolean.cr index ff38bfc5..6be9c923 100644 --- a/src/ameba/rule/comparison_to_boolean.cr +++ b/src/ameba/rule/comparison_to_boolean.cr @@ -39,7 +39,7 @@ module Ameba::Rule return unless comparison? && to_boolean? - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/rule/constant_names.cr b/src/ameba/rule/constant_names.cr index 30cef1a0..883b8fda 100644 --- a/src/ameba/rule/constant_names.cr +++ b/src/ameba/rule/constant_names.cr @@ -38,7 +38,7 @@ module Ameba::Rule name = target.names.first return if (expected = name.upcase) == name - source.error self, node.location, sprintf(MSG, expected, name) + issue_for node, MSG % {expected, name} end end end diff --git a/src/ameba/rule/debugger_statement.cr b/src/ameba/rule/debugger_statement.cr index 1c6c76d1..ee18e862 100644 --- a/src/ameba/rule/debugger_statement.cr +++ b/src/ameba/rule/debugger_statement.cr @@ -27,7 +27,7 @@ module Ameba::Rule node.args.empty? && node.obj.nil? - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/rule/empty_ensure.cr b/src/ameba/rule/empty_ensure.cr index b56da975..399fa545 100644 --- a/src/ameba/rule/empty_ensure.cr +++ b/src/ameba/rule/empty_ensure.cr @@ -47,7 +47,7 @@ module Ameba::Rule node_ensure = node.ensure return if node_ensure.nil? || !node_ensure.nop? - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/rule/empty_expression.cr b/src/ameba/rule/empty_expression.cr index 3b4b6ce0..2cd64415 100644 --- a/src/ameba/rule/empty_expression.cr +++ b/src/ameba/rule/empty_expression.cr @@ -47,12 +47,12 @@ module Ameba::Rule return if exp.nil? || exp == "nil" - source.error self, node.location, MSG % exp + issue_for node, MSG % exp end def test(source, node : Crystal::Expressions) if node.expressions.size == 1 && node.expressions.first.nop? - source.error self, node.location, MSG_EXRS + issue_for node, MSG_EXRS end end end diff --git a/src/ameba/rule/hash_duplicated_key.cr b/src/ameba/rule/hash_duplicated_key.cr index 828e2dae..9966aa27 100644 --- a/src/ameba/rule/hash_duplicated_key.cr +++ b/src/ameba/rule/hash_duplicated_key.cr @@ -34,7 +34,7 @@ module Ameba::Rule def test(source, node : Crystal::HashLiteral) return unless (keys = duplicated_keys(node.entries)).any? - source.error self, node.location, MSG % keys.join(", ") + issue_for node, MSG % keys.join(", ") end private def duplicated_keys(entries) diff --git a/src/ameba/rule/large_numbers.cr b/src/ameba/rule/large_numbers.cr index 3e15ac70..539c764e 100644 --- a/src/ameba/rule/large_numbers.cr +++ b/src/ameba/rule/large_numbers.cr @@ -42,7 +42,7 @@ module Ameba::Rule parsed = parse_number token.raw if allowed?(*parsed) && (expected = underscored *parsed) != token.raw - source.error self, token.location, MSG % expected + issue_for token, MSG % expected end end end diff --git a/src/ameba/rule/line_length.cr b/src/ameba/rule/line_length.cr index 7ab2ccf1..4e47e13f 100644 --- a/src/ameba/rule/line_length.cr +++ b/src/ameba/rule/line_length.cr @@ -22,7 +22,7 @@ module Ameba::Rule source.lines.each_with_index do |line, index| next unless line.size > max_length - source.error self, index + 1, line.size, MSG + issue_for({index + 1, line.size}, MSG) end end end diff --git a/src/ameba/rule/literal_in_condition.cr b/src/ameba/rule/literal_in_condition.cr index ef558e56..7d90d7b3 100644 --- a/src/ameba/rule/literal_in_condition.cr +++ b/src/ameba/rule/literal_in_condition.cr @@ -36,7 +36,7 @@ module Ameba::Rule def check_node(source, node) return unless literal?(node.cond) - source.error self, node.location, MSG + issue_for node, MSG end def test(source, node : Crystal::If) diff --git a/src/ameba/rule/literal_in_interpolation.cr b/src/ameba/rule/literal_in_interpolation.cr index 4d118ff7..93ea9c9b 100644 --- a/src/ameba/rule/literal_in_interpolation.cr +++ b/src/ameba/rule/literal_in_interpolation.cr @@ -32,7 +32,7 @@ module Ameba::Rule def test(source, node : Crystal::StringInterpolation) found = node.expressions.any? { |e| !string_literal?(e) && literal?(e) } return unless found - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/rule/method_names.cr b/src/ameba/rule/method_names.cr index bb58989e..423352c7 100644 --- a/src/ameba/rule/method_names.cr +++ b/src/ameba/rule/method_names.cr @@ -52,7 +52,7 @@ module Ameba::Rule def test(source, node : Crystal::Def) return if (expected = node.name.underscore) == node.name - source.error self, node.location, sprintf(MSG, expected, node.name) + issue_for node, MSG % {expected, node.name} end end end diff --git a/src/ameba/rule/negated_conditions_in_unless.cr b/src/ameba/rule/negated_conditions_in_unless.cr index e9e4e0c0..c60bae2b 100644 --- a/src/ameba/rule/negated_conditions_in_unless.cr +++ b/src/ameba/rule/negated_conditions_in_unless.cr @@ -40,7 +40,7 @@ module Ameba::Rule def test(source, node : Crystal::Unless) return unless negated_condition? node.cond - source.error self, node.location, MSG + issue_for node, MSG end private def negated_condition?(node) diff --git a/src/ameba/rule/percent_arrays.cr b/src/ameba/rule/percent_arrays.cr index 30335490..b7d66b1d 100644 --- a/src/ameba/rule/percent_arrays.cr +++ b/src/ameba/rule/percent_arrays.cr @@ -34,21 +34,21 @@ module Ameba::Rule MSG = "Symbols `%s` may be unwanted in %s array literals" def test(source) - error = start_token = nil + issue = start_token = nil Tokenizer.new(source).run do |token| case token.type when :STRING_ARRAY_START, :SYMBOL_ARRAY_START start_token = token.dup when :STRING - if start_token && error.nil? - error = array_entry_invalid?(token.value, start_token.not_nil!.raw) + if start_token && issue.nil? + issue = array_entry_invalid?(token.value, start_token.not_nil!.raw) end when :STRING_ARRAY_END, :SYMBOL_ARRAY_END - if error - source.error(self, start_token.try &.location, error.not_nil!) + if issue + issue_for start_token.not_nil!, issue.not_nil! end - error = start_token = nil + issue = start_token = nil end end end @@ -64,7 +64,7 @@ module Ameba::Rule private def check_array_entry(entry, symbols, literal) return unless entry =~ /[#{symbols}]/ - sprintf(MSG, symbols, literal) + MSG % {symbols, literal} end end end diff --git a/src/ameba/rule/predicate_name.cr b/src/ameba/rule/predicate_name.cr index 7f95e227..cae038b4 100644 --- a/src/ameba/rule/predicate_name.cr +++ b/src/ameba/rule/predicate_name.cr @@ -45,7 +45,7 @@ module Ameba::Rule alternative = $2 return unless alternative =~ /^[a-z][a-zA-Z_0-9]*$/ - source.error self, node.location, sprintf(MSG, alternative, node.name) + issue_for node, MSG % {alternative, node.name} end end end diff --git a/src/ameba/rule/rand_zero.cr b/src/ameba/rule/rand_zero.cr index df80ed93..80b9e2cd 100644 --- a/src/ameba/rule/rand_zero.cr +++ b/src/ameba/rule/rand_zero.cr @@ -42,7 +42,7 @@ module Ameba::Rule (value = arg.value) && (value == "0" || value == "1") - source.error self, node.location, MSG % node + issue_for node, MSG % node end end end diff --git a/src/ameba/rule/redundant_begin.cr b/src/ameba/rule/redundant_begin.cr index cc7f35e1..a81af866 100644 --- a/src/ameba/rule/redundant_begin.cr +++ b/src/ameba/rule/redundant_begin.cr @@ -71,7 +71,7 @@ module Ameba::Rule def test(source, node : Crystal::Def) return unless redundant_begin?(source, node) - source.error self, node.location, MSG + issue_for node, MSG end private def redundant_begin?(source, node) diff --git a/src/ameba/rule/shadowed_argument.cr b/src/ameba/rule/shadowed_argument.cr index e5b4d6bd..b1eb298b 100644 --- a/src/ameba/rule/shadowed_argument.cr +++ b/src/ameba/rule/shadowed_argument.cr @@ -51,7 +51,7 @@ module Ameba::Rule scope.arguments.each do |arg| next unless assign = arg.variable.assign_before_reference - source.error self, assign.location, MSG % arg.name + issue_for assign, MSG % arg.name end end end diff --git a/src/ameba/rule/shadowed_exception.cr b/src/ameba/rule/shadowed_exception.cr index 7af41f8b..21d18eb3 100644 --- a/src/ameba/rule/shadowed_exception.cr +++ b/src/ameba/rule/shadowed_exception.cr @@ -49,7 +49,7 @@ module Ameba::Rule return unless excs = node.rescues if (excs = shadowed excs.map(&.types)).any? - source.error self, node.location, MSG % excs.join(", ") + issue_for node, MSG % excs.join(", ") end end diff --git a/src/ameba/rule/shadowing_local_outer_var.cr b/src/ameba/rule/shadowing_local_outer_var.cr index 3f9fe164..2048f97e 100644 --- a/src/ameba/rule/shadowing_local_outer_var.cr +++ b/src/ameba/rule/shadowing_local_outer_var.cr @@ -54,7 +54,7 @@ module Ameba::Rule private def find_shadowing(source, scope) scope.arguments.each do |arg| if scope.outer_scope.try &.find_variable(arg.name) - source.error self, arg.location, MSG % arg.name + issue_for arg, MSG % arg.name end end end diff --git a/src/ameba/rule/syntax.cr b/src/ameba/rule/syntax.cr index b8c7fe90..0c57f903 100644 --- a/src/ameba/rule/syntax.cr +++ b/src/ameba/rule/syntax.cr @@ -23,7 +23,7 @@ module Ameba::Rule def test(source) source.ast rescue e : Crystal::SyntaxException - source.error self, e.line_number, e.column_number, e.message.to_s + issue_for({e.line_number, e.column_number}, e.message.to_s) end end end diff --git a/src/ameba/rule/trailing_blank_lines.cr b/src/ameba/rule/trailing_blank_lines.cr index 3b6663e7..251401bc 100644 --- a/src/ameba/rule/trailing_blank_lines.cr +++ b/src/ameba/rule/trailing_blank_lines.cr @@ -17,7 +17,7 @@ module Ameba::Rule def test(source) if source.lines.size > 1 && source.lines[-2, 2].join.strip.empty? - source.error self, source.lines.size, 1, MSG + issue_for({source.lines.size, 1}, MSG) end end end diff --git a/src/ameba/rule/trailing_whitespace.cr b/src/ameba/rule/trailing_whitespace.cr index 6ef481f6..9200110d 100644 --- a/src/ameba/rule/trailing_whitespace.cr +++ b/src/ameba/rule/trailing_whitespace.cr @@ -18,7 +18,7 @@ module Ameba::Rule def test(source) source.lines.each_with_index do |line, index| next unless line =~ /\s$/ - source.error self, index + 1, line.size, MSG + issue_for({index + 1, line.size}, MSG) end end end diff --git a/src/ameba/rule/type_names.cr b/src/ameba/rule/type_names.cr index 1b274f6b..78b116bb 100644 --- a/src/ameba/rule/type_names.cr +++ b/src/ameba/rule/type_names.cr @@ -68,7 +68,7 @@ module Ameba::Rule expected = name.camelcase return if expected == name - source.error self, node.location, sprintf(MSG, expected, name) + issue_for node, MSG % {expected, name} end def test(source, node : Crystal::ClassDef) diff --git a/src/ameba/rule/unless_else.cr b/src/ameba/rule/unless_else.cr index afab396f..4c6714c6 100644 --- a/src/ameba/rule/unless_else.cr +++ b/src/ameba/rule/unless_else.cr @@ -56,7 +56,7 @@ module Ameba::Rule def test(source, node : Crystal::Unless) return if node.else.nop? - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/rule/unneeded_disable_directive.cr b/src/ameba/rule/unneeded_disable_directive.cr index 02ecb7e2..4a34fa27 100644 --- a/src/ameba/rule/unneeded_disable_directive.cr +++ b/src/ameba/rule/unneeded_disable_directive.cr @@ -32,7 +32,7 @@ module Ameba::Rule next unless names = unneeded_disables(source, directive, token.location) next unless names.any? - source.error self, token.location, MSG % names.join(", ") + issue_for token, MSG % names.join(", ") end end @@ -40,19 +40,19 @@ module Ameba::Rule return unless directive[:action] == "disable" directive[:rules].reject do |rule_name| - source.errors.any? do |error| - error.rule.name == rule_name && - error.disabled? && - error_at_location?(source, error, location) + source.issues.any? do |issue| + issue.rule.name == rule_name && + issue.disabled? && + issue_at_location?(source, issue, location) end && rule_name != self.name end end - private def error_at_location?(source, error, location) - return false unless error_line_number = error.location.try(&.line_number) + private def issue_at_location?(source, issue, location) + return false unless issue_line_number = issue.location.try(&.line_number) - error_line_number == location.line_number || - ((prev_line_number = error_line_number - 1) && + issue_line_number == location.line_number || + ((prev_line_number = issue_line_number - 1) && prev_line_number == location.line_number && source.comment?(prev_line_number - 1)) end diff --git a/src/ameba/rule/unused_argument.cr b/src/ameba/rule/unused_argument.cr index 0f904feb..b1bb46e3 100644 --- a/src/ameba/rule/unused_argument.cr +++ b/src/ameba/rule/unused_argument.cr @@ -58,7 +58,7 @@ module Ameba::Rule next if argument.ignored? || scope.references?(argument.variable) name_suggestion = scope.node.is_a?(Crystal::Block) ? '_' : "_#{argument.name}" - source.error self, argument.location, MSG % {argument.name, name_suggestion} + issue_for argument, MSG % {argument.name, name_suggestion} end end end diff --git a/src/ameba/rule/useless_assign.cr b/src/ameba/rule/useless_assign.cr index 1e5a4d50..3a97f68b 100644 --- a/src/ameba/rule/useless_assign.cr +++ b/src/ameba/rule/useless_assign.cr @@ -43,7 +43,7 @@ module Ameba::Rule var.assignments.each do |assign| next if assign.referenced? - source.error self, assign.location, MSG % var.name + issue_for assign, MSG % var.name end end end diff --git a/src/ameba/rule/useless_condition_in_when.cr b/src/ameba/rule/useless_condition_in_when.cr index a8652383..c8e4fbb7 100644 --- a/src/ameba/rule/useless_condition_in_when.cr +++ b/src/ameba/rule/useless_condition_in_when.cr @@ -48,7 +48,7 @@ module Ameba::Rule .map(&.to_s) .none? { |c| c == cond_s } - source.error self, cond.location, MSG + issue_for cond, MSG end def test(source) diff --git a/src/ameba/rule/variable_names.cr b/src/ameba/rule/variable_names.cr index 8b4dbf4e..d179280a 100644 --- a/src/ameba/rule/variable_names.cr +++ b/src/ameba/rule/variable_names.cr @@ -45,7 +45,7 @@ module Ameba::Rule private def check_node(source, node) return if (expected = node.name.underscore) == node.name - source.error self, node.location, sprintf(MSG, expected, node.name) + issue_for node, MSG % {expected, node.name} end def test(source, node : Crystal::Var) diff --git a/src/ameba/rule/while_true.cr b/src/ameba/rule/while_true.cr index 22ab5585..308f2f78 100644 --- a/src/ameba/rule/while_true.cr +++ b/src/ameba/rule/while_true.cr @@ -39,7 +39,7 @@ module Ameba::Rule def test(source, node : Crystal::While) return unless node.cond.true_literal? - source.error self, node.location, MSG + issue_for node, MSG end end end diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index d23d4693..00e3536d 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -52,7 +52,7 @@ module Ameba # Performs the inspection. Iterates through all sources and test it using # list of rules. If a specific rule fails on a specific source, it adds - # an error to that source. + # an issue to that source. # # This action also notifies formatter when inspection is started/finished, # and when a specific source started/finished to be inspected. diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 3184b05b..9337f012 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -1,22 +1,9 @@ module Ameba # An entity that represents a Crystal source file. - # Has path, lines of code and errors reported by rules. + # Has path, lines of code and issues reported by rules. class Source include InlineComments - - # Represents an error caught by Ameba. - # - # Each error has the rule that created this error, - # location of the issue, message and status. - record Error, - rule : Rule::Base, - location : Crystal::Location?, - message : String, - status : Symbol? do - def disabled? - status == :disabled - end - end + include Reportable # Path to the source file. getter path : String @@ -24,9 +11,6 @@ module Ameba # Crystal code (content of a source file). getter code : String - # List of errors reported. - getter errors = [] of Error - @lines : Array(String)? @ast : Crystal::ASTNode? @fullpath : String? @@ -43,42 +27,6 @@ module Ameba def initialize(@code : String, @path = "") end - # Adds a new error to the list of errors. - # - # ``` - # source.error rule, location, "Line too long" - # ``` - # - def error(rule : Rule::Base, location, message : String, status = nil) - status ||= :disabled if location_disabled?(location, rule.name) - errors << Error.new rule, location, message, status - end - - # Adds a new error to the list of errors using line and column number. - # - # ``` - # source.error rule, line_number, column_number, "Bad code" - # ``` - # - def error(rule : Rule::Base, l, c, message : String, status = nil) - location = Crystal::Location.new path, l, c - error rule, location, message, status - end - - # Indicates whether source is valid or not. - # Returns true if the list or errors empty, false otherwise. - # - # ``` - # source = Ameba::Source.new code, path - # source.valid? # => true - # source.error rule, location, message - # source.valid? # => false - # ``` - # - def valid? - errors.reject(&.disabled?).empty? - 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