Merge pull request #100 from veelenga/feat/severity

Severity

spec/ameba/cli/cmd_spec.cr

@@ -114,6 +114,29 @@ module Ameba::Cli
         end
       end
 
+      context "--fail-level" do
+        it "configures fail level Refactoring" do
+          c = Cli.parse_args %w(--fail-level refactoring)
+          c.fail_level.should eq Severity::Refactoring
+        end
+
+        it "configures fail level Warning" do
+          c = Cli.parse_args %w(--fail-level Warning)
+          c.fail_level.should eq Severity::Warning
+        end
+
+        it "configures fail level Error" do
+          c = Cli.parse_args %w(--fail-level error)
+          c.fail_level.should eq Severity::Error
+        end
+
+        it "raises if fail level is incorrect" do
+          expect_raises(Exception, "Incorrect severity name JohnDoe") do
+            Cli.parse_args %w(--fail-level JohnDoe)
+          end
+        end
+      end
+
       it "accepts unknown args as globs" do
         c = Cli.parse_args %w(source1.cr source2.cr)
         c.globs.should eq %w(source1.cr source2.cr)

spec/ameba/formatter/dot_formatter_spec.cr

@@ -64,6 +64,19 @@ module Ameba::Formatter
           log.should contain "      \e[33m^\e[0m"
         end
 
+        it "writes severity" do
+          output.clear
+          s = Source.new(%(
+            a = 22
+            puts a
+          )).tap do |source|
+            source.add_issue(DummyRule.new, {1, 5}, "DummyRuleError")
+          end
+          subject.finished [s]
+          log = output.to_s
+          log.should contain "[R]"
+        end
+
         it "doesn't write affected code if it is disabled" do
           output.clear
           s = Source.new(%(

spec/ameba/formatter/explain_formatter_spec.cr

@@ -50,6 +50,7 @@ module Ameba
         source = Source.new "a = 42", "source.cr"
         output = explanation(source)
         output.should contain "RULE INFO"
+        output.should contain "Refactoring"
         output.should contain "Ameba/ErrorRule"
         output.should contain "Always adds an error at 1:1"
       end

spec/ameba/formatter/flycheck_formatter_spec.cr

@@ -22,7 +22,7 @@ module Ameba::Formatter
         subject = flycheck
         subject.source_finished s
         subject.output.to_s.should eq(
-          "source.cr:1:2: E: [#{DummyRule.rule_name}] message\n"
+          "source.cr:1:2: R: [#{DummyRule.rule_name}] message\n"
         )
       end
 
@@ -32,7 +32,7 @@ module Ameba::Formatter
         subject = flycheck
         subject.source_finished s
         subject.output.to_s.should eq(
-          "source.cr:1:2: E: [#{DummyRule.rule_name}] multi line\n"
+          "source.cr:1:2: R: [#{DummyRule.rule_name}] multi line\n"
         )
       end
 

spec/ameba/formatter/json_formatter_spec.cr

@@ -37,6 +37,14 @@ module Ameba
         result["sources"][0]["issues"][0]["rule_name"].should eq DummyRule.rule_name
       end
 
+      it "shows severity" do
+        s = Source.new ""
+        s.add_issue DummyRule.new, {1, 2}, "message"
+
+        result = get_result [s]
+        result["sources"][0]["issues"][0]["severity"].should eq "Refactoring"
+      end
+
       it "shows a message" do
         s = Source.new ""
         s.add_issue DummyRule.new, {1, 2}, "message"

spec/ameba/formatter/todo_formatter_spec.cr

@@ -43,6 +43,10 @@ module Ameba
         create_todo.should contain "DummyRule"
       end
 
+      it "creates a todo with severity" do
+        create_todo.should contain "Refactoring"
+      end
+
       it "creates a todo with problems count" do
         create_todo.should contain "Problems found: 1"
       end

spec/ameba/rule/lint/syntax_spec.cr

@@ -33,5 +33,9 @@ module Ameba::Rule::Lint
       issue.location.to_s.should eq "source.cr:1:11"
       issue.message.should eq "unexpected token: end (expected ';' or newline)"
     end
+
+    it "has highest severity" do
+      subject.severity.should eq Severity::Error
+    end
   end
 end

spec/ameba/runner_spec.cr

@@ -13,6 +13,7 @@ module Ameba
 
   describe Runner do
     formatter = DummyFormatter.new
+    default_severity = Severity::Refactoring
 
     describe "#run" do
       it "returns self" do
@@ -49,7 +50,7 @@ module Ameba
           rules << rule
         end
 
-        Runner.new(all_rules, [source], formatter).run.success?.should be_true
+        Runner.new(all_rules, [source], formatter, default_severity).run.success?.should be_true
       end
 
       context "invalid syntax" do
@@ -57,7 +58,7 @@ module Ameba
           rules = [Rule::Lint::Syntax.new] of Rule::Base
           source = Source.new "def bad_syntax"
 
-          Runner.new(rules, [source], formatter).run
+          Runner.new(rules, [source], formatter, default_severity).run
           source.should_not be_valid
           source.issues.first.rule.name.should eq Rule::Lint::Syntax.rule_name
         end
@@ -70,7 +71,7 @@ module Ameba
               when my_bad_syntax
           )
 
-          Runner.new(rules, [source], formatter).run
+          Runner.new(rules, [source], formatter, default_severity).run
           source.should_not be_valid
           source.issues.size.should eq 1
         end
@@ -83,7 +84,7 @@ module Ameba
             a = 1 # ameba:disable LineLength
           )
 
