Make config loading more flexible

README.md

@@ -108,35 +108,32 @@ Each rule is enabled by default, even if you remove it from the config file.
 
 ## Writing a new Rule
 
-Adding a new rule is as simple as inheriting from `Rule::Base` struct and implementing
-your logic to detect a problem:
+Adding a new rule is as simple as inheriting from `Ameba::Rule::Base` struct and implementing
+a logic to detect a problem in the source file:
 
 ```crystal
-struct DebuggerStatement < Rule::Base
+struct MySuperRule < Ameba::Rule::Base
   # This is a required method to be implemented by the rule.
-  # Source will be passed here. If rule finds an issue in this source,
-  # it reports an error: 
-  # 
-  #   source.error rule, line_number, message
+  # Source will be passed here. If rule detects an issue in the source,
+  # it reports an error:
+  #
+  #   source.error rule, location, message
   #
   def test(source)
-    # This line deletegates verification to a particular callback in the AST visitor.
-    AST::Visitor.new self, source
-  end
-
-  # This method is called once the visitor finds a needed node.
-  def test(source, node : Crystal::Call)
-    # It reports an error, if there is `debugger` method call
-    # without arguments and a receiver. That's it, somebody forgot
-    # to remove a debugger statement.
-    return unless node.name == "debugger" && node.args.empty? && node.obj.nil?
-
-    source.error self, node.location, "Possible forgotten debugger statement detected"
+    # TODO: test source
   end
 end
 
 ```
 
+As soon as a custom rule is defined, it becomes available in a full set of rules
+executed by default and also can be configured via config file:
+
+```yaml
+MySuperRule:
+  Enabled: false
+```
+
 ## Credits & inspirations
 
 - [Crystal Language](crystal-lang.org)

config/ameba.yml

@@ -1,6 +1,6 @@
 ComparisonToBoolean:
   # Disallows comparison to booleans in conditions.
-  Enabled: true
+  Enabled: false
 
 ConstantNames:
   # Enforces constant names to be in a screaming case.
@@ -33,7 +33,7 @@ LargeNumbers:
 
 LineLength:
   # Disallows lines longer that MaxLength number of symbols.
-  Enabled: true
+  Enabled: false
   MaxLength: 80
 
 LiteralInCondition:

spec/ameba/base_spec.cr

@@ -0,0 +1,28 @@
+require "../spec_helper"
+
+module Ameba::Rule
+  struct NoProperties < Rule::Base
+    def test(source)
+    end
+  end
+
+  describe Base do
+    context "properties" do
+      subject = DummyRule.new
+
+      it "is enabled by default" do
+        subject.enabled.should be_true
+      end
+
+      it "has a description property" do
+        subject.description.should_not be_nil
+      end
+    end
+
+    describe "when a rule does not have defined properties" do
+      it "is enabled by default" do
+        NoProperties.new.enabled.should be_true
+      end
+    end
+  end
+end

spec/ameba/rule/duplicated_when_spec.cr

@@ -112,7 +112,7 @@ module Ameba::Rule
       error = s.errors.first
       error.rule.should_not be_nil
       error.location.to_s.should eq "source.cr:2:9"
-      error.message.should eq "Duplicated when conditions in case."
+      error.message.should eq "Duplicated when conditions in case"
     end
   end
 end

spec/ameba/rule/large_numbers_spec.cr

@@ -116,5 +116,14 @@ module Ameba
       error.location.to_s.should eq "source.cr:2:10"
       error.message.should match /1_200_000/
     end
+
+    context "properties" do
+      it "allows to configure integer min digits" do
+        s = Source.new %q(1200000)
+        rule = Rule::LargeNumbers.new
+        rule.int_min_digits = 10
+        rule.catch(s).should be_valid
+      end
+    end
   end
 end

spec/ameba/rule/line_length_spec.cr

@@ -29,5 +29,14 @@ module Ameba::Rule
       error.location.to_s.should eq "source.cr:1:81"
       error.message.should eq "Line too long"
     end
+
+    context "properties" do
+      it "allows to configure max length of the line" do
+        source = Source.new long_line
+        rule = LineLength.new
+        rule.max_length = long_line.size
+        rule.catch(source).should be_valid
+      end
+    end
   end
 end

spec/ameba/rule/percent_arrays_spec.cr

