ExceptionHandler implicit return fixes (#549)

Last line of `ensure` isn't returned, and last line of body isn't returned if there's an `else`
crystal-ameba · Jan 26, 2025 · e61f56f · e61f56f
1 parent 728d0e9
commit e61f56f
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 28 deletions.
diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr
@@ -245,32 +245,49 @@ module Ameba::Rule::Lint
 
     it "fails if unused literals in rescue/ensure/else block" do
       expect_issue subject, <<-CRYSTAL
-      a = begin
-            1234
-          # ^^^^ error: Literal value is not used
-            1234_f32
-          rescue ASDF
-            "hello world"
-          # ^^^^^^^^^^^^^ error: Literal value is not used
-            "interp \#{string}"
-          rescue QWERTY
-            [1, 2, 3, 4, 5]
-          # ^^^^^^^^^^^^^^^ error: Literal value is not used
-            {"hello" => "world"}
-          else
-            '\t'
-          # ^^^ error: Literal value is not used
-            <<-HEREDOC
-          # ^^^^^^^^^^ error: Literal value is not used
-              this is a heredoc
-              HEREDOC
-            1..2
-          ensure
-            {goodnight: moon}
-          # ^^^^^^^^^^^^^^^^^ error: Literal value is not used
-            {1, 2, 3}
-          end
-      CRYSTAL
+        a = begin
+              1234
+            # ^^^^ error: Literal value is not used
+              1234_f32
+            # ^^^^^^^^ error: Literal value is not used
+            rescue ASDF
+              "hello world"
+            # ^^^^^^^^^^^^^ error: Literal value is not used
+              "interp \#{string}"
+            rescue QWERTY
+              [1, 2, 3, 4, 5]
+            # ^^^^^^^^^^^^^^^ error: Literal value is not used
+              {"hello" => "world"}
+            else
+              '\t'
+            # ^^^ error: Literal value is not used
+              <<-HEREDOC
+            # ^^^^^^^^^^ error: Literal value is not used
+                this is a heredoc
+                HEREDOC
+              1..2
+            ensure
+              {goodnight: moon}
+            # ^^^^^^^^^^^^^^^^^ error: Literal value is not used
+              {1, 2, 3}
+            # ^^^^^^^^^ error: Literal value is not used
+            end
+
+        b = begin
+              1234
+            # ^^^^ error: Literal value is not used
+              1234_f32
+            rescue ASDF
+              "hello world"
+            # ^^^^^^^^^^^^^ error: Literal value is not used
+              "interp \#{string}"
+            ensure
+              {goodnight: moon}
+            # ^^^^^^^^^^^^^^^^^ error: Literal value is not used
+              {1, 2, 3}
+            # ^^^^^^^^^ error: Literal value is not used
+            end
+        CRYSTAL
     end
 
     # Locations for Regex literals were added in Crystal v1.15.0

diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr
@@ -225,10 +225,18 @@ module Ameba::AST
     def visit(node : Crystal::ExceptionHandler) : Bool
       @rule.test(@source, node, @stack > 0)
 
-      node.body.accept(self)
+      if node.else
+        # Last line of body isn't implicitly returned if there's an else
+        swap_stack { node.body.try &.accept(self) }
+      else
+        node.body.accept(self)
+      end
+
       node.rescues.try &.each &.accept(self)
       node.else.try &.accept(self)
-      node.ensure.try &.accept(self)
+
+      # Last line of ensure isn't implicitly returned
+      swap_stack { node.ensure.try &.accept(self) }
 
       false
     end