NewRule: SpecFocus (#204)

* NewRule: SpecFocus

closes #172

* Utilize Source#spec?

spec/ameba/rule/lint/spec_focus_spec.cr

@@ -0,0 +1,127 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  describe SpecFocus do
+    subject = SpecFocus.new
+
+    it "does not report if spec is not focused" do
+      s = Source.new %(
+        context "context" {}
+        describe "describe" {}
+        it "it" {}
+        pending "pending" {}
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should be_valid
+    end
+
+    it "reports if there is a focused context" do
+      s = Source.new %(
+        context "context", focus: true do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports if there is a focused describe block" do
+      s = Source.new %(
+        describe "describe", focus: true do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports if there is a focused it block" do
+      s = Source.new %(
+        it "it", focus: true do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports if there is a focused pending block" do
+      s = Source.new %(
+        pending "pending", focus: true do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+    end
+
+    it "reports if there is a spec item with `focus: false`" do
+      s = Source.new %(
+        it "it", focus: false do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+    end
+
+    it "does not report if there is non spec block with :focus" do
+      s = Source.new %(
+        some_method "foo", focus: true do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should be_valid
+    end
+
+    it "does not report if there is a tagged item with :focus" do
+      s = Source.new %(
+        it "foo", tags: "focus" do
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should be_valid
+    end
+
+    it "does not report if there are focused spec items without blocks" do
+      s = Source.new %(
+        describe "foo", focus: true
+        context "foo", focus: true
+        it "foo", focus: true
+        pending "foo", focus: true
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should be_valid
+    end
+
+    it "does not report if there are focused items out of spec file" do
+      s = Source.new %(
+        describe "foo", focus: true {}
+        context "foo", focus: true {}
+        it "foo", focus: true {}
+        pending "foo", focus: true {}
+      )
+
+      subject.catch(s).should be_valid
+    end
+
+    it "reports rule, pos and message" do
+      s = Source.new %(
+        it "foo", focus: true do
+          it "bar", focus: true {}
+        end
+      ), path: "source_spec.cr"
+
+      subject.catch(s).should_not be_valid
+
+      s.issues.size.should eq(2)
+
+      first, second = s.issues
+
+      first.rule.should_not be_nil
+      first.location.to_s.should eq "source_spec.cr:1:11"
+      first.end_location.to_s.should eq ""
+      first.message.should eq "Focused spec item detected"
+
+      second.rule.should_not be_nil
+      second.location.to_s.should eq "source_spec.cr:2:13"
+      second.end_location.to_s.should eq ""
+      second.message.should eq "Focused spec item detected"
+    end
+  end
+end

spec/ameba/source_spec.cr

@@ -23,6 +23,18 @@ module Ameba
       end
     end
 
+    describe "#spec?" do
+      it "returns true if the source is a spec file" do
+        s = Source.new "", "./source_spec.cr"
+        s.spec?.should be_true
+      end
+
+      it "returns false if the source is not a spec file" do
+        s = Source.new "", "./source.cr"
+        s.spec?.should be_false
+      end
+    end
+
     describe "#matches_path?" do
       it "returns true if source's path is matched" do
         s = Source.new "", "source.cr"

src/ameba/rule/lint/spec_focus.cr

@@ -0,0 +1,70 @@
+module Ameba::Rule::Lint
+  # Checks if specs are focused.
+  #
+  # In specs `focus: true` is mainly used to focus on a spec
+  # item locally during development. However, if such change
+  # is committed, it silently runs only focused spec on all
+  # other enviroment, which is undesired.
+  #
+  # This is considered bad:
+  #
+  # ```
+  # describe MyClass, focus: true do
+  # end
+  #
+  # describe ".new", focus: true do
+  # end
+  #
+  # context "my context", focus: true do
+  # end
+  #
+  # it "works", focus: true do
+  # end
+  # ```
+  #
+  # And it should be written as the following:
+  #
+  # ```
+  # describe MyClass do
+  # end
+  #
+  # describe ".new" do
+  # end
+  #
+  # context "my context" do
+  # end
+  #
+  # it "works" do
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/SpecFocus:
+  #   Enabled: true
+  # ```
+  class SpecFocus < Base
+    properties do
+      description "Reports focused spec items"
+    end
+
+    MSG             = "Focused spec item detected"
+    SPEC_ITEM_NAMES = %w(describe context it pending)
+
+    def test(source)
+      return unless source.spec?
+
+      AST::NodeVisitor.new self, source
+    end
+
+    def test(source, node : Crystal::Call)
+      return unless node.name.in?(SPEC_ITEM_NAMES)
+      return unless node.block
+
+      arg = node.named_args.try &.find(&.name.== "focus")
+
+      issue_for arg, MSG if arg
+    end
+  end
+end

src/ameba/source.cr

@@ -53,7 +53,12 @@ module Ameba
       File.expand_path(path)
     end
 
-    # Returns true if *filepath* matches the source's path, false if it does not.
+    # Returns `true` if the source is a spec file, `false` otherwise.
+    def spec?
+      path.ends_with?("_spec.cr")
+    end
+
+    # Returns `true` if *filepath* matches the source's path, `false` if it does not.
     def matches_path?(filepath)
       path == filepath || path == File.expand_path(filepath)
     end