From 65ab317a3bb716cc9973fd5ea3e8e01254b9a61d Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Wed, 27 Dec 2023 19:28:56 +0100
Subject: [PATCH 1/5] Merge `delegate` calls

---
 src/ameba/ast/scope.cr                        | 5 ++---
 src/ameba/ast/variabling/argument.cr          | 5 ++---
 src/ameba/ast/variabling/assignment.cr        | 5 ++---
 src/ameba/ast/variabling/ivariable.cr         | 6 ++----
 src/ameba/ast/variabling/type_dec_variable.cr | 5 ++---
 src/ameba/ast/variabling/variable.cr          | 6 ++----
 6 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr
index 97b71483b..3e0b4573b 100644
--- a/src/ameba/ast/scope.cr
+++ b/src/ameba/ast/scope.cr
@@ -34,9 +34,8 @@ module Ameba::AST
     # The actual AST node that represents a current scope.
     getter node : Crystal::ASTNode
 
-    delegate to_s, to: node
-    delegate location, to: node
-    delegate end_location, to: node
+    delegate location, end_location, to_s,
+      to: @node
 
     def_equals_and_hash node, location
 
diff --git a/src/ameba/ast/variabling/argument.cr b/src/ameba/ast/variabling/argument.cr
index 59b546618..d080e9a17 100644
--- a/src/ameba/ast/variabling/argument.cr
+++ b/src/ameba/ast/variabling/argument.cr
@@ -19,9 +19,8 @@ module Ameba::AST
     # Variable of this argument (may be the same node)
     getter variable : Variable
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     # Creates a new argument.
     #
diff --git a/src/ameba/ast/variabling/assignment.cr b/src/ameba/ast/variabling/assignment.cr
index 7168f5cba..ece19b804 100644
--- a/src/ameba/ast/variabling/assignment.cr
+++ b/src/ameba/ast/variabling/assignment.cr
@@ -19,9 +19,8 @@ module Ameba::AST
     # A scope assignment belongs to
     getter scope : Scope
 
-    delegate to_s, to: @node
-    delegate location, to: @node
-    delegate end_location, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     # Creates a new assignment.
     #
diff --git a/src/ameba/ast/variabling/ivariable.cr b/src/ameba/ast/variabling/ivariable.cr
index 836ad345f..db6f3bd36 100644
--- a/src/ameba/ast/variabling/ivariable.cr
+++ b/src/ameba/ast/variabling/ivariable.cr
@@ -2,10 +2,8 @@ module Ameba::AST
   class InstanceVariable
     getter node : Crystal::InstanceVar
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate name, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, name, to_s,
+      to: @node
 
     def initialize(@node)
     end
diff --git a/src/ameba/ast/variabling/type_dec_variable.cr b/src/ameba/ast/variabling/type_dec_variable.cr
index 3376b37fa..74cc3fbba 100644
--- a/src/ameba/ast/variabling/type_dec_variable.cr
+++ b/src/ameba/ast/variabling/type_dec_variable.cr
@@ -2,9 +2,8 @@ module Ameba::AST
   class TypeDecVariable
     getter node : Crystal::TypeDeclaration
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     def initialize(@node)
     end
diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr
index c2eee962b..615b588eb 100644
--- a/src/ameba/ast/variabling/variable.cr
+++ b/src/ameba/ast/variabling/variable.cr
@@ -17,10 +17,8 @@ module Ameba::AST
     # Node of the first assignment which can be available before any reference.
     getter assign_before_reference : Crystal::ASTNode?
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate name, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, name, to_s,
+      to: @node
 
     # Creates a new variable(in the scope).
     #

From 6d0b12c70fb10f586aebc3863247475a1d6aa386 Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Wed, 27 Dec 2023 19:38:44 +0100
Subject: [PATCH 2/5] Cleanup docs

---
 src/ameba/formatter/base_formatter.cr         | 6 +++---
 src/ameba/rule/base.cr                        | 5 +++--
 src/ameba/rule/naming/block_parameter_name.cr | 2 +-
 src/ameba/rule/style/verbose_block.cr         | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/ameba/formatter/base_formatter.cr b/src/ameba/formatter/base_formatter.cr
index 4009c9921..a6ceafdf0 100644
--- a/src/ameba/formatter/base_formatter.cr
+++ b/src/ameba/formatter/base_formatter.cr
@@ -17,13 +17,13 @@ module Ameba::Formatter
     # A list of sources to inspect is passed as an argument.
     def started(sources); end
 
-    # Callback that indicates when source inspection is finished.
+    # Callback that indicates when source inspection is started.
     # A corresponding source is passed as an argument.