@@ -65,5 +65,21 @@ module Ameba::Rule
         "Symbols `,\"` may be unwanted in %w array literals"
       )
     end
+
+    context "properties" do
+      it "allows to configure string_array_unwanted_symbols" do
+        rule = PercentArrays.new
+        rule.string_array_unwanted_symbols = ","
+        s = Source.new %( %w("one") )
+        rule.catch(s).should be_valid
+      end
+
+      it "allows to configure symbol_array_unwanted_symbols" do
+        rule = PercentArrays.new
+        rule.symbol_array_unwanted_symbols = ","
+        s = Source.new %( %i(:one) )
+        rule.catch(s).should be_valid
+      end
+    end
   end
 end

spec/ameba/rule/redundant_begin_spec.cr

@@ -207,7 +207,7 @@ module Ameba::Rule
       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."
+      error.message.should eq "Redundant `begin` block detected"
     end
   end
 end

spec/spec_helper.cr

@@ -3,6 +3,10 @@ require "../src/ameba"
 
 module Ameba
   struct DummyRule < Rule::Base
+    properties do
+      description : String = "Dummy rule that does nothing."
+    end
+
     def test(source)
     end
   end

src/ameba/config.cr

@@ -80,66 +80,71 @@ class Ameba::Config
     Formatter::DotFormatter.new
   end
 
-  # An entity that represents a corresponding configuration for a specific Rule.
+  # :no_doc:
   module Rule
-    # Represents a configuration of a specific Rule.
-    getter config : YAML::Any?
-
-    # A macro that defines a dsl to define configurable properties.
-    #
-    # ```
-    # class Configurable
-    #   include Ameba::Config::Rule
-    #
-    #   prop enabled? = false
-    #   prop max_length = 80
-    #   prop wildcard = "*"
-    # end
-    # ```
-    #
-    macro prop(assign)
-      # Rule configuration property.
-      def {{assign.target}}
-        {% prop_name = assign.target.id.camelcase.gsub(/\?/, "") %}
-
-        {% if assign.value.is_a? NumberLiteral %}
-          int_prop "{{prop_name}}", {{assign.value}}
-        {% elsif assign.value.is_a? BoolLiteral %}
-          bool_prop "{{prop_name}}", {{assign.value}}
-        {% elsif assign.value.is_a? StringLiteral %}
-          str_prop "{{prop_name}}", {{assign.value}}
+    macro properties(&block)
+      {% definitions = [] of NamedTuple %}
+      {% if block.body.is_a? Assign %}
+        {% definitions << {var: block.body.target, value: block.body.value} %}
+      {% elsif block.body.is_a? TypeDeclaration %}
+        {% definitions << {var: block.body.var, value: block.body.value, type: block.body.type} %}
+      {% elsif block.body.is_a? Expressions %}
+        {% for prop in block.body.expressions %}
+          {% if prop.is_a? Assign %}
+            {% definitions << {var: prop.target, value: prop.value} %}
+          {% elsif prop.is_a? TypeDeclaration %}
+            {% definitions << {var: prop.var, value: prop.value, type: prop.type} %}
+          {% end %}
         {% end %}
+      {% end %}
+
+      {% properties = {} of MacroId => NamedTuple %}
+      {% for df in definitions %}
+        {% name = df[:var].id %}
+        {% key = name.camelcase.stringify %}
+        {% value = df[:value] %}
+        {% type = df[:type] %}
+
+        {% if type == nil %}
+          {% if value.is_a? BoolLiteral %}
+            {% type = Bool %}
+          {% elsif value.is_a? StringLiteral %}
+            {% type = String %}
+          {% elsif value.is_a? NumberLiteral %}
+            {% if value.kind == :i32 %}
+              {% type = Int32 %}
+            {% elsif value.kind == :i64 %}
+              {% type = Int64 %}
+            {% elsif value.kind == :f32 %}
+              {% type = Float32 %}
+            {% elsif value.kind == :f64 %}
+              {% type = Float64 %}
+            {% end %}
+          {% end %}
+
+          {% type = Nil if type == nil %}
+        {% end %}
+
+        {% properties[name] = {key: key, default: value, type: type} %}
+      {% end %}
+
+      {% if properties["enabled".id] == nil %}
+        {% properties["enabled".id] = {key: "Enabled", default: true, type: Bool} %}
+      {% end %}
+
+      YAML.mapping({{properties}})
+    end
+
+    macro included
+      macro inherited
+        # allow creating rules without properties
+        properties {}
+
+        def self.new(config : Ameba::Config? = nil)
+          yaml = config.try &.subconfig(class_name).try &.to_yaml || "{}"
+          from_yaml yaml
+        end
       end
     end
