New rule: UnlessElse

README.md

@@ -40,18 +40,18 @@ Ameba.run
 ```
 
 ```sh
-Inspecting 7 files.
+Inspecting 18 files.
 
 
-..F...F
+...............F.F
 
-7 inspected, 2 failures.
+18 inspected, 2 failures.
 
-src/ameba/formatter.cr:47
+src/ameba/source.cr:26
-LineLength: Line too long [122]
+LineLength: Line too long (82 symbols)
 
-src/ameba.cr:18
+src/ameba.cr:12
-LineLength: Line too long [81]
+UnlessElse: Favour if over unless with else
 ```
 
 ## Contributing

spec/ameba/rules/unless_else_spec.cr

@@ -0,0 +1,44 @@
+require "../../spec_helper"
+
+module Ameba::Rules
+  subject = UnlessElse.new
+
+  describe UnlessElse do
+    it "passes if unless hasn't else" do
+      s = Source.new %(
+        unless something
+          :ok
+        end
+      )
+      subject.catch(s).valid?.should be_true
+    end
+
+    it "fails if unless has else" do
+      s = Source.new %(
+        unless something
+          :one
+        else
+          :two
+        end
+      )
+      subject.catch(s).valid?.should be_false
+    end
+
+    it "reports rule, pos and message" do
+      s = Source.new %(
+        unless something
+          :one
+        else
+          :two
+        end
+      )
+      subject.catch(s)
+
+      error = s.errors.first
+      error.should_not be_nil
+      error.rule.should_not be_nil
+      error.pos.should eq 2
+      error.message.should eq "Favour if over unless with else"
+    end
+  end
+end

src/ameba.cr

@@ -13,7 +13,7 @@ module Ameba
   end
 
   def run(files, formatter : Formatter)
-    sources = files.map { |path| Source.new(File.read path) }
+    sources = files.map { |path| Source.new(File.read(path), path) }
 
     reporter = Reporter.new formatter
     reporter.start sources

src/ameba/dsl.cr

@@ -0,0 +1,32 @@
+module Ameba
+  macro rule(name, &block)
+    module Ameba::Rules
+      struct {{name.id}} < Rule
+        def test(source)
+          {{block.body}}
+        end
+      end
+    end
+  end
+
+  macro visitor(name, node, &block)
+    module Ameba::Rules
+      class {{name.id}}Visitor < Crystal::Visitor
+        @rule : Rule
+        @source : Source
+
+        def initialize(@rule, @source)
+          @source.ast.accept self
+        end
+
+        def visit(node : Crystal::ASTNode)
+          true
+        end
+
+        def visit(node : {{node.id}})
+          {{block.body}}
+        end
+      end
+    end
+  end
+end

src/ameba/rule.cr

@@ -3,18 +3,9 @@ module Ameba
     Rules::LineLength,
     Rules::TrailingBlankLines,
     Rules::TrailingWhitespace,
+    Rules::UnlessElse,
   ]
 
-  macro rule(name, &block)
-    module Ameba::Rules
-      struct {{name.id}} < Rule
-        def test(source)
-          {{block.body}}
-        end
-      end
-    end
-  end
-
   abstract struct Rule
     abstract def test(source : Source)
 

src/ameba/rules/unless_else.cr

@@ -0,0 +1,47 @@
+# A rule that disallows the use of an `else` block with the `unless`.
+#
+# For example, the rule considers these valid:
+#
+# ```crystal
+# unless something
+#   :ok
+# end
+#
+# if something
+#   :one
+# else
+#   :two
+# end
+# ```
+#
+# But it considers this one invalid as it is an `unless` with an `else`:
+#
+# ```crystal
+# unless something
+#   :one
+# else
+#   :two
+# end
+# ```
+#
+# The solution is to swap the order of the blocks, and change the `unless` to
+# an `if`, so the previous invalid example would become this:
+#
+# ```crystal
+# if something
+#   :two
+# else
+#   :one
+# end
+# ```
+
+Ameba.rule UnlessElse do |source|
+  UnlessElseVisitor.new self, source
+end
+
+Ameba.visitor UnlessElse, Crystal::Unless do |node|
+  unless node.else.is_a?(Crystal::Nop)
+    @source.error @rule, node.location.try &.line_number,
+      "Favour if over unless with else"
+  end
+end

src/ameba/source.cr

@@ -10,7 +10,7 @@ module Ameba
     # position of the error and a message.
     record Error,
       rule : Rule,
-      pos : Int32,
+      pos : Int32?,
       message : String
 
     getter lines : Array(String)
@@ -22,7 +22,7 @@ module Ameba
       @lines = @content.split("\n")
     end
 
-    def error(rule : Rule, line_number : Int32, message : String)
+    def error(rule : Rule, line_number : Int32?, message : String)
       errors << Error.new rule, line_number, message
     end