Performance improvements (#15)

* Performance improvements

Two main changes:

1. Cache parsed AST in a Source. So Parser will parse content only once.
2. Use one unified visitor with multiple callbacks instead of multiple
visitors to traverse AST.

This improves performance significantly, for example running it on
Crystal repository takes ~1 second, which 6 times faster than before.

* Change readme example

README.md

@@ -67,22 +67,21 @@ your logic to detect a problem:
 ```crystal
 struct DebuggerStatement < Rule
   # This is a required method to be implemented by the rule.
-  # Source will pass here. If rule finds an issue in this source,
+  # Source will be passed here. If rule finds an issue in this source,
-  # it adds an error: 
+  # it reports an error: 
   # 
   #   source.error rule, line_number, message
   #
   def test(source)
     # This line deletegates verification to a particular AST visitor.
-    AST::CallVisitor.new self, source
+    AST::Visitor.new self, source
   end
 
-  # This method is called once our visitor finds a required node.
+  # This method is called once the visitor finds a required node.
-  # 
-  # It reports an error, once there is `debugger` method call
-  # without arguments and a receiver. That's it, somebody forgot
-  # to remove debugger statement.
   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.try &.line_number,

spec/ameba/ast/traverse_spec.cr

@@ -8,7 +8,7 @@ module Ameba::AST
     {% for name in NODE_VISITORS %}
       describe "{{name}}" do
         it "allow to visit {{name}} node" do
-          visitor = {{name}}Visitor.new rule, source
+          visitor = Visitor.new rule, source
           nodes = Crystal::Parser.new("").parse
           nodes.accept visitor
         end

src/ameba/ast/traverse.cr

@@ -19,26 +19,23 @@ module Ameba::AST
     Var,
   ]
 
-  abstract class Visitor < Crystal::Visitor
+  class Visitor < Crystal::Visitor
     @rule : Rule
     @source : Source
 
     def initialize(@rule, @source)
-      parser = Crystal::Parser.new(@source.content)
+      @source.ast.accept self
-      parser.filename = @source.path
-      parser.parse.accept self
     end
 
     def visit(node : Crystal::ASTNode)
       true
     end
-  end
 
-  {% for name in NODE_VISITORS %}
+    {% for name in NODE_VISITORS %}
-    class {{name}}Visitor < Visitor
       def visit(node : Crystal::{{name}})
         @rule.test @source, node
+        true
       end
-    end
+    {% end %}
-  {% end %}
+  end
 end

src/ameba/rule.cr

@@ -3,7 +3,7 @@ module Ameba
     abstract def test(source : Source)
 
     def test(source : Source, node : Crystal::ASTNode)
-      raise "Unimplemented"
+      # can't be abstract
     end
 
     def catch(source : Source)

src/ameba/rules/comparison_to_boolean.cr

@@ -14,7 +14,7 @@ module Ameba::Rules
   #
   struct ComparisonToBoolean < Rule
     def test(source)
-      AST::CallVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Call)

src/ameba/rules/constant_names.cr

@@ -17,7 +17,7 @@ module Ameba::Rules
   #
   struct ConstantNames < Rule
     def test(source)
-      AST::AssignVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Assign)

src/ameba/rules/debugger_statement.cr

@@ -6,7 +6,7 @@ module Ameba::Rules
   #
   struct DebuggerStatement < Rule
     def test(source)
-      AST::CallVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Call)

src/ameba/rules/literal_in_condition.cr

@@ -17,9 +17,7 @@ module Ameba::Rules
     include AST::Util
 
     def test(source)
-      AST::IfVisitor.new self, source
+      AST::Visitor.new self, source
-      AST::UnlessVisitor.new self, source
-      AST::CaseVisitor.new self, source
     end
 
     def check_node(source, node)

src/ameba/rules/literal_in_interpolation.cr

@@ -13,7 +13,7 @@ module Ameba::Rules
     include AST::Util
 
     def test(source)
-      AST::StringInterpolationVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::StringInterpolation)

src/ameba/rules/method_names.cr

@@ -32,7 +32,7 @@ module Ameba::Rules
   # ```
   struct MethodNames < Rule
     def test(source)
-      AST::DefVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Def)

src/ameba/rules/negated_conditions_in_unless.cr

@@ -22,7 +22,7 @@ module Ameba::Rules
   #
   struct NegatedConditionsInUnless < Rule
     def test(source)
-      AST::UnlessVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Unless)

src/ameba/rules/predicate_name.cr

@@ -24,7 +24,7 @@ module Ameba::Rules
   #
   struct PredicateName < Rule
     def test(source)
-      AST::DefVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Def)

src/ameba/rules/type_names.cr

@@ -47,13 +47,7 @@ module Ameba::Rules
   #
   struct TypeNames < Rule
     def test(source)
-      [
+      AST::Visitor.new self, source
-        AST::ClassDefVisitor,
-        AST::EnumDefVisitor,
-        AST::ModuleDefVisitor,
-        AST::AliasVisitor,
-        AST::LibDefVisitor,
-      ].each(&.new self, source)
     end
 
     private def check_node(source, node)

src/ameba/rules/unless_else.cr

@@ -38,7 +38,7 @@ module Ameba::Rules
   #
   struct UnlessElse < Rule
     def test(source)
-      AST::UnlessVisitor.new self, source
+      AST::Visitor.new self, source
     end
 
     def test(source, node : Crystal::Unless)

src/ameba/rules/variable_names.cr

@@ -26,11 +26,7 @@ module Ameba::Rules
   #
   struct VariableNames < Rule
     def test(source)
-      [
+      AST::Visitor.new self, source
-        AST::VarVisitor,
-        AST::InstanceVarVisitor,
-        AST::ClassVarVisitor,
-      ].each &.new(self, source)
     end
 
     private def check_node(source, node)

src/ameba/source.cr

@@ -15,6 +15,7 @@ module Ameba
     getter errors = [] of Error
     getter path : String?
     getter content : String
+    getter ast : Crystal::ASTNode?
 
     def initialize(@content : String, @path = nil)
     end
@@ -30,5 +31,12 @@ module Ameba
     def lines
       @lines ||= @content.split("\n")
     end
+
+    def ast
+      @ast ||=
+        Crystal::Parser.new(content)
+                       .tap { |parser| parser.filename = @path }
+                       .parse
+    end
   end
 end