Few refactors

spec/ameba/ast/scope_spec.cr

@@ -57,13 +57,15 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
-      variable.reference nodes.var_nodes.first, scope.inner_scopes.first
+      variable.reference(nodes.var_nodes.first, scope.inner_scopes.first)
 
       scope.references?(variable).should be_true
     end
@@ -77,13 +79,15 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
-      variable.reference nodes.var_nodes.first, scope.inner_scopes.first
+      variable.reference(nodes.var_nodes.first, scope.inner_scopes.first)
 
       scope.references?(variable, check_inner_scopes: false).should be_false
     end
@@ -98,9 +102,11 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
@@ -120,7 +126,7 @@ module Ameba::AST
   describe "#find_variable" do
     it "returns the variable in the scope by name" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.find_variable("foo").should_not be_nil
     end
 
@@ -133,7 +139,7 @@ module Ameba::AST
   describe "#assign_variable" do
     it "creates a new assignment" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.assign_variable("foo", Crystal::Var.new "foo")
       var = scope.find_variable("foo").should_not be_nil
       var.assignments.size.should eq 1
@@ -141,7 +147,7 @@ module Ameba::AST
 
     it "does not create the assignment if variable is wrong" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.assign_variable("bar", Crystal::Var.new "bar")
       var = scope.find_variable("foo").should_not be_nil
       var.assignments.size.should eq 0

spec/ameba/ast/variabling/variable_spec.cr

@@ -85,13 +85,16 @@ module Ameba::AST
             3.times { |i| a = a + i }
           end
           CRYSTAL
-        scope = Scope.new nodes.def_nodes.first
+
         var_node = nodes.var_nodes.first
-        scope.add_variable var_node
+
+        scope = Scope.new(nodes.def_nodes.first)
+        scope.add_variable(var_node)
         scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
         variable = Variable.new(var_node, scope)
-        variable.reference nodes.var_nodes.last, scope.inner_scopes.last
+        variable.reference(nodes.var_nodes.last, scope.inner_scopes.last)
+
         variable.captured_by_block?.should be_truthy
       end
 
@@ -101,8 +104,10 @@ module Ameba::AST
             a = 1
           end
           CRYSTAL
-        scope.add_variable Crystal::Var.new "a"
+
+        scope.add_variable(Crystal::Var.new "a")
         variable = scope.variables.first
+
         variable.captured_by_block?.should be_falsey
       end
     end

spec/ameba/config_spec.cr

@@ -85,7 +85,7 @@ module Ameba
       end
 
       it "raises when custom config file doesn't exist" do
-        expect_raises(Exception, "Unable to load config file: Config file does not exist foo.yml") do
+        expect_raises(Exception, "Unable to load config file: Config file does not exist") do
           Config.load "foo.yml"
         end
       end

spec/ameba/rule/lint/literals_comparison_spec.cr

@@ -10,6 +10,8 @@ module Ameba::Rule::Lint
         ["foo"] === [foo]
         "foo" == foo
         "foo" != foo
+        "foo" == FOO
+        FOO == "foo"
         foo == "foo"
         foo != "foo"
         CRYSTAL

src/ameba/ast/variabling/variable.cr

@@ -52,7 +52,7 @@ module Ameba::AST
     #
     # ```
     # variable = Variable.new(node, scope)
-    # variable.reference(var_node)
+    # variable.reference(var_node, some_scope)
     # variable.referenced? # => true
     # ```
     def referenced?
@@ -206,9 +206,9 @@ module Ameba::AST
       return if references.size > assignments.size
       return if assignments.any?(&.op_assign?)
 
-      @assign_before_reference = assignments.find { |ass|
+      @assign_before_reference = assignments
-        !ass.in_branch?
+        .find(&.in_branch?.!)
-      }.try &.node
+        .try(&.node)
     end
   end
 end

src/ameba/ast/visitors/scope_visitor.cr

@@ -153,7 +153,7 @@ module Ameba::AST
 
     # :nodoc:
     def visit(node : Crystal::Var)
-      variable = @current_scope.find_variable node.name
+      variable = @current_scope.find_variable(node.name)
 
       case
       when @current_scope.arg?(node) # node is an argument
@@ -161,7 +161,7 @@ module Ameba::AST
       when variable.nil? && @current_assign # node is a variable
         @current_scope.add_variable(node)
       when variable # node is a reference