-
-    # Creates an instance of a Rule configuration.
-    #
-    # ```
-    # class Configurable
-    #   include Ameba::Config::Rule
-    #
-    #   prop enabled? = false
-    #   prop max_length = 80
-    #   prop wildcard = "*"
-    # end
-    #
-    # Configurable.new config
-    # ```
-    #
-    def initialize(config = nil)
-      @config = config.try &.subconfig(name)
-    end
-
-    protected def int_prop(name, default : Number)
-      str_prop(name, default).to_i
-    end
-
-    protected def bool_prop(name, default : Bool)
-      str_prop(name, default.to_s) == "true"
-    end
-
-    protected def str_prop(name, default)
-      config.try &.[name]?.try &.as_s || default
-    end
   end
 end

src/ameba/rule/base.cr

@@ -27,10 +27,6 @@ module Ameba::Rule
     # that are tested by this rule, it should add an error.
     abstract def test(source : Source)
 
-    # Enabled property indicates whether this rule enabled or not.
-    # Only enabled rules will be included into the inspection.
-    prop enabled? = true
-
     def test(source : Source, node : Crystal::ASTNode)
       # can't be abstract
     end
@@ -59,7 +55,11 @@ module Ameba::Rule
     # ```
     #
     def name
-      self.class.name.gsub("Ameba::Rule::", "")
+      {{@type}}.class_name
+    end
+
+    protected def self.class_name
+      name.gsub("Ameba::Rule::", "")
     end
 
     protected def self.subclasses

src/ameba/rule/comparison_to_boolean.cr

@@ -21,6 +21,11 @@ module Ameba::Rule
   # ```
   #
   struct ComparisonToBoolean < Base
+    properties do
+      enabled = false
+      description = "Disallows comparison to booleans"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/constant_names.cr

@@ -23,6 +23,10 @@ module Ameba::Rule
   # ```
   #
   struct ConstantNames < Base
+    properties do
+      description = "Enforces constant names to be in screaming case"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/debugger_statement.cr

@@ -12,6 +12,10 @@ module Ameba::Rule
   # ```
   #
   struct DebuggerStatement < Base
+    properties do
+      description = "Disallows calls to debugger"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/duplicated_when.cr

@@ -31,6 +31,10 @@ module Ameba::Rule
   # ```
   #
   struct DuplicatedWhen < Base
+    properties do
+      description = "Disallows duplicated when conditions in case"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end
@@ -38,7 +42,7 @@ module Ameba::Rule
     def test(source, node : Crystal::Case)
       return unless duplicated_whens?(node.whens)
 
-      source.error self, node.location, "Duplicated when conditions in case."
+      source.error self, node.location, "Duplicated when conditions in case"
     end
 
     private def duplicated_whens?(whens)

src/ameba/rule/empty_ensure.cr

@@ -33,6 +33,10 @@ module Ameba::Rule
   # ```
   #
   struct EmptyEnsure < Base
+    properties do
+      description = "Disallows empty ensure statement"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/empty_expression.cr

@@ -31,6 +31,10 @@ module Ameba::Rule
   struct EmptyExpression < Base
     include AST::Util
 
+    properties do
+      description = "Disallows empty expressions"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/hash_duplicated_key.cr

@@ -21,6 +21,10 @@ module Ameba::Rule
   # ```
   #
   struct HashDuplicatedKey < Base
+    properties do
+      description = "Disallows duplicated keys in hash literals"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/large_numbers.cr

@@ -28,7 +28,10 @@ module Ameba::Rule
   # ```
   #
   struct LargeNumbers < Base
-    prop int_min_digits = 5
+    properties do
+      description = "Disallows usage of large numbers without underscore"
+      int_min_digits = 5
+    end
 
     def test(source)
       Tokenizer.new(source).run do |token|

src/ameba/rule/line_length.cr

@@ -10,7 +10,11 @@ module Ameba::Rule
   # ```
   #
   struct LineLength < Base
