Add Lint/AmbiguousAssignment rule (#244)

spec/ameba/ast/util_spec.cr

@@ -44,7 +44,7 @@ module Ameba::AST
         )
         node = Crystal::Parser.new(s).parse
         source = subject.node_source node, s.split("\n")
-        source.should eq ["a = 1"]
+        source.should eq "a = 1"
       end
 
       it "returns original source of multiline node" do
@@ -55,11 +55,11 @@ module Ameba::AST
         )
         node = Crystal::Parser.new(s).parse
         source = subject.node_source node, s.split("\n")
-        source.should eq([
+        source.should eq <<-CRYSTAL
-          "if ()",
+          if ()
-          "            :ok",
+                      :ok
-          "          end",
+                    end
-        ])
+          CRYSTAL
       end
 
       it "does not report source of node which has incorrect location" do
@@ -81,7 +81,7 @@ module Ameba::AST
         if SemanticVersion.parse(Crystal::VERSION) <= SemanticVersion.parse("0.35.1")
           source.should be_nil
         else
-          source.should eq %w(nil)
+          source.should eq "nil"
         end
       end
     end

spec/ameba/rule/lint/ambiguous_assignment_spec.cr

@@ -0,0 +1,169 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe AmbiguousAssignment do
+    subject = AmbiguousAssignment.new
+
+    context "when using `-`" do
+      it "registers an offense with `x`" do
+        source = Source.new("x =- y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "registers an offense with `@x`" do
+        source = Source.new("@x =- y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?"
+        issue.location.to_s.should eq "source.cr:1:4"
+        issue.end_location.to_s.should eq "source.cr:1:5"
+      end
+
+      it "registers an offense with `@@x`" do
+        source = Source.new("@@x =- y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?"
+        issue.location.to_s.should eq "source.cr:1:5"
+        issue.end_location.to_s.should eq "source.cr:1:6"
+      end
+
+      it "registers an offense with `X`" do
+        source = Source.new("X =- y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "does not register an offense when no mistype assignments" do
+        subject.catch(Source.new(<<-CRYSTAL)).should be_valid
+          x = 1
+          x -= y
+          x = -y
+        CRYSTAL
+      end
+    end
+
+    context "when using `+`" do
+      it "registers an offense with `x`" do
+        source = Source.new("x =+ y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "registers an offense with `@x`" do
+        source = Source.new("@x =+ y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?"
+        issue.location.to_s.should eq "source.cr:1:4"
+        issue.end_location.to_s.should eq "source.cr:1:5"
+      end
+
+      it "registers an offense with `@@x`" do
+        source = Source.new("@@x =+ y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?"
+        issue.location.to_s.should eq "source.cr:1:5"
+        issue.end_location.to_s.should eq "source.cr:1:6"
+      end
+
+      it "registers an offense with `X`" do
+        source = Source.new("X =+ y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "does not register an offense when no mistype assignments" do
+        subject.catch(Source.new(<<-CRYSTAL)).should be_valid
+          x = 1
+          x += y
+          x = +y
+        CRYSTAL
+      end
+    end
+
+    context "when using `!`" do
+      it "registers an offense with `x`" do
+        source = Source.new("x =! y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "registers an offense with `@x`" do
+        source = Source.new("@x =! y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?"
+        issue.location.to_s.should eq "source.cr:1:4"
+        issue.end_location.to_s.should eq "source.cr:1:5"
+      end
+
+      it "registers an offense with `@@x`" do
+        source = Source.new("@@x =! y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?"
+        issue.location.to_s.should eq "source.cr:1:5"
+        issue.end_location.to_s.should eq "source.cr:1:6"
+      end
+
+      it "registers an offense with `X`" do
+        source = Source.new("X =! y", "source.cr")
+        subject.catch(source).should_not be_valid
+        source.issues.size.should eq 1
+
+        issue = source.issues.first
+        issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?"
+        issue.location.to_s.should eq "source.cr:1:3"
+        issue.end_location.to_s.should eq "source.cr:1:4"
+      end
+
+      it "does not register an offense when no mistype assignments" do
+        subject.catch(Source.new(<<-CRYSTAL)).should be_valid
+          x = false
+          x != y
+          x = !y
+        CRYSTAL
+      end
+    end
+  end
+end

src/ameba/ast/util.cr

@@ -35,6 +35,11 @@ module Ameba::AST::Util
 
     return unless loc && end_loc
 
+    source_between(loc, end_loc, code_lines)
+  end
+
+  # Returns the source code from *loc* to *end_loc* (inclusive).
+  def source_between(loc, end_loc, code_lines) : String?
     line, column = loc.line_number - 1, loc.column_number - 1
     end_line, end_column = end_loc.line_number - 1, end_loc.column_number - 1
     node_lines = code_lines[line..end_line]
@@ -53,7 +58,7 @@ module Ameba::AST::Util
     return if last_line.size < end_column + 1
     node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "")
 
-    node_lines
+    node_lines.join('\n')
   end
 
   # Returns true if node is a flow command, false - otherwise.

src/ameba/rule/lint/ambiguous_assignment.cr

@@ -0,0 +1,50 @@
+module Ameba::Rule::Lint
+  # This rule checks for mistyped shorthand assignments.
+  #
+  #     # bad
+  #     x =- y
+  #     x =+ y
+  #     x =! y
+  #
+  #     # good
+  #     x -= y # or x = -y
+  #     x += y # or x = +y
+  #     x != y # or x = !y
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/AmbiguousAssignment:
+  #   Enabled: true
+  # ```
+  class AmbiguousAssignment < Base
+    include AST::Util
+
+    properties do
+      description "Disallows ambiguous `=-/=+/=!`"
+    end
+
+    MSG = "Suspicious assignment detected. Did you mean `%s`?"
+
+    MISTAKES = {
+      "=-" => "-=",
+      "=+" => "+=",
+      "=!" => "!=",
+    }
+
+    def test(source, node : Crystal::Assign)
+      return unless op_end_location = node.value.location
+
+      op_location = Crystal::Location.new(
+        op_end_location.filename,
+        op_end_location.line_number,
+        op_end_location.column_number - 1
+      )
+      op_text = source_between(op_location, op_end_location, source.lines)
+      return unless op_text
+      return unless MISTAKES.has_key?(op_text)
+
+      issue_for op_location, op_end_location, MSG % MISTAKES[op_text]
+    end
+  end
+end

src/ameba/rule/lint/empty_expression.cr

@@ -39,7 +39,7 @@ module Ameba::Rule::Lint
     MSG_EXRS = "Avoid empty expressions"
 
     def test(source, node : Crystal::NilLiteral)
-      exp = node_source(node, source.lines).try &.join
+      exp = node_source(node, source.lines)
       return if exp.in?(nil, "nil")
 
       issue_for node, MSG % exp

src/ameba/rule/style/redundant_begin.cr

@@ -84,7 +84,7 @@ module Ameba::Rule::Style
     private def redundant_begin_in_handler?(source, handler, node)
       return false if begin_exprs_in_handler?(handler) || inner_handler?(handler)
 
-      code = node_source(node, source.lines).try &.join('\n')
+      code = node_source(node, source.lines)
       def_redundant_begin? code if code
     rescue
       false