From a6958ab455d8e2a481fdad75dd139acc1e1a222a Mon Sep 17 00:00:00 2001
From: Vitalii Elenhaupt <velenhaupt@gmail.com>
Date: Thu, 26 Mar 2020 10:27:00 +0200
Subject: [PATCH] New rule: EmptyLoop

---
 spec/ameba/rule/lint/empty_loop_spec.cr | 88 +++++++++++++++++++++++++
 src/ameba/ast/visitors/node_visitor.cr  |  1 +
 src/ameba/rule/lint/empty_loop.cr       | 68 +++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 spec/ameba/rule/lint/empty_loop_spec.cr
 create mode 100644 src/ameba/rule/lint/empty_loop.cr

diff --git a/spec/ameba/rule/lint/empty_loop_spec.cr b/spec/ameba/rule/lint/empty_loop_spec.cr
new file mode 100644
index 00000000..26f63fba
--- /dev/null
+++ b/spec/ameba/rule/lint/empty_loop_spec.cr
@@ -0,0 +1,88 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe EmptyLoop do
+    subject = EmptyLoop.new
+
+    it "does not report if there are not empty loops" do
+      s = Source.new %(
+        a = 1
+
+        while a < 10
+          a += 1
+        end
+
+        until a == 10
+         a += 1
+        end
+
+        loop do
+          a += 1
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "reports if there is an empty while loop" do
+      s = Source.new %(
+        a = 1
+        while true
+        end
+      )
+      subject.catch(s).should_not be_valid
+    end
+
+    it "doesn't report if while loop has non-literals in cond block" do
+      s = Source.new %(
+        a = 1
+        while a = gets.to_s
+          # nothing here
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "reports if there is an empty until loop" do
+      s = Source.new %(
+        do_something
+        until false
+        end
+      )
+      subject.catch(s).should_not be_valid
+    end
+
+    it "doesn't report if until loop has non-literals in cond block" do
+      s = Source.new %(
+        until socket_open?
+        end
+      )
+      subject.catch(s).should be_valid
+    end
+
+    it "reports if there an empty loop" do
+      s = Source.new %(
+        a = 1
+        loop do
+
+        end
+      )
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports rule, message and location" do
+      s = Source.new %(
+        a = 1
+        loop do
+          # comment goes here
+        end
+      ), "source.cr"
+      subject.catch(s).should_not be_valid
+      s.issues.size.should eq 1
+      issue = s.issues.first
+      issue.rule.should_not be_nil
+      issue.location.to_s.should eq "source.cr:2:1"
+      issue.end_location.to_s.should eq "source.cr:4:3"
+      issue.message.should eq EmptyLoop::MSG
+    end
+  end
+end
diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr
index 2c4c3032..2384548b 100644
--- a/src/ameba/ast/visitors/node_visitor.cr
+++ b/src/ameba/ast/visitors/node_visitor.cr
@@ -25,6 +25,7 @@ module Ameba::AST
     Var,
     When,
     While,
+    Until,
   ]
 
   # An AST Visitor that traverses the source and allows all nodes
diff --git a/src/ameba/rule/lint/empty_loop.cr b/src/ameba/rule/lint/empty_loop.cr
new file mode 100644
index 00000000..19eafd2f
--- /dev/null
+++ b/src/ameba/rule/lint/empty_loop.cr
@@ -0,0 +1,68 @@
+module Ameba::Rule::Lint
+  # A rule that disallows empty loops.
+  #
+  # This is considered invalid:
+  #
+  # ```
+  # while false
+  # end
+  #
+  # until 10
+  # end
+  #
+  # loop do
+  #   # nothing here
+  # end
+  # ```
+  #
+  # And this is valid:
+  #
+  # ```
+  # a = 1
+  # while a < 10
+  #   a += 1
+  # end
+  #
+  # until socket_opened?
+  # end
+  #
+  # loop do
+  #   do_something_here
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/EmptyLoop:
+  #   Enabled: true
+  # ```
+  struct EmptyLoop < Base
+    include AST::Util
+
+    properties do
+      description "Disallows empty loops"
+    end
+
+    MSG = "Empty loop detected"
+
+    def test(source, node : Crystal::Call)
+      return unless loop?(node)
+
+      check_node(source, node, node.block)
+    end
+
+    def test(source, node : Crystal::While)
+      check_node(source, node, node.body) if literal?(node.cond)
+    end
+
+    def test(source, node : Crystal::Until)
+      check_node(source, node, node.body) if literal?(node.cond)
+    end
+
+    private def check_node(source, node, loop_body)
+      body = loop_body.is_a?(Crystal::Block) ? loop_body.body : loop_body
+      issue_for node, MSG if body.nil? || body.nop?
+    end
+  end
+end