-    prop max_length = 80
+    properties do
+      enabled = false
+      description = "Disallows lines longer than `MaxLength` number of symbols"
+      max_length = 80
+    end
 
     def test(source)
       source.lines.each_with_index do |line, index|

src/ameba/rule/literal_in_condition.cr

@@ -23,6 +23,11 @@ module Ameba::Rule
   struct LiteralInCondition < Base
     include AST::Util
 
+    properties do
+      description = "Disallows useless conditional statements that contain\
+        a literal in place of a variable or predicate function"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/literal_in_interpolation.cr

@@ -19,6 +19,10 @@ module Ameba::Rule
   struct LiteralInInterpolation < Base
     include AST::Util
 
+    properties do
+      description = "Disallows useless string interpolations"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/method_names.cr

@@ -39,6 +39,10 @@ module Ameba::Rule
   # ```
   #
   struct MethodNames < Base
+    properties do
+      description = "Enforces method names to be in underscored case"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/negated_conditions_in_unless.cr

@@ -28,6 +28,10 @@ module Ameba::Rule
   # ```
   #
   struct NegatedConditionsInUnless < Base
+    properties do
+      description = "Disallows negated conditions in unless"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/percent_arrays.cr

@@ -25,8 +25,11 @@ module Ameba::Rule
   # ```
   #
   struct PercentArrays < Base
-    prop string_array_unwanted_symbols = ",\""
-    prop symbol_array_unwanted_symbols = ",:"
+    properties do
+      description = "Disallows some unwanted symbols in percent array literals"
+      string_array_unwanted_symbols = ",\""
+      symbol_array_unwanted_symbols = ",:"
+    end
 
     def test(source)
       error = start_token = nil

src/ameba/rule/predicate_name.cr

@@ -30,6 +30,10 @@ module Ameba::Rule
   # ```
   #
   struct PredicateName < Base
+    properties do
+      description = "Disallows tautological predicate names"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/redundant_begin.cr

@@ -57,6 +57,9 @@ module Ameba::Rule
   #
   struct RedundantBegin < Base
     include AST::Util
+    properties do
+      description = "Disallows redundant begin blocks"
+    end
 
     def test(source)
       AST::Visitor.new self, source
@@ -65,7 +68,7 @@ module Ameba::Rule
     def test(source, node : Crystal::Def)
       return unless redundant_begin?(source, node)
 
-      source.error self, node.location, "Redundant `begin` block detected."
+      source.error self, node.location, "Redundant `begin` block detected"
     end
 
     private def redundant_begin?(source, node)

src/ameba/rule/trailing_blank_lines.cr

@@ -9,6 +9,10 @@ module Ameba::Rule
   # ```
   #
   struct TrailingBlankLines < Base
+    properties do
+      description = "Disallows trailing blank lines"
+    end
+
     def test(source)
       if source.lines.size > 1 && source.lines[-2, 2].join.strip.empty?
         source.error self, source.location(source.lines.size, 1),

src/ameba/rule/trailing_whitespace.cr

@@ -9,6 +9,10 @@ module Ameba::Rule
   # ```
   #
   struct TrailingWhitespace < Base
+    properties do
+      description = "Disallows trailing whitespaces"
+    end
+
     def test(source)
       source.lines.each_with_index do |line, index|
         next unless line =~ /\s$/

src/ameba/rule/type_names.cr

@@ -53,6 +53,10 @@ module Ameba::Rule
   # ```
   #
   struct TypeNames < Base
+    properties do
+      description = "Enforces type names in camelcase manner"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/unless_else.cr

@@ -44,6 +44,10 @@ module Ameba::Rule
   # ```
   #
   struct UnlessElse < Base
+    properties do
+      description = "Disallows the use of an `else` block with the `unless`"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/rule/variable_names.cr

@@ -32,6 +32,10 @@ module Ameba::Rule
   # ```
   #
   struct VariableNames < Base
+    properties do
+      description = "Enforces variable names to be in underscored case"
+    end
+
     def test(source)
       AST::Visitor.new self, source
     end

src/ameba/runner.cr

@@ -93,7 +93,7 @@ module Ameba
     end
 
     private def load_rules(config)
-      Rule.rules.map { |r| r.new config }.select &.enabled?
+      Rule.rules.map { |r| r.new config }.select &.enabled
     end
   end
 end