-        reference = variable.reference node, @current_scope
+        reference = variable.reference(node, @current_scope)
         if @current_assign.is_a?(Crystal::OpAssign) || !reference.target_of?(@current_assign)
           variable.reference_assignments!
         end

src/ameba/config.cr

@@ -97,8 +97,9 @@ class Ameba::Config
     @excluded = load_array_section(config, "Excluded")
     @globs = load_array_section(config, "Globs", DEFAULT_GLOBS)
 
-    return unless formatter_name = load_formatter_name(config)
+    if formatter_name = load_formatter_name(config)
-    self.formatter = formatter_name
+      self.formatter = formatter_name
+    end
   end
 
   # Loads YAML configuration file by `path`.
@@ -120,8 +121,8 @@ class Ameba::Config
 
   protected def self.read_config(path = nil)
     if path
-      raise ArgumentError.new("Config file does not exist #{path}") unless File.exists?(path)
+      return File.read(path) if File.exists?(path)
-      return File.read(path)
+      raise "Config file does not exist"
     end
     each_config_path do |config_path|
       return File.read(config_path) if File.exists?(config_path)

src/ameba/formatter/todo_formatter.cr

@@ -26,16 +26,21 @@ module Ameba::Formatter
     end
 
     private def generate_todo_config(issues)
-      file = File.new(@config_path, mode: "w")
+      File.open(@config_path, mode: "w") do |file|
-      file << header
+        file << header
-      rule_issues_map(issues).each do |rule, rule_issues|
+
-        file << "\n# Problems found: #{rule_issues.size}"
+        rule_issues_map(issues).each do |rule, rule_issues|
-        file << "\n# Run `ameba --only #{rule.name}` for details"
+          rule_todo = rule_todo(rule, rule_issues)
-        file << rule_todo(rule, rule_issues).gsub("---", "")
+          rule_todo =
+            {rule_todo.name => rule_todo}
+              .to_yaml.gsub("---", "")
+
+          file << "\n# Problems found: #{rule_issues.size}"
+          file << "\n# Run `ameba --only #{rule.name}` for details"
+          file << rule_todo
+        end
+        file
       end
-      file
-    ensure
-      file.close if file
     end
 
     private def rule_issues_map(issues)
@@ -60,11 +65,11 @@ module Ameba::Formatter
     end
 
     private def rule_todo(rule, issues)
-      rule.excluded = issues
+      rule.dup.tap do |rule_todo|
-        .compact_map(&.location.try &.filename.try &.to_s)
+        rule_todo.excluded = issues
-        .uniq!
+          .compact_map(&.location.try &.filename.try &.to_s)
-
+          .uniq!
-      {rule.name => rule}.to_yaml
+      end
     end
   end
 end

src/ameba/rule/style/verbose_block.cr

@@ -110,23 +110,22 @@ module Ameba::Rule::Style
       i
     end
 
-    protected def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false)
+    protected def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false) : Nil
-      node.args.dup.tap do |args|
+      args = node.args.dup
-        args.pop? if skip_last_arg
+      args.pop? if skip_last_arg
-        args.join io, ", "
+      args.join io, ", "
 
-        named_args = node.named_args
+      named_args = node.named_args
-        if named_args
+      if named_args
-          io << ", " unless args.empty? || named_args.empty?
+        io << ", " unless args.empty? || named_args.empty?
-          named_args.join io, ", " do |arg, inner_io|
+        named_args.join io, ", " do |arg, inner_io|
-            inner_io << arg.name << ": " << arg.value
+          inner_io << arg.name << ": " << arg.value
-          end
         end
+      end
 
-        if short_block
+      if short_block
-          io << ", " unless args.empty? && (named_args.nil? || named_args.empty?)
+        io << ", " unless args.empty? && (named_args.nil? || named_args.empty?)
-          io << short_block
+        io << short_block
-        end
       end
     end
 
@@ -164,9 +163,7 @@ module Ameba::Rule::Style
       return unless block_end_location = block.body.end_location
 
       block_code = source_between(block_location, block_end_location, source.lines)
-      return unless block_code.try(&.starts_with?("&."))
+      block_code if block_code.try(&.starts_with?("&."))
-
-      block_code
     end
 
     protected def call_code(source, call, body)