-          Runner.new(rules, [source], formatter).run
+          Runner.new(rules, [source], formatter, default_severity).run
           source.should_not be_valid
           source.issues.first.rule.name.should eq Rule::Lint::UnneededDisableDirective.rule_name
         end
@@ -107,7 +108,7 @@ module Ameba
             a = 1
           ), "source.cr"
 
-        runner = Runner.new(rules, [source], formatter).run
+        runner = Runner.new(rules, [source], formatter, default_severity).run
         runner.explain({file: "source.cr", line: 1, column: 1}, io)
         io.to_s.should_not be_empty
       end
@@ -119,7 +120,7 @@ module Ameba
             a = 1
           ), "source.cr"
 
-        runner = Runner.new(rules, [source], formatter).run
+        runner = Runner.new(rules, [source], formatter, default_severity).run
         runner.explain({file: "source.cr", line: 1, column: 2}, io)
         io.to_s.should be_empty
       end
@@ -139,7 +140,15 @@ module Ameba
         s = Source.new %q(
           WrongConstant = 5
         )
-        Runner.new(rules, [s], formatter).run.success?.should be_false
+        Runner.new(rules, [s], formatter, default_severity).run.success?.should be_false
+      end
+
+      it "depends on the level of severity" do
+        rules = Rule.rules.map &.new.as(Rule::Base)
+        s = Source.new %q(WrongConstant = 5)
+        Runner.new(rules, [s], formatter, Severity::Error).run.success?.should be_true
+        Runner.new(rules, [s], formatter, Severity::Warning).run.success?.should be_true
+        Runner.new(rules, [s], formatter, Severity::Refactoring).run.success?.should be_false
       end
     end
   end

spec/ameba/severity_spec.cr

@@ -0,0 +1,107 @@
+require "../spec_helper"
+
+module Ameba
+  describe Severity do
+    describe "#symbol" do
+      it "returns the symbol for each severity in the enum" do
+        Severity.values.each do |severity|
+          severity.symbol.should_not be_nil
+        end
+      end
+
+      it "returns symbol for Error" do
+        Severity::Error.symbol.should eq 'E'
+      end
+
+      it "returns symbol for Warning" do
+        Severity::Warning.symbol.should eq 'W'
+      end
+
+      it "returns symbol for Refactoring" do
+        Severity::Refactoring.symbol.should eq 'R'
+      end
+    end
+
+    describe ".from_name" do
+      it "creates error severity by name" do
+        Severity.from_name("Error").should eq Severity::Error
+      end
+
+      it "creates warning severity by name" do
+        Severity.from_name("Warning").should eq Severity::Warning
+      end
+
+      it "creates refactoring severity by name" do
+        Severity.from_name("Refactoring").should eq Severity::Refactoring
+      end
+
+      it "raises when name is incorrect" do
+        expect_raises(Exception, "Incorrect severity name BadName. Try one of [Error, Warning, Refactoring]") do
+          Severity.from_name("BadName")
+        end
+      end
+    end
+  end
+
+  struct SeverityConvertable
+    YAML.mapping(
+      severity: {type: Severity, converter: SeverityYamlConverter}
+    )
+  end
+
+  describe SeverityYamlConverter do
+    describe ".from_yaml" do
+      it "converts from yaml to Severity::Error" do
+        yaml = {severity: "error"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml)
+        converted.severity.should eq Severity::Error
+      end
+
+      it "converts from yaml to Severity::Warning" do
+        yaml = {severity: "warning"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml)
+        converted.severity.should eq Severity::Warning
+      end
+
+      it "converts from yaml to Severity::Refactoring" do
+        yaml = {severity: "refactoring"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml)
+        converted.severity.should eq Severity::Refactoring
+      end
+
+      it "raises if severity is not a scalar" do
+        yaml = {severity: {refactoring: true}}.to_yaml
+        expect_raises(Exception, "Severity must be a scalar") do
+          SeverityConvertable.from_yaml(yaml)
+        end
+      end
+
+      it "raises if severity has a wrong type" do
+        yaml = {severity: [1, 2, 3]}.to_yaml
+        expect_raises(Exception, "Severity must be a scalar") do
+          SeverityConvertable.from_yaml(yaml)
+        end
+      end
+    end
+
+    describe ".to_yaml" do
+      it "converts Severity::Error to yaml" do
+        yaml = {severity: "error"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml).to_yaml
+        converted.should eq "---\nseverity: Error\n"
+      end
+
+      it "converts Severity::Warning to yaml" do
+        yaml = {severity: "warning"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml).to_yaml
+        converted.should eq "---\nseverity: Warning\n"
+      end
+
+      it "converts Severity::Refactoring to yaml" do
+        yaml = {severity: "refactoring"}.to_yaml
+        converted = SeverityConvertable.from_yaml(yaml).to_yaml
+        converted.should eq "---\nseverity: Refactoring\n"
+      end
+    end
+  end
+end