-    def source_finished(source : Source); end
+    def source_started(source : Source); end
 
     # Callback that indicates when source inspection is finished.
     # A corresponding source is passed as an argument.
-    def source_started(source : Source); end
+    def source_finished(source : Source); end
 
     # Callback that indicates when inspection is finished.
     # A list of inspected sources is passed as an argument.
diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr
index f28c83d0e..242c86f0b 100644
--- a/src/ameba/rule/base.cr
+++ b/src/ameba/rule/base.cr
@@ -32,14 +32,15 @@ module Ameba::Rule
     # This method is designed to test the source passed in. If source has issues
     # that are tested by this rule, it should add an issue.
     #
-    # Be default it uses a node visitor to traverse all the nodes in the source.
+    # By default it uses a node visitor to traverse all the nodes in the source.
+    #
     # NOTE: Must be overridden for other type of rules.
     def test(source : Source)
       AST::NodeVisitor.new self, source
     end
 
+    # NOTE: Can't be abstract
     def test(source : Source, node : Crystal::ASTNode, *opts)
-      # can't be abstract
     end
 
     # A convenient addition to `#test` method that does the same
diff --git a/src/ameba/rule/naming/block_parameter_name.cr b/src/ameba/rule/naming/block_parameter_name.cr
index 2311b8900..71e6a6a87 100644
--- a/src/ameba/rule/naming/block_parameter_name.cr
+++ b/src/ameba/rule/naming/block_parameter_name.cr
@@ -41,7 +41,7 @@ module Ameba::Rule::Naming
     end
 
     private def valid_name?(name)
-      return true if name.blank? # happens with compound names like `(arg1, arg2)`
+      return true if name.blank? # TODO: handle unpacked variables
       return true if name.in?(allowed_names)
 
       return false if name.in?(forbidden_names)
diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr
index fb69f14fe..fee2cecd3 100644
--- a/src/ameba/rule/style/verbose_block.cr
+++ b/src/ameba/rule/style/verbose_block.cr
@@ -230,7 +230,7 @@ module Ameba::Rule::Style
       # we filter out the blocks that are of call type - `i.to_i64.odd?`
       return unless (body = block.body).is_a?(Crystal::Call)
 
-      # we need to "unwind" the chain calls, so the final receiver object
+      # we need to "unwind" the call chain, so the final receiver object
       # ends up being a variable - `i`
       obj = body.obj
       while obj.is_a?(Crystal::Call)

From e99a69765fc7483e3dd9b0554dea9eda46c2992e Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Wed, 27 Dec 2023 19:42:50 +0100
Subject: [PATCH 3/5] Few refactors

---
 spec/ameba/ast/scope_spec.cr                  | 28 +++++++++-------
 spec/ameba/ast/variabling/variable_spec.cr    | 13 +++++---
 spec/ameba/config_spec.cr                     |  2 +-
 .../rule/lint/literals_comparison_spec.cr     |  2 ++
 src/ameba/ast/variabling/variable.cr          |  8 ++---
 src/ameba/ast/visitors/scope_visitor.cr       |  4 +--
 src/ameba/config.cr                           |  9 ++---
 src/ameba/formatter/todo_formatter.cr         | 33 +++++++++++--------
 src/ameba/rule/style/verbose_block.cr         | 33 +++++++++----------
 9 files changed, 74 insertions(+), 58 deletions(-)

diff --git a/spec/ameba/ast/scope_spec.cr b/spec/ameba/ast/scope_spec.cr
index c36ef62d0..3d8b6ba30 100644
--- a/spec/ameba/ast/scope_spec.cr
+++ b/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
diff --git a/spec/ameba/ast/variabling/variable_spec.cr b/spec/ameba/ast/variabling/variable_spec.cr
index 83792e2ef..97f72f84e 100644
--- a/spec/ameba/ast/variabling/variable_spec.cr
+++ b/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
diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr
index 4d6f95f04..678da23ba 100644
--- a/spec/ameba/config_spec.cr
+++ b/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
diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr
index ded948e66..ef75b6824 100644
--- a/spec/ameba/rule/lint/literals_comparison_spec.cr
+++ b/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
diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr
index 615b588eb..5380d8faa 100644
--- a/src/ameba/ast/variabling/variable.cr
+++ b/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|
-        !ass.in_branch?
-      }.try &.node
+      @assign_before_reference = assignments
+        .find(&.in_branch?.!)
+        .try(&.node)
     end
   end
 end
diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr
index 0b93cdb9c..a7f132dbd 100644
--- a/src/ameba/ast/visitors/scope_visitor.cr
+++ b/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
diff --git a/src/ameba/config.cr b/src/ameba/config.cr
index a8db5c669..195c70587 100644
--- a/src/ameba/config.cr
+++ b/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)
-    self.formatter = formatter_name
+    if formatter_name = load_formatter_name(config)
+      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)
+      return File.read(path) if File.exists?(path)
+      raise "Config file does not exist"
     end
     each_config_path do |config_path|
       return File.read(config_path) if File.exists?(config_path)
diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr
index 5dc797e86..00a2d28f1 100644
--- a/src/ameba/formatter/todo_formatter.cr
+++ b/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 << header
-      rule_issues_map(issues).each do |rule, rule_issues|
-        file << "\n# Problems found: #{rule_issues.size}"
-        file << "\n# Run `ameba --only #{rule.name}` for details"
-        file << rule_todo(rule, rule_issues).gsub("---", "")
+      File.open(@config_path, mode: "w") do |file|
+        file << header
+
+        rule_issues_map(issues).each do |rule, rule_issues|
+          rule_todo = rule_todo(rule, rule_issues)
+          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
-        .compact_map(&.location.try &.filename.try &.to_s)
-        .uniq!
-
-      {rule.name => rule}.to_yaml
+      rule.dup.tap do |rule_todo|
+        rule_todo.excluded = issues
+          .compact_map(&.location.try &.filename.try &.to_s)
+          .uniq!
+      end
     end
   end
 end
diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr
index fee2cecd3..fa4ae961e 100644
--- a/src/ameba/rule/style/verbose_block.cr
+++ b/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)
-      node.args.dup.tap do |args|
-        args.pop? if skip_last_arg
-        args.join io, ", "
-
-        named_args = node.named_args
-        if named_args
-          io << ", " unless args.empty? || named_args.empty?
-          named_args.join io, ", " do |arg, inner_io|
-            inner_io << arg.name << ": " << arg.value
-          end
+    protected def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false) : Nil
+      args = node.args.dup
+      args.pop? if skip_last_arg
+      args.join io, ", "
+
+      named_args = node.named_args
+      if named_args
+        io << ", " unless args.empty? || named_args.empty?
+        named_args.join io, ", " do |arg, inner_io|
+          inner_io << arg.name << ": " << arg.value
         end
+      end
 
-        if short_block
-          io << ", " unless args.empty? && (named_args.nil? || named_args.empty?)
-          io << short_block
-        end
+      if short_block
+        io << ", " unless args.empty? && (named_args.nil? || named_args.empty?)
+        io << short_block
       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
+      block_code if block_code.try(&.starts_with?("&."))
     end
 
     protected def call_code(source, call, body)

From 444b07c1796594f94da85ab727bf8972d4103530 Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Wed, 27 Dec 2023 22:51:36 +0100
Subject: [PATCH 4/5] Bump version to 1.6.1

---
 shard.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shard.yml b/shard.yml
index 5ebd6c8b5..74ce7c671 100644
--- a/shard.yml
+++ b/shard.yml
@@ -1,5 +1,5 @@
 name: ameba
-version: 1.6.0
+version: 1.6.1
 
 authors:
   - Vitalii Elenhaupt <velenhaupt@gmail.com>

From ce3f2b7e4b6f728d29bf8ff15db5ffd210ea4c6b Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Thu, 28 Dec 2023 04:01:05 +0100
Subject: [PATCH 5/5] Add QoL `Variable#reference(scope)` method

---
 src/ameba/ast/variabling/variable.cr    | 5 +++++
 src/ameba/ast/visitors/scope_visitor.cr | 4 +---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr
index 5380d8faa..8b513948c 100644
--- a/src/ameba/ast/variabling/variable.cr
+++ b/src/ameba/ast/variabling/variable.cr
@@ -72,6 +72,11 @@ module Ameba::AST
       end
     end
 
+    # :ditto:
+    def reference(scope : Scope)
+      reference(node, scope)
+    end
+
     # Reference variable's assignments.
     #
     # ```
diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr
index a7f132dbd..53ea39ebe 100644
--- a/src/ameba/ast/visitors/scope_visitor.cr
+++ b/src/ameba/ast/visitors/scope_visitor.cr
@@ -178,9 +178,7 @@ module Ameba::AST
       when scope.type_definition? && accessor_macro?(node) then return false
       when scope.def? && special_node?(node)
         scope.arguments.each do |arg|
-          variable = arg.variable
-
-          ref = variable.reference(variable.node, scope)
+          ref = arg.variable.reference(scope)
           ref.explicit = false
         end
       end