Merge pull request #563 from crystal-ameba/remove-node-is-used-argume…

…nt-from-implicit-return-visitor
crystal-ameba · Feb 12, 2025 · faee15b · faee15b
2 parents bbee018 + a841076
commit faee15b
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 49 deletions.
diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr
@@ -16,7 +16,7 @@ module Ameba::AST
     # their parents scope. Only the last line in an expressions node can be
     # captured by their parent node.
     def visit(node : Crystal::Expressions) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       last_idx = node.expressions.size - 1
 
@@ -37,7 +37,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::BinaryOp) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       if node.right.is_a?(Crystal::Call)
         incr_stack { node.left.accept(self) }
@@ -51,7 +51,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Call) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.obj.try &.accept(self)
@@ -65,31 +65,31 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Arg) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.default_value.try &.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::EnumDef) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.members.each &.accept(self)
 
       false
     end
 
     def visit(node : Crystal::Assign | Crystal::OpAssign) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.value.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::MultiAssign) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.targets.each &.accept(self)
       incr_stack { node.values.each &.accept(self) }
@@ -98,7 +98,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::If | Crystal::Unless) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.cond.accept(self) }
       node.then.accept(self)
@@ -108,7 +108,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::While | Crystal::Until) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.cond.accept(self) }
       node.body.accept(self)
@@ -117,7 +117,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Def) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.args.each &.accept(self)
@@ -141,7 +141,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Macro) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.args.each &.accept(self)
@@ -154,15 +154,15 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::ClassDef | Crystal::ModuleDef) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.body.accept(self)
 
       false
     end
 
     def visit(node : Crystal::FunDef) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.args.each &.accept(self)
@@ -173,15 +173,15 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Cast | Crystal::NilableCast) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.obj.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::Annotation) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.args.each &.accept(self)
@@ -192,23 +192,23 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::TypeDeclaration) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.value.try &.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::ArrayLiteral | Crystal::TupleLiteral) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.elements.each &.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::StringInterpolation) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.expressions.each do |exp|
         incr_stack { exp.accept(self) }
@@ -218,15 +218,15 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::HashLiteral | Crystal::NamedTupleLiteral) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.entries.each &.value.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::Case) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.cond.try &.accept(self) }
       node.whens.each &.accept(self)
@@ -236,7 +236,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Select) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.whens.each &.accept(self)
       node.else.try &.accept(self)
@@ -245,7 +245,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::When) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.conds.each &.accept(self) }
       node.body.accept(self)
@@ -254,15 +254,15 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Rescue) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.body.accept(self)
 
       false
     end
 
     def visit(node : Crystal::ExceptionHandler) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       if node.else
         # Last line of body isn't implicitly returned if there's an else
@@ -281,15 +281,15 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::ControlExpression) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.exp.try &.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::RangeLiteral) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack do
         node.from.accept(self)
@@ -300,7 +300,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::RegexLiteral) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       # Regex literals either contain string literals or string interpolations,
       # both of which are "captured" by the parent regex literal
@@ -313,21 +313,21 @@ module Ameba::AST
       node : Crystal::BoolLiteral | Crystal::CharLiteral | Crystal::NumberLiteral |
              Crystal::StringLiteral | Crystal::SymbolLiteral | Crystal::ProcLiteral,
     ) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       true
     end
 
     def visit(node : Crystal::Yield) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.exps.each &.accept(self) }
 
       false
     end
 
     def visit(node : Crystal::MacroExpression) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       if node.output?
         incr_stack { node.exp.accept(self) }
@@ -339,7 +339,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::MacroIf) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       incr_stack { node.cond.accept(self) }
       node.then.accept(self)
@@ -349,7 +349,7 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::MacroFor) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       node.body.accept(self)
 
@@ -361,25 +361,25 @@ module Ameba::AST
     end
 
     def visit(node : Crystal::Generic | Crystal::Path | Crystal::Union) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       false
     end
 
     def visit(node : Crystal::UninitializedVar) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       false
     end
 
     def visit(node : Crystal::LibDef) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       false
     end
 
     def visit(node : Crystal::Include | Crystal::Extend) : Bool
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       false
     end
@@ -393,11 +393,15 @@ module Ameba::AST
     end
 
     def visit(node)
-      @rule.test(@source, node, @stack.positive?)
+      report_implicit_return(node)
 
       true
     end
 
+    private def report_implicit_return(node) : Nil
+      @rule.test(@source, node) unless @stack.positive?
+    end
+
     # Indicates that any nodes visited within the block are captured / used.
     private def incr_stack(&) : Nil
       @stack += 1

diff --git a/src/ameba/rule/lint/unused_comparison.cr b/src/ameba/rule/lint/unused_comparison.cr
@@ -48,8 +48,8 @@ module Ameba::Rule::Lint
       AST::ImplicitReturnVisitor.new(self, source)
     end
 
-    def test(source, node : Crystal::Call, node_is_used : Bool)
-      if !node_is_used && node.name.in?(COMPARISON_OPERATORS) && node.args.size == 1
+    def test(source, node : Crystal::Call)
+      if node.name.in?(COMPARISON_OPERATORS) && node.args.size == 1
         issue_for node, MSG
       end
     end

diff --git a/src/ameba/rule/lint/unused_generic_or_union.cr b/src/ameba/rule/lint/unused_generic_or_union.cr
@@ -47,14 +47,12 @@ module Ameba::Rule::Lint
       AST::ImplicitReturnVisitor.new(self, source)
     end
 
-    def test(source, node : Crystal::Call, node_is_used : Bool)
-      return if node_is_used || !path_or_generic_union?(node)
-
-      issue_for node, MSG_UNION
+    def test(source, node : Crystal::Call)
+      issue_for node, MSG_UNION if path_or_generic_union?(node)
     end
 
-    def test(source, node : Crystal::Generic, node_is_used : Bool)
-      issue_for node, MSG_GENERIC unless node_is_used
+    def test(source, node : Crystal::Generic)
+      issue_for node, MSG_GENERIC
     end
 
     private def path_or_generic_union?(node : Crystal::Call) : Bool

diff --git a/src/ameba/rule/lint/unused_literal.cr b/src/ameba/rule/lint/unused_literal.cr
@@ -55,10 +55,10 @@ module Ameba::Rule::Lint
       AST::ImplicitReturnVisitor.new(self, source)
     end
 
-    def test(source, node : Crystal::RegexLiteral, node_is_used : Bool)
+    def test(source, node : Crystal::RegexLiteral)
       # Locations for Regex literals were added in Crystal v1.15.0
       {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %}
-        issue_for node, MSG unless node_is_used
+        issue_for node, MSG
       {% end %}
     end
 
@@ -69,9 +69,8 @@ module Ameba::Rule::Lint
              Crystal::TupleLiteral | Crystal::NumberLiteral |
              Crystal::StringLiteral | Crystal::SymbolLiteral |
              Crystal::NamedTupleLiteral | Crystal::StringInterpolation,
-      node_is_used : Bool,
     )
-      issue_for node, MSG unless node_is_used
+      issue_for node, MSG
     end
   end
 end