src/ameba/cli/cmd.cr

@@ -9,6 +9,7 @@ module Ameba::Cli
     opts = parse_args args
     config = Config.load opts.config, opts.colors?
     config.globs = opts.globs
+    config.severity = opts.fail_level.not_nil! if opts.fail_level
 
     configure_formatter(config, opts)
     configure_rules(config, opts)
@@ -32,6 +33,7 @@ module Ameba::Cli
     property only : Array(String)?
     property except : Array(String)?
     property location_to_explain : NamedTuple(file: String, line: Int32, column: Int32)?
+    property fail_level : Severity?
     property? all = false
     property? colors = true
     property? without_affected_code = false
@@ -82,6 +84,10 @@ module Ameba::Cli
         opts.config = ""
       end
 
+      parser.on("--fail-level SEVERITY", "Change the level of failure to exit. Defaults to Refactoring") do |level|
+        opts.fail_level = Severity.from_name(level)
+      end
+
       parser.on("-e", "--explain PATH:line:column",
         "Explain an issue at a specified location") do |loc|
         configure_explain_opts(loc, opts)

src/ameba/config.cr

@@ -28,6 +28,7 @@ class Ameba::Config
   setter formatter : Formatter::BaseFormatter?
   setter globs : Array(String)?
   getter rules : Array(Rule::Base)
+  property severity = Severity::Refactoring
 
   @rule_groups : Hash(String, Array(Rule::Base))
 
@@ -193,6 +194,12 @@ class Ameba::Config
         {% key = name.camelcase.stringify %}
         {% value = df[:value] %}
         {% type = df[:type] %}
+        {% converter = nil %}
+
+        {% if key == "Severity" %}
+          {% type = Severity %}
+          {% converter = SeverityYamlConverter %}
+        {% end %}
 
         {% if type == nil %}
           {% if value.is_a? BoolLiteral %}
@@ -214,13 +221,18 @@ class Ameba::Config
           {% type = Nil if type == nil %}
         {% end %}
 
-        {% properties[name] = {key: key, default: value, type: type} %}
+        {% properties[name] = {key: key, default: value, type: type, converter: converter} %}
       {% end %}
 
       {% if properties["enabled".id] == nil %}
         {% properties["enabled".id] = {key: "Enabled", default: true, type: Bool} %}
       {% end %}
 
+      {% if properties["severity".id] == nil %}
+        {% default = @type.name.starts_with?("Ameba::Rule::Lint") ? "Severity::Warning".id : "Severity::Refactoring".id %}
+        {% properties["severity".id] = {key: "Severity", default: default, type: Severity, converter: SeverityYamlConverter} %}
+      {% end %}
+
       {% if properties["excluded".id] == nil %}
         {% properties["excluded".id] = {key: "Excluded", type: "Array(String)?".id} %}
       {% end %}

src/ameba/formatter/dot_formatter.cr

@@ -35,7 +35,7 @@ module Ameba::Formatter
           next if (location = issue.location).nil?
 
           output << "#{location}\n".colorize(:cyan)
-          output << "#{issue.rule.name}: #{issue.message}\n".colorize(:red)
+          output << "[#{issue.rule.severity.symbol}] #{issue.rule.name}: #{issue.message}\n".colorize(:red)
 
           if show_affected_code && (code = affected_code(source, location))
             output << code

src/ameba/formatter/explain_formatter.cr

@@ -57,7 +57,7 @@ module Ameba::Formatter
 
       if rule.responds_to?(:description)
         output_title "RULE INFO"
-        output_paragraph [rule.name, rule.description]
+        output_paragraph [rule.severity.to_s, rule.name, rule.description]
       end
 
       output_title "DETAILED DESCRIPTION"

src/ameba/formatter/flycheck_formatter.cr

@@ -5,7 +5,7 @@ module Ameba::Formatter
         next if e.disabled?
         if loc = e.location
           output.printf "%s:%d:%d: %s: [%s] %s\n",
-            source.path, loc.line_number, loc.column_number, "E",
+            source.path, loc.line_number, loc.column_number, e.rule.severity.symbol,
             e.rule.name, e.message.gsub("\n", " ")
         end
       end

src/ameba/formatter/json_formatter.cr

@@ -25,6 +25,7 @@ module Ameba::Formatter
   #           },
   #           "message":   "Useless assignment to variable `a`",
   #           "rule_name": "UselessAssign",
+  #           "severity":  "Refactoring",
   #         },
   #         {
   #           "location": {
@@ -49,6 +50,7 @@ module Ameba::Formatter
   #           },
   #           "message":   "Useless assignment to variable `a`",
   #           "rule_name": "UselessAssign",
+  #           "severity":  "Refactoring",
   #         },
   #       ],
   #       "path": "src/ameba/formatter/json_formatter.cr",
@@ -75,7 +77,7 @@ module Ameba::Formatter
 
       source.issues.each do |e|
         next if e.disabled?
-        json_source.issues << AsJSON::Issue.new(e.rule.name, e.location, e.end_location, e.message)
+        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
 
@@ -107,12 +109,14 @@ module Ameba::Formatter
 
     record Issue,
       rule_name : String,
+      severity : String,
       location : Crystal::Location?,
       end_location : Crystal::Location?,
       message : String do
       def to_json(json)
         json.object do
           json.field :rule_name, rule_name
+          json.field :severity, severity
           json.field :message, message
           json.field :location,
             {line: location.try &.line_number, column: location.try &.column_number}

src/ameba/rule/lint/syntax.cr

@@ -20,7 +20,10 @@ module Ameba::Rule::Lint
   # ```
   #
   struct Syntax < Base
-    getter description = "Reports invalid Crystal syntax."
+    properties do
+      description "Reports invalid Crystal syntax"
+      severity Severity::Error
+    end
 
     def test(source)
       source.ast

src/ameba/runner.cr

@@ -17,6 +17,9 @@ module Ameba
     # A list of sources to run inspection on.
     getter sources : Array(Source)
 
+    # A level of severity to be reported.
+    @severity : Severity
+
     # A formatter to prepare report.
     @formatter : Formatter::BaseFormatter
 
@@ -39,6 +42,7 @@ module Ameba
     def initialize(config : Config)
       @sources = config.sources
       @formatter = config.formatter
+      @severity = config.severity
       @rules = config.rules.select(&.enabled).reject!(&.special?)
 
       @unneeded_disable_directive_rule =
@@ -47,7 +51,7 @@ module Ameba
     end
 
     # :nodoc:
-    protected def initialize(@rules, @sources, @formatter)
+    protected def initialize(@rules, @sources, @formatter, @severity)
     end
 
     # Performs the inspection. Iterates through all sources and test it using
@@ -98,7 +102,7 @@ module Ameba
     end
 
     # Indicates whether the last inspection successful or not.
-    # It returns true if no issues in sources found, false otherwise.
+    # It returns true if no issues matching severity in sources found, false otherwise.
     #
     # ```
     # runner = Ameba::Runner.new config
@@ -107,7 +111,9 @@ module Ameba
     # ```
     #
     def success?
-      @sources.all? &.valid?
+      @sources.all? do |source|
+        source.issues.none? { |issue| issue.rule.severity <= @severity }
+      end
     end
 
     private def check_unneeded_directives(source)

src/ameba/severity.cr

@@ -0,0 +1,49 @@
+module Ameba
+  enum Severity
+    Error
+    Warning
+    Refactoring
+
+    # Returns a symbol uniquely indicating severity.
+    #
+    # ```
+    # Severity::Warning.symbol # => 'W'
+    # ```
+    def symbol
+      to_s[0]
+    end
+
+    # Creates Severity by the name.
+    #
+    # ```
+    # Severity.from_name('refactoring') # => Severity::Refactoring
+    # Severity.from_name('foo-bar')     # => Exception: Incorrect severity name..
+    # ```
+    #
+    def self.from_name(name : String)
+      parse(name)
+    rescue ArgumentError
+      raise "Incorrect severity name #{name}. Try one of #{Severity.values}"
+    end
+  end
+
+  # Converter for `YAML.mapping` which converts severity enum to and from YAML.
+  class SeverityYamlConverter
+    def self.from_yaml(ctx : YAML::ParseContext, node : YAML::Nodes::Node)
+      unless node.is_a?(YAML::Nodes::Scalar)
+        raise "Severity must be a scalar, not #{node.class}"
+      end
+
+      case value = node.value
+      when String then Severity.from_name(value)
+      when Nil    then nil
+      else
+        raise "Incorrect severity: #{value}"
+      end
+    end
+
+    def self.to_yaml(value : Severity, yaml : YAML::Nodes::Builder)
+      yaml.scalar value
+    end
+  end
+end