Merge pull request #465 from crystal-ameba/fix-issue-464

Revert to `nil` in `Rule::Lint::Typos.BIN_PATH` if `Process.find_executable` fails

.ameba.yml

@@ -0,0 +1,7 @@
+Documentation/DocumentationAdmonition:
+  Timezone: UTC
+  Admonitions: [FIXME, BUG]
+
+Lint/Typos:
+  Excluded:
+    - spec/ameba/rule/lint/typos_spec.cr

.github/workflows/cd.yml

@@ -24,18 +24,18 @@ jobs:
 
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Set up QEMU
-        uses: docker/setup-qemu-action@v2
+        uses: docker/setup-qemu-action@v3
 
       - name: Setup Docker Buildx
-        uses: docker/setup-buildx-action@v2
+        uses: docker/setup-buildx-action@v3
 
       # Login against a Docker registry except on PR
       # https://github.com/docker/login-action
       - name: Log into ${{ env.REGISTRY }} registry
-        uses: docker/login-action@v2
+        uses: docker/login-action@v3
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}
@@ -45,7 +45,7 @@ jobs:
       # https://github.com/docker/metadata-action
       - name: Extract Docker metadata
         id: meta
-        uses: docker/metadata-action@v4
+        uses: docker/metadata-action@v5
         with:
           images: |
             ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
@@ -61,7 +61,7 @@ jobs:
       # https://github.com/docker/build-push-action
       - name: Build and push Docker image
         id: build-and-push
-        uses: docker/build-push-action@v4
+        uses: docker/build-push-action@v6
         with:
           context: .
           push: true

.github/workflows/ci.yml

@@ -18,17 +18,24 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     steps:
+      - name: Set timezone to UTC
+        uses: szenius/set-timezone@v2.0
+
       - name: Install Crystal
         uses: crystal-lang/install-crystal@v1
         with:
           crystal: ${{ matrix.crystal }}
 
       - name: Download source
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install dependencies
         run: shards install
 
+      - name: Install typos-cli
+        if: matrix.os == 'macos-latest'
+        run: brew install typos-cli
+
       - name: Run specs
         run: crystal spec
 
@@ -36,4 +43,4 @@ jobs:
         run: shards build -Dpreview_mt
 
       - name: Run ameba linter
-        run: bin/ameba --all
+        run: bin/ameba

.github/workflows/docs.yml

@@ -19,7 +19,7 @@ jobs:
         uses: crystal-lang/install-crystal@v1
 
       - name: Download source
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - name: Install dependencies
         run: shards install

Dockerfile

@@ -6,8 +6,9 @@ COPY . /ameba/
 RUN make clean && make
 
 FROM alpine:latest
-RUN apk add --update yaml pcre gc libevent libgcc
+RUN apk add --update yaml pcre2 gc libevent libgcc
 RUN mkdir /src
 WORKDIR /src
 COPY --from=builder /ameba/bin/ameba /usr/bin/
+RUN ameba -v
 ENTRYPOINT [ "/usr/bin/ameba" ]

Makefile

@@ -1,38 +1,92 @@
+.POSIX:
+all:
+
+# Recipes
+
+## Build ameba
+##   $ make
+## Run tests
+##   $ make test
+## Install ameba
+##   $ sudo make install
+
+-include Makefile.local # for optional local options
+
+BUILD_TARGET ::= bin/ameba
+
+DESTDIR ?=          ## Install destination dir
+PREFIX ?= /usr/local## Install path prefix
+BINDIR ?= $(DESTDIR)$(PREFIX)/bin
+
+# The crystal command to use
 CRYSTAL_BIN ?= crystal
+# The shards command to use
 SHARDS_BIN ?= shards
-PREFIX ?= /usr/local
+# The install command to use
+INSTALL_BIN ?= /usr/bin/install
+
 SHARD_BIN ?= ../../bin
 CRFLAGS ?= -Dpreview_mt
 
+SRC_SOURCES ::= $(shell find src -name '*.cr' 2>/dev/null)
+DOC_SOURCE   ::= src/**
+
+.PHONY: all
+all: build
+
 .PHONY: build
-build:
+build: ## Build the application binary
+build: $(BUILD_TARGET)
+
+$(BUILD_TARGET): $(SRC_SOURCES)
 	$(SHARDS_BIN) build $(CRFLAGS)
 
+docs: ## Generate API docs
+docs: $(SRC_SOURCES)
+	$(CRYSTAL_BIN) docs -o docs $(DOC_SOURCE)
+
 .PHONY: lint
-lint: build
-	./bin/ameba --all
+lint: ## Run ameba on ameba's code base
+lint: $(BUILD_TARGET)
+	$(BUILD_TARGET)
 
 .PHONY: spec
+spec: ## Run the spec suite
 spec:
 	$(CRYSTAL_BIN) spec
 
 .PHONY: clean
+clean: ## Remove application binary
 clean:
-	rm -f ./bin/ameba ./bin/ameba.dwarf
+	@rm -f "$(BUILD_TARGET)" "$(BUILD_TARGET).dwarf"
 
 .PHONY: install
-install: build
-	mkdir -p $(PREFIX)/bin
-	cp ./bin/ameba $(PREFIX)/bin
+install: ## Install application binary into $DESTDIR
+install: $(BUILD_TARGET)
+	$(INSTALL_BIN) -m 0755 "$(BUILD_TARGET)" "$(BINDIR)/ameba"
 
 .PHONY: bin
 bin: build
 	mkdir -p $(SHARD_BIN)
-	cp ./bin/ameba $(SHARD_BIN)
-
-.PHONY: run_file
-run_file:
-	cp -n ./bin/ameba.cr $(SHARD_BIN) || true
+	cp $(BUILD_TARGET) $(SHARD_BIN)
 
 .PHONY: test
+test: ## Run the spec suite and linter
 test: spec lint
+
+.PHONY: help
+help: ## Show this help
+	@echo
+	@printf '\033[34mtargets:\033[0m\n'
+	@grep -hE '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) |\
+		sort |\
+		awk 'BEGIN {FS = ":.*?## "}; {printf "  \033[36m%-15s\033[0m %s\n", $$1, $$2}'
+	@echo
+	@printf '\033[34moptional variables:\033[0m\n'
+	@grep -hE '^[a-zA-Z_-]+ \?=.*?## .*$$' $(MAKEFILE_LIST) |\
+		sort |\
+		awk 'BEGIN {FS = " \\?=.*?## "}; {printf "  \033[36m%-15s\033[0m %s\n", $$1, $$2}'
+	@echo
+	@printf '\033[34mrecipes:\033[0m\n'
+	@grep -hE '^##.*$$' $(MAKEFILE_LIST) |\
+		awk 'BEGIN {FS = "## "}; /^## [a-zA-Z_-]/ {printf "  \033[36m%s\033[0m\n", $$2}; /^##  / {printf "  %s\n", $$2}'

README.md

@@ -8,7 +8,7 @@
     </sup>
   </p>
   <p align="center">
-    <a href="https://github.com/crystal-ameba/ameba/actions?query=workflow%3ACI"><img src="https://github.com/crystal-ameba/ameba/workflows/CI/badge.svg"></a>
+    <a href="https://github.com/crystal-ameba/ameba/actions/workflows/ci.yml"><img src="https://github.com/crystal-ameba/ameba/actions/workflows/ci.yml/badge.svg"></a>
     <a href="https://github.com/crystal-ameba/ameba/releases"><img src="https://img.shields.io/github/release/crystal-ameba/ameba.svg?maxAge=360"></a>
     <a href="https://github.com/crystal-ameba/ameba/blob/master/LICENSE"><img src="https://img.shields.io/github/license/crystal-ameba/ameba.svg"></a>
   </p>
@@ -99,15 +99,6 @@ $ ameba --explain crystal/command/format.cr:26:83 # same thing
 
 ### Run in parallel
 
-Starting from 0.31.0 Crystal [supports parallelism](https://crystal-lang.org/2019/09/06/parallelism-in-crystal.html).
-It allows to run linting in parallel too.
-In order to take advantage of this feature you need to build ameba with preview_mt support:
-
-```sh
-$ crystal build src/cli.cr -Dpreview_mt -o bin/ameba
-$ make install
-```
-
 Some quick benchmark results measured while running Ameba on Crystal repo:
 
 ```sh
@@ -127,7 +118,6 @@ Add this to your application's `shard.yml`:
 development_dependencies:
   ameba:
     github: crystal-ameba/ameba
-    version: ~> 1.4.0
 ```
 
 Build `bin/ameba` binary within your project directory while running `shards install`.
@@ -135,7 +125,7 @@ Build `bin/ameba` binary within your project directory while running `shards ins
 ### OS X
 
 ```sh
-$ brew tap veelenga/tap
+$ brew tap crystal-ameba/ameba
 $ brew install ameba
 ```
 
@@ -174,7 +164,7 @@ Generate new file by running `ameba --gen-config`.
 **List of sources to run Ameba on can be configured globally via:**
 
 - `Globs` section - an array of wildcards (or paths) to include to the
-  inspection. Defaults to `%w(**/*.cr !lib)`, meaning it includes all project
+  inspection. Defaults to `%w[**/*.cr !lib]`, meaning it includes all project
   files with `*.cr` extension except those which exist in `lib` folder.
 - `Excluded` section - an array of wildcards (or paths) to exclude from the
   source list defined by `Globs`. Defaults to an empty array.
@@ -195,8 +185,8 @@ Excluded:
 ``` yaml
 Style/RedundantBegin:
   Excluded:
-  - src/server/processor.cr
-  - src/server/api.cr
+    - src/server/processor.cr
+    - src/server/api.cr
 ```
 
 ### Rules
@@ -249,4 +239,4 @@ time = Time.epoch(1483859302) # ameba:disable Style, Lint
 ## Contributors
 
 - [veelenga](https://github.com/veelenga) Vitalii Elenhaupt - creator, maintainer
-- [Sija](https://github.com/Sija) Sijawusz Pur Rahnama - maintainer
+- [Sija](https://github.com/Sija) Sijawusz Pur Rahnama - contributor, maintainer

bench/check_sources.cr

@@ -15,7 +15,7 @@ Benchmark.ips do |x|
     20,
     30,
     40,
-  ].each do |n|
+  ].each do |n| # ameba:disable Naming/BlockParameterName
     config = Ameba::Config.load
     config.formatter = Ameba::Formatter::BaseFormatter.new
     config.globs = get_files(n)

shard.yml

@@ -1,20 +1,22 @@
 name: ameba
-version: 1.4.3
+version: 1.6.1
 
 authors:
   - Vitalii Elenhaupt <velenhaupt@gmail.com>
+  - Sijawusz Pur Rahnama <sija@sija.pl>
 
 targets:
   ameba:
     main: src/cli.cr
 
 scripts:
-  # TODO: remove pre-compiled executable in future releases
-  postinstall: make bin && make run_file
+  postinstall: shards build -Dpreview_mt
 
+# TODO: remove pre-compiled executable in future releases
 executables:
   - ameba
+  - ameba.cr
 
-crystal: "~> 1.7.0"
+crystal: ~> 1.10
 
 license: MIT

spec/ameba/ast/branch_spec.cr

@@ -298,6 +298,34 @@ module Ameba::AST
       end
     end
 
+    context "Crystal::Call" do
+      context "loop" do
+        it "constructs a branch in block" do
+          branch = branch_of_assign_in_def <<-CRYSTAL
+            def method(a)
+              loop do
+                b = (a = 1)
+              end
+            end
+            CRYSTAL
+          branch.to_s.should eq "b = (a = 1)"
+        end
+      end
+
+      context "other" do
+        it "skips constructing a branch in block" do
+          branch = branch_of_assign_in_def <<-CRYSTAL
+            def method(a)
+              1.upto(10) do
+                b = (a = 1)
+              end
+            end
+            CRYSTAL
+          branch.should be_nil
+        end
+      end
+    end
+
     describe "#initialize" do
       it "creates new branch" do
         nodes = as_nodes <<-CRYSTAL
@@ -358,6 +386,30 @@ module Ameba::AST
         branch = Branch.new nodes.assign_nodes.first, branchable
         branch.in_loop?.should be_false
       end
+
+      context "Crystal::Call" do
+        it "returns true if branch is in a loop" do
+          nodes = as_nodes <<-CRYSTAL
+            loop do
+              a = 1
+            end
+            CRYSTAL
+          branchable = Branchable.new nodes.call_nodes.first
+          branch = Branch.new nodes.assign_nodes.first, branchable
+          branch.in_loop?.should be_true
+        end
+
+        it "returns false if branch is not in a loop" do
+          nodes = as_nodes <<-CRYSTAL
+            1.upto(10) do
+              a = 1
+            end
+            CRYSTAL
+          branchable = Branchable.new nodes.call_nodes.first
+          branch = Branch.new nodes.assign_nodes.first, branchable
+          branch.in_loop?.should be_false
+        end
+      end
     end
   end
 end

spec/ameba/ast/scope_spec.cr

@@ -57,13 +57,15 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
-      variable.reference nodes.var_nodes.first, scope.inner_scopes.first
+      variable.reference(nodes.var_nodes.first, scope.inner_scopes.first)
 
       scope.references?(variable).should be_true
     end
@@ -77,13 +79,15 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
-      variable.reference nodes.var_nodes.first, scope.inner_scopes.first
+      variable.reference(nodes.var_nodes.first, scope.inner_scopes.first)
 
       scope.references?(variable, check_inner_scopes: false).should be_false
     end
@@ -98,9 +102,11 @@ module Ameba::AST
           end
         end
         CRYSTAL
-      scope = Scope.new nodes.def_nodes.first
+
       var_node = nodes.var_nodes.first
-      scope.add_variable var_node
+
+      scope = Scope.new nodes.def_nodes.first
+      scope.add_variable(var_node)
       scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
       variable = Variable.new(var_node, scope)
@@ -120,7 +126,7 @@ module Ameba::AST
   describe "#find_variable" do
     it "returns the variable in the scope by name" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.find_variable("foo").should_not be_nil
     end
 
@@ -133,7 +139,7 @@ module Ameba::AST
   describe "#assign_variable" do
     it "creates a new assignment" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.assign_variable("foo", Crystal::Var.new "foo")
       var = scope.find_variable("foo").should_not be_nil
       var.assignments.size.should eq 1
@@ -141,7 +147,7 @@ module Ameba::AST
 
     it "does not create the assignment if variable is wrong" do
       scope = Scope.new as_node("foo = 1")
-      scope.add_variable Crystal::Var.new "foo"
+      scope.add_variable(Crystal::Var.new "foo")
       scope.assign_variable("bar", Crystal::Var.new "bar")
       var = scope.find_variable("foo").should_not be_nil
       var.assignments.size.should eq 0
@@ -174,6 +180,28 @@ module Ameba::AST
     end
   end
 
+  describe "#def?" do
+    context "when check_outer_scopes: true" do
+      it "returns true if outer scope is Crystal::Def" do
+        nodes = as_nodes("def foo; 3.times {}; end")
+        outer_scope = Scope.new nodes.def_nodes.first
+        scope = Scope.new nodes.block_nodes.first, outer_scope
+        scope.def?(check_outer_scopes: true).should be_true
+      end
+    end
+
+    it "returns true if Crystal::Def" do
+      nodes = as_nodes("def foo; end")
+      scope = Scope.new nodes.def_nodes.first
+      scope.def?.should be_true
+    end
+
+    it "returns false otherwise" do
+      scope = Scope.new as_node("a = 1")
+      scope.def?.should be_false
+    end
+  end
+
   describe "#in_macro?" do
     it "returns true if Crystal::Macro" do
       nodes = as_nodes <<-CRYSTAL

spec/ameba/ast/util_spec.cr

@@ -36,6 +36,43 @@ module Ameba::AST
       end
     end
 
+    describe "#static/dynamic_literal?" do
+      [
+        Crystal::ArrayLiteral.new,
+        Crystal::ArrayLiteral.new([Crystal::StringLiteral.new("foo")] of Crystal::ASTNode),
+        Crystal::BoolLiteral.new(false),
+        Crystal::CharLiteral.new('a'),
+        Crystal::HashLiteral.new,
+        Crystal::NamedTupleLiteral.new,
+        Crystal::NilLiteral.new,
+        Crystal::NumberLiteral.new(42),
+        Crystal::RegexLiteral.new(Crystal::StringLiteral.new("")),
+        Crystal::StringLiteral.new("foo"),
+        Crystal::SymbolLiteral.new("foo"),
+        Crystal::TupleLiteral.new([] of Crystal::ASTNode),
+        Crystal::TupleLiteral.new([Crystal::StringLiteral.new("foo")] of Crystal::ASTNode),
+        Crystal::RangeLiteral.new(
+          Crystal::NumberLiteral.new(0),
+          Crystal::NumberLiteral.new(10),
+          true),
+      ].each do |literal|
+        it "properly identifies static node #{literal}" do
+          subject.static_literal?(literal).should be_true
+          subject.dynamic_literal?(literal).should be_false
+        end
+      end
+
+      [
+        Crystal::ArrayLiteral.new([Crystal::Path.new(%w[IO])] of Crystal::ASTNode),
+        Crystal::TupleLiteral.new([Crystal::Path.new(%w[IO])] of Crystal::ASTNode),
+      ].each do |literal|
+        it "properly identifies dynamic node #{literal}" do
+          subject.dynamic_literal?(literal).should be_true
+          subject.static_literal?(literal).should be_false
+        end
+      end
+    end
+
     describe "#node_source" do
       it "returns original source of the node" do
         s = <<-CRYSTAL

spec/ameba/ast/variabling/assignment_spec.cr

@@ -85,30 +85,5 @@ module Ameba::AST
         assignment.branch.should be_nil
       end
     end
-
-    describe "#transformed?" do
-      it "returns false if the assignment is not transformed by the compiler" do
-        nodes = as_nodes <<-CRYSTAL
-          def method(a)
-            a = 2
-          end
-          CRYSTAL
-        scope = Scope.new nodes.def_nodes.first
-        variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
-        assignment.transformed?.should be_false
-      end
-
-      it "returns true if the assignment is transformed by the compiler" do
-        nodes = as_nodes <<-CRYSTAL
-          array.each do |(a, b)|
-          end
-          CRYSTAL
-        scope = Scope.new nodes.block_nodes.first
-        variable = Variable.new(nodes.var_nodes.first, scope)
-        assignment = Assignment.new(nodes.assign_nodes.first, variable, scope)
-        assignment.transformed?.should be_true
-      end
-    end
   end
 end

spec/ameba/ast/variabling/variable_spec.cr

@@ -85,13 +85,16 @@ module Ameba::AST
             3.times { |i| a = a + i }
           end
           CRYSTAL
-        scope = Scope.new nodes.def_nodes.first
+
         var_node = nodes.var_nodes.first
-        scope.add_variable var_node
+
+        scope = Scope.new(nodes.def_nodes.first)
+        scope.add_variable(var_node)
         scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope)
 
         variable = Variable.new(var_node, scope)
-        variable.reference nodes.var_nodes.last, scope.inner_scopes.last
+        variable.reference(nodes.var_nodes.last, scope.inner_scopes.last)
+
         variable.captured_by_block?.should be_truthy
       end
 
@@ -101,8 +104,10 @@ module Ameba::AST
             a = 1
           end
           CRYSTAL
-        scope.add_variable Crystal::Var.new "a"
+
+        scope.add_variable(Crystal::Var.new "a")
         variable = scope.variables.first
+
         variable.captured_by_block?.should be_falsey
       end
     end

spec/ameba/ast/visitors/flow_expression_visitor_spec.cr

@@ -1,8 +1,6 @@
 require "../../../spec_helper"
 
 module Ameba::AST
-  source = Source.new ""
-
   describe FlowExpressionVisitor do
     it "creates an expression for return" do
       rule = FlowExpressionRule.new

spec/ameba/ast/visitors/scope_visitor_spec.cr

@@ -2,6 +2,17 @@ require "../../../spec_helper"
 
 module Ameba::AST
   describe ScopeVisitor do
+    {% for type in %w[class module enum].map(&.id) %}
+      it "creates a scope for the {{ type }} def" do
+        rule = ScopeRule.new
+        ScopeVisitor.new rule, Source.new <<-CRYSTAL
+          {{ type }} Foo
+          end
+          CRYSTAL
+        rule.scopes.size.should eq 1
+      end
+    {% end %}
+
     it "creates a scope for the def" do
       rule = ScopeRule.new
       ScopeVisitor.new rule, Source.new <<-CRYSTAL
@@ -54,5 +65,33 @@ module Ameba::AST
         outer_block.outer_scope.should be_nil
       end
     end
+
+    context "#visibility" do
+      it "is being properly set" do
+        rule = ScopeRule.new
+        ScopeVisitor.new rule, Source.new <<-CRYSTAL
+          private class Foo
+          end
+          CRYSTAL
+        rule.scopes.size.should eq 1
+        rule.scopes.first.visibility.should eq Crystal::Visibility::Private
+      end
+
+      it "is being inherited from the outer scope(s)" do
+        rule = ScopeRule.new
+        ScopeVisitor.new rule, Source.new <<-CRYSTAL
+          private class Foo
+            class Bar
+              def baz
+              end
+            end
+          end
+          CRYSTAL
+        rule.scopes.size.should eq 3
+        rule.scopes.each &.visibility.should eq Crystal::Visibility::Private
+        rule.scopes.last.node.visibility.should eq Crystal::Visibility::Private
+        rule.scopes[0...-1].each &.node.visibility.should eq Crystal::Visibility::Public
+      end
+    end
   end
 end

spec/ameba/base_spec.cr

@@ -10,14 +10,16 @@ module Ameba::Rule
       end
 
       it "contains rules across all the available groups" do
-        Rule.rules.map(&.group_name).uniq!.reject!(&.empty?).sort.should eq %w(
+        Rule.rules.map(&.group_name).uniq!.reject!(&.empty?).sort.should eq %w[
           Ameba
+          Documentation
           Layout
           Lint
           Metrics
+          Naming
           Performance
           Style
-        )
+        ]
       end
     end
 
@@ -48,25 +50,25 @@ module Ameba::Rule
 
       it "returns false if source is not excluded from this rule" do
         rule = DummyRule.new
-        rule.excluded = %w(some_source.cr)
+        rule.excluded = %w[some_source.cr]
         rule.excluded?(Source.new "", "another_source.cr").should_not be_true
       end
 
       it "returns true if source is excluded from this rule" do
         rule = DummyRule.new
-        rule.excluded = %w(source.cr)
+        rule.excluded = %w[source.cr]
         rule.excluded?(Source.new "", "source.cr").should be_true
       end
 
       it "returns true if source matches the wildcard" do
         rule = DummyRule.new
-        rule.excluded = %w(**/*.cr)
+        rule.excluded = %w[**/*.cr]
         rule.excluded?(Source.new "", __FILE__).should be_true
       end
 
       it "returns false if source does not match the wildcard" do
         rule = DummyRule.new
-        rule.excluded = %w(*_spec.cr)
+        rule.excluded = %w[*_spec.cr]
         rule.excluded?(Source.new "", "source.cr").should be_false
       end
     end

spec/ameba/cli/cmd_spec.cr

@@ -5,97 +5,97 @@ module Ameba::Cli
   describe "Cmd" do
     describe ".run" do
       it "runs ameba" do
-        r = Cli.run %w(-f silent file.cr)
+        r = Cli.run %w[-f silent file.cr]
         r.should be_nil
       end
     end
 
     describe ".parse_args" do
-      %w(-s --silent).each do |f|
-        it "accepts #{f} flag" do
-          c = Cli.parse_args [f]
+      %w[-s --silent].each do |flag|
+        it "accepts #{flag} flag" do
+          c = Cli.parse_args [flag]
           c.formatter.should eq :silent
         end
       end
 
-      %w(-c --config).each do |f|
-        it "accepts #{f} flag" do
-          c = Cli.parse_args [f, "config.yml"]
+      %w[-c --config].each do |flag|
+        it "accepts #{flag} flag" do
+          c = Cli.parse_args [flag, "config.yml"]
           c.config.should eq Path["config.yml"]
         end
       end
 
-      %w(-f --format).each do |f|
-        it "accepts #{f} flag" do
-          c = Cli.parse_args [f, "my-formatter"]
+      %w[-f --format].each do |flag|
+        it "accepts #{flag} flag" do
+          c = Cli.parse_args [flag, "my-formatter"]
           c.formatter.should eq "my-formatter"
         end
       end
 
       it "accepts --only flag" do
         c = Cli.parse_args ["--only", "RULE1,RULE2"]
-        c.only.should eq %w(RULE1 RULE2)
+        c.only.should eq %w[RULE1 RULE2]
       end
 
       it "accepts --except flag" do
         c = Cli.parse_args ["--except", "RULE1,RULE2"]
-        c.except.should eq %w(RULE1 RULE2)
+        c.except.should eq %w[RULE1 RULE2]
       end
 
       it "defaults rules? flag to false" do
-        c = Cli.parse_args %w(file.cr)
+        c = Cli.parse_args %w[file.cr]
         c.rules?.should be_false
       end
 
       it "defaults skip_reading_config? flag to false" do
-        c = Cli.parse_args %w(file.cr)
+        c = Cli.parse_args %w[file.cr]
         c.skip_reading_config?.should be_false
       end
 
       it "accepts --rules flag" do
-        c = Cli.parse_args %w(--rules)
+        c = Cli.parse_args %w[--rules]
         c.rules?.should eq true
       end
 
       it "defaults all? flag to false" do
-        c = Cli.parse_args %w(file.cr)
+        c = Cli.parse_args %w[file.cr]
         c.all?.should be_false
       end
 
       it "accepts --all flag" do
-        c = Cli.parse_args %w(--all)
+        c = Cli.parse_args %w[--all]
         c.all?.should eq true
       end
 
       it "accepts --gen-config flag" do
-        c = Cli.parse_args %w(--gen-config)
+        c = Cli.parse_args %w[--gen-config]
         c.formatter.should eq :todo
       end
 
       it "accepts --no-color flag" do
-        c = Cli.parse_args %w(--no-color)
+        c = Cli.parse_args %w[--no-color]
         c.colors?.should be_false
       end
 
       it "accepts --without-affected-code flag" do
-        c = Cli.parse_args %w(--without-affected-code)
+        c = Cli.parse_args %w[--without-affected-code]
         c.without_affected_code?.should be_true
       end
 
       it "doesn't disable colors by default" do
-        c = Cli.parse_args %w(--all)
+        c = Cli.parse_args %w[--all]
         c.colors?.should be_true
       end
 
       it "ignores --config if --gen-config flag passed" do
-        c = Cli.parse_args %w(--gen-config --config my_config.yml)
+        c = Cli.parse_args %w[--gen-config --config my_config.yml]
         c.formatter.should eq :todo
         c.skip_reading_config?.should be_true
       end
 
       describe "-e/--explain" do
         it "configures file/line/column" do
-          c = Cli.parse_args %w(--explain src/file.cr:3:5)
+          c = Cli.parse_args %w[--explain src/file.cr:3:5]
 
           location_to_explain = c.location_to_explain.should_not be_nil
           location_to_explain[:file].should eq "src/file.cr"
@@ -105,59 +105,59 @@ module Ameba::Cli
 
         it "raises an error if location is not valid" do
           expect_raises(Exception, "location should have PATH:line:column") do
-            Cli.parse_args %w(--explain src/file.cr:3)
+            Cli.parse_args %w[--explain src/file.cr:3]
           end
         end
 
         it "raises an error if line number is not valid" do
           expect_raises(Exception, "location should have PATH:line:column") do
-            Cli.parse_args %w(--explain src/file.cr:a:3)
+            Cli.parse_args %w[--explain src/file.cr:a:3]
           end
         end
 
         it "raises an error if column number is not valid" do
           expect_raises(Exception, "location should have PATH:line:column") do
-            Cli.parse_args %w(--explain src/file.cr:3:&)
+            Cli.parse_args %w[--explain src/file.cr:3:&]
           end
         end
 
         it "raises an error if line/column are missing" do
           expect_raises(Exception, "location should have PATH:line:column") do
-            Cli.parse_args %w(--explain src/file.cr)
+            Cli.parse_args %w[--explain src/file.cr]
           end
         end
       end
 
       context "--fail-level" do
         it "configures fail level Convention" do
-          c = Cli.parse_args %w(--fail-level convention)
+          c = Cli.parse_args %w[--fail-level convention]
           c.fail_level.should eq Severity::Convention
         end
 
         it "configures fail level Warning" do
-          c = Cli.parse_args %w(--fail-level Warning)
+          c = Cli.parse_args %w[--fail-level Warning]
           c.fail_level.should eq Severity::Warning
         end
 
         it "configures fail level Error" do
-          c = Cli.parse_args %w(--fail-level error)
+          c = Cli.parse_args %w[--fail-level error]
           c.fail_level.should eq Severity::Error
         end
 
         it "raises if fail level is incorrect" do
           expect_raises(Exception, "Incorrect severity name JohnDoe") do
-            Cli.parse_args %w(--fail-level JohnDoe)
+            Cli.parse_args %w[--fail-level JohnDoe]
           end
         end
       end
 
       it "accepts unknown args as globs" do
-        c = Cli.parse_args %w(source1.cr source2.cr)
-        c.globs.should eq %w(source1.cr source2.cr)
+        c = Cli.parse_args %w[source1.cr source2.cr]
+        c.globs.should eq %w[source1.cr source2.cr]
       end
 
       it "accepts one unknown arg as explain location if it has correct format" do
-        c = Cli.parse_args %w(source.cr:3:22)
+        c = Cli.parse_args %w[source.cr:3:22]
 
         location_to_explain = c.location_to_explain.should_not be_nil
         location_to_explain[:file].should eq "source.cr"

spec/ameba/config_spec.cr

@@ -2,7 +2,7 @@ require "../spec_helper"
 
 module Ameba
   describe Config do
-    config_sample = "config/ameba.yml"
+    config_sample = "spec/fixtures/config.yml"
 
     it "should have a list of available formatters" do
       Config::AVAILABLE_FORMATTERS.should_not be_nil
@@ -21,7 +21,7 @@ module Ameba
           Globs: src/*.cr
           CONFIG
         config = Config.new(yml)
-        config.globs.should eq %w(src/*.cr)
+        config.globs.should eq %w[src/*.cr]
       end
 
       it "initializes globs as array" do
@@ -32,7 +32,7 @@ module Ameba
            - "!spec"
           CONFIG
         config = Config.new(yml)
-        config.globs.should eq %w(src/*.cr !spec)
+        config.globs.should eq %w[src/*.cr !spec]
       end
 
       it "raises if Globs has a wrong type" do
@@ -51,7 +51,7 @@ module Ameba
           Excluded: spec
           CONFIG
         config = Config.new(yml)
-        config.excluded.should eq %w(spec)
+        config.excluded.should eq %w[spec]
       end
 
       it "initializes excluded as array" do
@@ -62,7 +62,7 @@ module Ameba
            - lib/*.cr
           CONFIG
         config = Config.new(yml)
-        config.excluded.should eq %w(spec lib/*.cr)
+        config.excluded.should eq %w[spec lib/*.cr]
       end
 
       it "raises if Excluded has a wrong type" do
@@ -84,6 +84,12 @@ module Ameba
         config.formatter.should_not be_nil
       end
 
+      it "raises when custom config file doesn't exist" do
+        expect_raises(Exception, "Unable to load config file: Config file does not exist") do
+          Config.load "foo.yml"
+        end
+      end
+
       it "loads default config" do
         config = Config.load
         config.should_not be_nil
@@ -128,12 +134,12 @@ module Ameba
       end
 
       it "returns a list of sources matching globs" do
-        config.globs = %w(**/config_spec.cr)
+        config.globs = %w[**/config_spec.cr]
         config.sources.size.should eq(1)
       end
 
       it "returns a list of sources excluding 'Excluded'" do
-        config.excluded = %w(**/config_spec.cr)
+        config.excluded = %w[**/config_spec.cr]
         config.sources.any?(&.fullpath.==(__FILE__)).should be_false
       end
     end
@@ -175,7 +181,7 @@ module Ameba
 
       it "updates excluded property" do
         name = DummyRule.rule_name
-        excluded = %w(spec/source.cr)
+        excluded = %w[spec/source.cr]
         config.update_rule name, excluded: excluded
         rule = config.rules.find!(&.name.== name)
         rule.excluded.should eq excluded
@@ -194,7 +200,7 @@ module Ameba
 
       it "updates multiple rules by excluded property" do
         name = DummyRule.rule_name
-        excluded = %w(spec/source.cr)
+        excluded = %w[spec/source.cr]
         config.update_rules [name], excluded: excluded
         rule = config.rules.find!(&.name.== name)
         rule.excluded.should eq excluded
@@ -209,7 +215,7 @@ module Ameba
 
       it "updates a group by excluded property" do
         name = DummyRule.group_name
-        excluded = %w(spec/source.cr)
+        excluded = %w[spec/source.cr]
         config.update_rules [name], excluded: excluded
         rule = config.rules.find!(&.name.== DummyRule.rule_name)
         rule.excluded.should eq excluded

spec/ameba/formatter/todo_formatter_spec.cr

@@ -1,10 +1,12 @@
 require "../../spec_helper"
 require "file_utils"
 
+CONFIG_PATH = Path[Dir.tempdir] / Ameba::Config::FILENAME
+
 module Ameba
   private def with_formatter(&)
     io = IO::Memory.new
-    formatter = Formatter::TODOFormatter.new(io)
+    formatter = Formatter::TODOFormatter.new(io, CONFIG_PATH)
 
     yield formatter, io
   end
@@ -20,7 +22,7 @@ module Ameba
 
   describe Formatter::TODOFormatter do
     ::Spec.after_each do
-      FileUtils.rm_rf(Ameba::Config::DEFAULT_PATH)
+      FileUtils.rm_rf(CONFIG_PATH)
     end
 
     context "problems not found" do
@@ -45,7 +47,7 @@ module Ameba
           s = Source.new "a = 1", "source.cr"
           s.add_issue DummyRule.new, {1, 2}, "message"
           formatter.finished([s])
-          io.to_s.should contain "Created #{Config::DEFAULT_PATH}"
+          io.to_s.should contain "Created #{CONFIG_PATH}"
         end
       end
 

spec/ameba/glob_utils_spec.cr

@@ -1,11 +1,7 @@
 require "../spec_helper"
 
 module Ameba
-  struct GlobUtilsClass
-    include GlobUtils
-  end
-
-  subject = GlobUtilsClass.new
+  subject = GlobUtils
   current_file_basename = File.basename(__FILE__)
   current_file_path = "spec/ameba/#{current_file_basename}"
 
@@ -45,6 +41,12 @@ module Ameba
         subject.expand(["**/#{current_file_basename}", "**/#{current_file_basename}"])
           .should eq [current_file_path]
       end
+
+      it "does not list folders" do
+        subject.expand(["**/*"]).each do |path|
+          fail "#{path.inspect} should be a file" unless File.file?(path)
+        end
+      end
     end
   end
 end

spec/ameba/presenter/rule_collection_presenter_spec.cr

@@ -0,0 +1,32 @@
+require "../../spec_helper"
+
+module Ameba
+  private def with_rule_collection_presenter(&)
+    with_presenter(Presenter::RuleCollectionPresenter) do |presenter, io|
+      rules = Config.load.rules
+      presenter.run(rules)
+
+      output = io.to_s
+      output = Formatter::Util.deansify(output).to_s
+
+      yield rules, output, presenter
+    end
+  end
+
+  describe Presenter::RuleCollectionPresenter do
+    it "outputs rule collection details" do
+      with_rule_collection_presenter do |rules, output|
+        rules.each do |rule|
+          output.should contain rule.name
+          output.should contain rule.severity.symbol
+
+          if description = rule.description
+            output.should contain description
+          end
+        end
+        output.should contain "Total rules: #{rules.size}"
+        output.should match /\d+ enabled/
+      end
+    end
+  end
+end

spec/ameba/presenter/rule_presenter_spec.cr

@@ -0,0 +1,30 @@
+require "../../spec_helper"
+
+module Ameba
+  private def rule_presenter_each_rule(&)
+    with_presenter(Presenter::RulePresenter) do |presenter, io|
+      rules = Config.load.rules
+      rules.each do |rule|
+        presenter.run(rule)
+
+        output = io.to_s
+        output = Formatter::Util.deansify(output).to_s
+
+        yield rule, output, presenter
+      end
+    end
+  end
+
+  describe Presenter::RulePresenter do
+    it "outputs rule details" do
+      rule_presenter_each_rule do |rule, output|
+        output.should contain rule.name
+        output.should contain rule.severity.to_s
+
+        if description = rule.description
+          output.should contain description
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/documentation/documentation_admonition_spec.cr

@@ -0,0 +1,113 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Documentation
+  subject = DocumentationAdmonition.new
+
+  describe DocumentationAdmonition do
+    it "passes for comments with admonition mid-word/sentence" do
+      subject.admonitions.each do |admonition|
+        expect_no_issues subject, <<-CRYSTAL
+          # Mentioning #{admonition} mid-sentence
+          # x#{admonition}x
+          # x#{admonition}
+          # #{admonition}x
+          CRYSTAL
+      end
+    end
+
+    it "fails for comments with admonition" do
+      subject.admonitions.each do |admonition|
+        expect_issue subject, <<-CRYSTAL
+          # #{admonition}: Single-line comment
+          # ^{} error: Found a #{admonition} admonition in a comment
+          CRYSTAL
+
+        expect_issue subject, <<-CRYSTAL
+          # Text before ...
+          # #{admonition}(some context): Part of multi-line comment
+          # ^{} error: Found a #{admonition} admonition in a comment
+          # Text after ...
+          CRYSTAL
+
+        expect_issue subject, <<-CRYSTAL
+          # #{admonition}
+          # ^{} error: Found a #{admonition} admonition in a comment
+          if rand > 0.5
+          end
+          CRYSTAL
+      end
+    end
+
+    context "with date" do
+      it "passes for admonitions with future date" do
+        subject.admonitions.each do |admonition|
+          future_date = (Time.utc + 21.days).to_s(format: "%F")
+          expect_no_issues subject, <<-CRYSTAL
+            # #{admonition}(#{future_date}): sth in the future
+            CRYSTAL
+        end
+      end
+
+      it "fails for admonitions with past date" do
+        subject.admonitions.each do |admonition|
+          past_date = (Time.utc - 21.days).to_s(format: "%F")
+          expect_issue subject, <<-CRYSTAL
+            # #{admonition}(#{past_date}): sth in the past
+            # ^{} error: Found a #{admonition} admonition in a comment (21 days past)
+            CRYSTAL
+        end
+      end
+
+      it "fails for admonitions with yesterday's date" do
+        subject.admonitions.each do |admonition|
+          yesterday_date = (Time.utc - 1.day).to_s(format: "%F")
+          expect_issue subject, <<-CRYSTAL
+            # #{admonition}(#{yesterday_date}): sth in the past
+            # ^{} error: Found a #{admonition} admonition in a comment (1 day past)
+            CRYSTAL
+        end
+      end
+
+      it "fails for admonitions with today's date" do
+        subject.admonitions.each do |admonition|
+          today_date = Time.utc.to_s(format: "%F")
+          expect_issue subject, <<-CRYSTAL
+            # #{admonition}(#{today_date}): sth in the past
+            # ^{} error: Found a #{admonition} admonition in a comment (today is the day!)
+            CRYSTAL
+        end
+      end
+
+      it "fails for admonitions with invalid date" do
+        subject.admonitions.each do |admonition|
+          expect_issue subject, <<-CRYSTAL
+            # #{admonition}(0000-00-00): sth wrong
+            # ^{} error: #{admonition} admonition error: Invalid time: "0000-00-00"
+            CRYSTAL
+        end
+      end
+    end
+
+    context "properties" do
+      describe "#admonitions" do
+        it "lets setting custom admonitions" do
+          rule = DocumentationAdmonition.new
+          rule.admonitions = %w[FOO BAR]
+
+          rule.admonitions.each do |admonition|
+            expect_issue rule, <<-CRYSTAL
+              # #{admonition}
+              # ^{} error: Found a #{admonition} admonition in a comment
+              CRYSTAL
+          end
+
+          subject.admonitions.each do |admonition|
+            expect_no_issues rule, <<-CRYSTAL
+              # #{admonition}
+              CRYSTAL
+          end
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/documentation/documentation_spec.cr

@@ -0,0 +1,151 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Documentation
+  subject = Documentation.new
+    .tap(&.ignore_classes = false)
+    .tap(&.ignore_modules = false)
+    .tap(&.ignore_enums = false)
+    .tap(&.ignore_defs = false)
+    .tap(&.ignore_macros = false)
+
+  describe Documentation do
+    it "passes for undocumented private types" do
+      expect_no_issues subject, <<-CRYSTAL
+        private class Foo
+          def foo
+          end
+        end
+
+        private module Bar
+          def bar
+          end
+        end
+
+        private enum Baz
+        end
+
+        private def bat
+        end
+
+        private macro bag
+        end
+      CRYSTAL
+    end
+
+    it "passes for documented public types" do
+      expect_no_issues subject, <<-CRYSTAL
+        # Foo
+        class Foo
+          # foo
+          def foo
+          end
+        end
+
+        # Bar
+        module Bar
+          # bar
+          def bar
+          end
+        end
+
+        # Baz
+        enum Baz
+        end
+
+        # bat
+        def bat
+        end
+
+        # bag
+        macro bag
+        end
+      CRYSTAL
+    end
+
+    it "fails if there is an undocumented public type" do
+      expect_issue subject, <<-CRYSTAL
+        class Foo
+      # ^^^^^^^^^ error: Missing documentation
+        end
+
+        module Bar
+      # ^^^^^^^^^^ error: Missing documentation
+        end
+
+        enum Baz
+      # ^^^^^^^^ error: Missing documentation
+        end
+
+        def bat
+      # ^^^^^^^ error: Missing documentation
+        end
+
+        macro bag
+      # ^^^^^^^^^ error: Missing documentation
+        end
+      CRYSTAL
+    end
+
+    context "properties" do
+      describe "#ignore_classes" do
+        it "lets the rule to ignore method definitions if true" do
+          rule = Documentation.new
+          rule.ignore_classes = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            class Foo
+            end
+            CRYSTAL
+        end
+      end
+
+      describe "#ignore_modules" do
+        it "lets the rule to ignore method definitions if true" do
+          rule = Documentation.new
+          rule.ignore_modules = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            module Bar
+            end
+            CRYSTAL
+        end
+      end
+
+      describe "#ignore_enums" do
+        it "lets the rule to ignore method definitions if true" do
+          rule = Documentation.new
+          rule.ignore_enums = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            enum Baz
+            end
+            CRYSTAL
+        end
+      end
+
+      describe "#ignore_defs" do
+        it "lets the rule to ignore method definitions if true" do
+          rule = Documentation.new
+          rule.ignore_defs = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            def bat
+            end
+            CRYSTAL
+        end
+      end
+
+      describe "#ignore_macros" do
+        it "lets the rule to ignore macros if true" do
+          rule = Documentation.new
+          rule.ignore_macros = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            macro bag
+            end
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/lint/empty_expression_spec.cr

@@ -4,16 +4,16 @@ module Ameba
   subject = Rule::Lint::EmptyExpression.new
 
   private def it_detects_empty_expression(code, *, file = __FILE__, line = __LINE__)
-    it %(detects empty expression "#{code}"), file, line do
-      s = Source.new code
+    it "detects empty expression #{code.inspect}", file, line do
+      source = Source.new code
       rule = Rule::Lint::EmptyExpression.new
-      rule.catch(s).should_not be_valid, file: file, line: line
+      rule.catch(source).should_not be_valid, file: file, line: line
     end
   end
 
   describe Rule::Lint::EmptyExpression do
     it "passes if there is no empty expression" do
-      s = Source.new <<-CRYSTAL
+      expect_no_issues subject, <<-CRYSTAL
         def method()
         end
 
@@ -31,7 +31,6 @@ module Ameba
         begin "" end
         [nil] << nil
         CRYSTAL
-      subject.catch(s).should be_valid
     end
 
     it_detects_empty_expression %(())
@@ -91,10 +90,10 @@ module Ameba
     )
 
     it "does not report empty expression in macro" do
-      s = Source.new %q(
+      expect_no_issues subject, <<-CRYSTAL
         module MyModule
           macro conditional_error_for_inline_callbacks
-            \{%
+            \\{%
               raise ""
             %}
           end
@@ -102,8 +101,7 @@ module Ameba
           macro before_save(x = nil)
           end
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
   end
 end

spec/ameba/rule/lint/literals_comparison_spec.cr

@@ -6,8 +6,12 @@ module Ameba::Rule::Lint
   describe LiteralsComparison do
     it "passes for valid cases" do
       expect_no_issues subject, <<-CRYSTAL
+        {start.year, start.month} == {stop.year, stop.month}
+        ["foo"] === [foo]
         "foo" == foo
         "foo" != foo
+        "foo" == FOO
+        FOO == "foo"
         foo == "foo"
         foo != "foo"
         CRYSTAL
@@ -15,8 +19,8 @@ module Ameba::Rule::Lint
 
     it "reports if there is a dynamic comparison possibly evaluating to the same" do
       expect_issue subject, <<-CRYSTAL
-        [foo] === ["foo"]
-        # ^^^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same
+        [foo] === [foo]
+        # ^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same
         CRYSTAL
     end
 

spec/ameba/rule/lint/not_nil_after_no_bang_spec.cr

@@ -7,8 +7,11 @@ module Ameba::Rule::Lint
     it "passes for valid cases" do
       expect_no_issues subject, <<-CRYSTAL
         (1..3).index(1).not_nil!(:foo)
+        (1..3).rindex(1).not_nil!(:foo)
         (1..3).index { |i| i > 2 }.not_nil!(:foo)
+        (1..3).rindex { |i| i > 2 }.not_nil!(:foo)
         (1..3).find { |i| i > 2 }.not_nil!(:foo)
+        /(.)(.)(.)/.match("abc", &.itself).not_nil!
         CRYSTAL
     end
 
@@ -23,6 +26,28 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
+    it "reports if there is an `rindex` call followed by `not_nil!`" do
+      source = expect_issue subject, <<-CRYSTAL
+        (1..3).rindex(1).not_nil!
+             # ^^^^^^^^^^^^^^^^^^ error: Use `rindex! {...}` instead of `rindex {...}.not_nil!`
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        (1..3).rindex!(1)
+        CRYSTAL
+    end
+
+    it "reports if there is an `match` call followed by `not_nil!`" do
+      source = expect_issue subject, <<-CRYSTAL
+        /(.)(.)(.)/.match("abc").not_nil![2]
+                  # ^^^^^^^^^^^^^^^^^^^^^ error: Use `match! {...}` instead of `match {...}.not_nil!`
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        /(.)(.)(.)/.match!("abc")[2]
+        CRYSTAL
+    end
+
     it "reports if there is an `index` call with block followed by `not_nil!`" do
       source = expect_issue subject, <<-CRYSTAL
         (1..3).index { |i| i > 2 }.not_nil!
@@ -34,6 +59,17 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
+    it "reports if there is an `rindex` call with block followed by `not_nil!`" do
+      source = expect_issue subject, <<-CRYSTAL
+        (1..3).rindex { |i| i > 2 }.not_nil!
+             # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `rindex! {...}` instead of `rindex {...}.not_nil!`
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        (1..3).rindex! { |i| i > 2 }
+        CRYSTAL
+    end
+
     it "reports if there is a `find` call with block followed by `not_nil!`" do
       source = expect_issue subject, <<-CRYSTAL
         (1..3).find { |i| i > 2 }.not_nil!

spec/ameba/rule/lint/percent_arrays_spec.cr

@@ -6,41 +6,41 @@ module Ameba::Rule::Lint
 
     it "passes if percent arrays are written correctly" do
       s = Source.new %q(
-        %i(one two three)
-        %w(one two three)
+        %i[one two three]
+        %w[one two three]
 
-        %i(1 2 3)
-        %w(1 2 3)
+        %i[1 2 3]
+        %w[1 2 3]
 
-        %i()
-        %w()
+        %i[]
+        %w[]
       )
       subject.catch(s).should be_valid
     end
 
     it "fails if string percent array has commas" do
-      s = Source.new %( %w(one, two) )
+      s = Source.new %( %w[one, two] )
       subject.catch(s).should_not be_valid
     end
 
     it "fails if string percent array has quotes" do
-      s = Source.new %( %w("one" "two") )
+      s = Source.new %( %w["one" "two"] )
       subject.catch(s).should_not be_valid
     end
 
     it "fails if symbols percent array has commas" do
-      s = Source.new %( %i(one, two) )
+      s = Source.new %( %i[one, two] )
       subject.catch(s).should_not be_valid
     end
 
     it "fails if symbols percent array has a colon" do
-      s = Source.new %( %i(:one :two) )
+      s = Source.new %( %i[:one :two] )
       subject.catch(s).should_not be_valid
     end
 
     it "reports rule, location and message for %i" do
       s = Source.new %(
-        %i(:one)
+        %i[:one]
       ), "source.cr"
 
       subject.catch(s).should_not be_valid
@@ -54,7 +54,7 @@ module Ameba::Rule::Lint
 
     it "reports rule, location and message for %w" do
       s = Source.new %(
-        %w("one")
+        %w["one"]
       ), "source.cr"
 
       subject.catch(s).should_not be_valid
@@ -71,14 +71,14 @@ module Ameba::Rule::Lint
       it "#string_array_unwanted_symbols" do
         rule = PercentArrays.new
         rule.string_array_unwanted_symbols = ","
-        s = Source.new %( %w("one") )
+        s = Source.new %( %w["one"] )
         rule.catch(s).should be_valid
       end
 
       it "#symbol_array_unwanted_symbols" do
         rule = PercentArrays.new
         rule.symbol_array_unwanted_symbols = ","
-        s = Source.new %( %i(:one) )
+        s = Source.new %( %i[:one] )
         rule.catch(s).should be_valid
       end
     end

spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr

@@ -31,6 +31,30 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
+    pending "reports if there is a shadowing in an unpacked variable in a block" do
+      expect_issue subject, <<-CRYSTAL
+        def some_method
+          foo = 1
+
+          [{3}].each do |(foo)|
+                        # ^ error: Shadowing outer local variable `foo`
+          end
+        end
+        CRYSTAL
+    end
+
+    pending "reports if there is a shadowing in an unpacked variable in a block (2)" do
+      expect_issue subject, <<-CRYSTAL
+        def some_method
+          foo = 1
+
+          [{[3]}].each do |((foo))|
+                           # ^ error: Shadowing outer local variable `foo`
+          end
+        end
+        CRYSTAL
+    end
+
     it "does not report outer vars declared below shadowed block" do
       expect_no_issues subject, <<-CRYSTAL
         methods = klass.methods.select { |m| m.annotation(MyAnn) }
@@ -44,7 +68,7 @@ module Ameba::Rule::Lint
           foo = 1
 
           -> (foo : Int32) {}
-            # ^ error: Shadowing outer local variable `foo`
+            # ^^^^^^^^^^^ error: Shadowing outer local variable `foo`
         end
         CRYSTAL
     end
@@ -69,7 +93,7 @@ module Ameba::Rule::Lint
         3.times do |foo|
                   # ^ error: Shadowing outer local variable `foo`
           -> (foo : Int32) { foo + 1 }
-            # ^ error: Shadowing outer local variable `foo`
+            # ^^^^^^^^^^^ error: Shadowing outer local variable `foo`
         end
         CRYSTAL
     end

spec/ameba/rule/lint/shared_var_in_fiber_spec.cr

@@ -39,7 +39,7 @@ module Ameba::Rule::Lint
         CRYSTAL
     end
 
-    it "reports if there is a shared var in spawn" do
+    it "reports if there is a shared var in spawn (while)" do
       source = expect_issue subject, <<-CRYSTAL
         i = 0
         while i < 10
@@ -56,6 +56,24 @@ module Ameba::Rule::Lint
       expect_no_corrections source
     end
 
+    it "reports if there is a shared var in spawn (loop)" do
+      source = expect_issue subject, <<-CRYSTAL
+        i = 0
+        loop do
+          break if i >= 10
+          spawn do
+            puts(i)
+               # ^ error: Shared variable `i` is used in fiber
+          end
+          i += 1
+        end
+
+        Fiber.yield
+        CRYSTAL
+
+      expect_no_corrections source
+    end
+
     it "reports reassigned reference to shared var in spawn" do
       source = expect_issue subject, <<-CRYSTAL
         channel = Channel(String).new

spec/ameba/rule/lint/spec_filename_spec.cr

@@ -0,0 +1,46 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  subject = SpecFilename.new
+
+  describe SpecFilename do
+    it "passes if relative file path does not start with `spec/`" do
+      expect_no_issues subject, code: "", path: "src/spec/foo.cr"
+      expect_no_issues subject, code: "", path: "src/spec/foo/bar.cr"
+    end
+
+    it "passes if file extension is not `.cr`" do
+      expect_no_issues subject, code: "", path: "spec/foo.json"
+      expect_no_issues subject, code: "", path: "spec/foo/bar.json"
+    end
+
+    it "passes if filename is correct" do
+      expect_no_issues subject, code: "", path: "spec/foo_spec.cr"
+      expect_no_issues subject, code: "", path: "spec/foo/bar_spec.cr"
+    end
+
+    it "fails if filename is wrong" do
+      expect_issue subject, <<-CRYSTAL, path: "spec/foo.cr"
+
+        # ^{} error: Spec filename should have `_spec` suffix: foo_spec.cr, not foo.cr
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#ignored_dirs" do
+        it "provide sane defaults" do
+          expect_no_issues subject, code: "", path: "spec/support/foo.cr"
+          expect_no_issues subject, code: "", path: "spec/fixtures/foo.cr"
+          expect_no_issues subject, code: "", path: "spec/data/foo.cr"
+        end
+      end
+
+      context "#ignored_filenames" do
+        it "ignores spec_helper by default" do
+          expect_no_issues subject, code: "", path: "spec/spec_helper.cr"
+          expect_no_issues subject, code: "", path: "spec/foo/spec_helper.cr"
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/lint/spec_focus_spec.cr

@@ -115,12 +115,12 @@ module Ameba::Rule::Lint
 
       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.end_location.to_s.should eq "source_spec.cr:1:21"
       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.end_location.to_s.should eq "source_spec.cr:2:23"
       second.message.should eq "Focused spec item detected"
     end
   end

spec/ameba/rule/lint/typos_spec.cr

@@ -0,0 +1,35 @@
+require "../../../spec_helper"
+
+private def check_typos_bin!
+  unless Ameba::Rule::Lint::Typos::BIN_PATH
+    pending! "`typos` executable is not available"
+  end
+end
+
+module Ameba::Rule::Lint
+  subject = Typos.new
+    .tap(&.fail_on_error = true)
+
+  describe Typos do
+    it "reports typos" do
+      check_typos_bin!
+
+      source = expect_issue subject, <<-CRYSTAL
+        # method with no arugments
+                       # ^^^^^^^^^ error: Typo found: arugments -> arguments
+        def tpos
+          # ^^^^ error: Typo found: tpos -> typos
+          :otput
+         # ^^^^^ error: Typo found: otput -> output
+        end
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        # method with no arguments
+        def typos
+          :output
+        end
+        CRYSTAL
+    end
+  end
+end

spec/ameba/rule/lint/unused_argument_spec.cr

@@ -52,7 +52,7 @@ module Ameba::Rule::Lint
     it "reports if proc argument is unused" do
       source = expect_issue subject, <<-CRYSTAL
         -> (a : Int32, b : String) do
-                     # ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used.
+                     # ^^^^^^^^^^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used.
           a = a + 1
         end
         CRYSTAL
@@ -306,7 +306,7 @@ module Ameba::Rule::Lint
 
           expect_issue rule, <<-CRYSTAL
             ->(a : Int32) {}
-             # ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used.
+             # ^^^^^^^^^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used.
             CRYSTAL
         end
       end

spec/ameba/rule/naming/accessor_method_name_spec.cr

@@ -0,0 +1,93 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = AccessorMethodName.new
+
+  describe AccessorMethodName do
+    it "passes if accessor method name is correct" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def self.instance
+          end
+
+          def self.instance=(value)
+          end
+
+          def user
+          end
+
+          def user=(user)
+          end
+        end
+        CRYSTAL
+    end
+
+    it "passes if accessor method is defined in top-level scope" do
+      expect_no_issues subject, <<-CRYSTAL
+        def get_user
+        end
+
+        def set_user(user)
+        end
+        CRYSTAL
+    end
+
+    it "fails if accessor method is defined with receiver in top-level scope" do
+      expect_issue subject, <<-CRYSTAL
+        def Foo.get_user
+              # ^^^^^^^^ error: Favour method name 'user' over 'get_user'
+        end
+
+        def Foo.set_user(user)
+              # ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
+        end
+        CRYSTAL
+    end
+
+    it "fails if accessor method name is wrong" do
+      expect_issue subject, <<-CRYSTAL
+        class Foo
+          def self.get_instance
+                 # ^^^^^^^^^^^^ error: Favour method name 'instance' over 'get_instance'
+          end
+
+          def self.set_instance(value)
+                 # ^^^^^^^^^^^^ error: Favour method name 'instance=' over 'set_instance'
+          end
+
+          def get_user
+            # ^^^^^^^^ error: Favour method name 'user' over 'get_user'
+          end
+
+          def set_user(user)
+            # ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
+          end
+        end
+        CRYSTAL
+    end
+
+    it "ignores if alternative name isn't valid syntax" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def get_404
+          end
+
+          def set_404(value)
+          end
+        end
+        CRYSTAL
+    end
+
+    it "ignores if the method has unexpected arity" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def get_user(type)
+          end
+
+          def set_user(user, type)
+          end
+        end
+        CRYSTAL
+    end
+  end
+end

spec/ameba/rule/naming/ascii_identifiers_spec.cr

@@ -0,0 +1,151 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = AsciiIdentifiers.new
+
+  describe AsciiIdentifiers do
+    it "reports classes with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        class BigAwesome🐺
+            # ^^^^^^^^^^^ error: Identifier contains non-ascii characters
+          @🐺_name : String
+        # ^^^^^^^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports modules with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        module Bąk
+             # ^^^ error: Identifier contains non-ascii characters
+          @@bąk_name : String
+        # ^^^^^^^^^^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports enums with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        enum TypeOf🔥
+           # ^^^^^^^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports defs with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        def łódź
+          # ^^^^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports defs with parameter names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        def forest_adventure(include_🐺 = true, include_🐿 = true)
+                           # ^ error: Identifier contains non-ascii characters
+                                             # ^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports defs with parameter default values containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        def forest_adventure(animal_type = :🐺)
+                                         # ^^ error: Identifier contains non-ascii characters
+        end
+        CRYSTAL
+    end
+
+    it "reports argument names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        %w[wensleydale cheddar brie].each { |🧀| nil }
+                                           # ^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports calls with arguments containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        %i[🐺 🐿].index!(:🐺)
+                     # ^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports calls with named arguments containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        %i[🐺 🐿].index!(obj: :🐺)
+                          # ^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports aliases with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        alias JSON🧀 = JSON::Any
+            # ^^^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports constants with names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        I_LOVE_🍣 = true
+        # ^^^^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports assignments with variable names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        space_👾 = true
+        # ^^^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports multiple assignments with variable names containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        foo, space_👾 = true, true
+           # ^^^^^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports assignments with symbol literals containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        foo = :신장
+            # ^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "reports multiple assignments with symbol literals containing non-ascii characters" do
+      expect_issue subject, <<-CRYSTAL
+        foo, bar = :신장, true
+                 # ^^^ error: Identifier contains non-ascii characters
+        CRYSTAL
+    end
+
+    it "passes for strings with non-ascii characters" do
+      expect_no_issues subject, <<-CRYSTAL
+        space = "👾"
+        space = :invader # 👾
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#ignore_symbols" do
+        it "returns `false` by default" do
+          rule = AsciiIdentifiers.new
+          rule.ignore_symbols?.should be_false
+        end
+
+        it "stops reporting symbol literals if set to `true`" do
+          rule = AsciiIdentifiers.new
+          rule.ignore_symbols = true
+
+          expect_no_issues rule, <<-CRYSTAL
+            def forest_adventure(animal_type = :🐺); end
+            %i[🐺 🐿].index!(:🐺)
+            foo, bar = :신장, true
+            foo = :신장
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/naming/binary_operator_parameter_name_spec.cr

@@ -0,0 +1,50 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = BinaryOperatorParameterName.new
+
+  describe BinaryOperatorParameterName do
+    it "ignores `other` parameter name in binary method definitions" do
+      expect_no_issues subject, <<-CRYSTAL
+        def +(other); end
+        def -(other); end
+        def *(other); end
+        CRYSTAL
+    end
+
+    it "ignores binary method definitions with arity other than 1" do
+      expect_no_issues subject, <<-CRYSTAL
+        def +; end
+        def +(foo, bar); end
+        def -; end
+        def -(foo, bar); end
+        CRYSTAL
+    end
+
+    it "ignores non-binary method definitions" do
+      expect_no_issues subject, <<-CRYSTAL
+        def foo(bar); end
+        def bąk(genus); end
+        CRYSTAL
+    end
+
+    it "reports binary methods definitions with incorrectly named parameter" do
+      expect_issue subject, <<-CRYSTAL
+        def +(foo); end
+            # ^ error: When defining the `+` operator, name its argument `other`
+        def -(foo); end
+            # ^ error: When defining the `-` operator, name its argument `other`
+        def *(foo); end
+            # ^ error: When defining the `*` operator, name its argument `other`
+        CRYSTAL
+    end
+
+    it "ignores methods from #excluded_operators" do
+      subject.excluded_operators.each do |op|
+        expect_no_issues subject, <<-CRYSTAL
+          def #{op}(foo); end
+          CRYSTAL
+      end
+    end
+  end
+end

spec/ameba/rule/naming/block_parameter_name_spec.cr

@@ -0,0 +1,100 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = BlockParameterName.new
+    .tap(&.min_name_length = 3)
+    .tap(&.allowed_names = %w[_ e i j k v])
+
+  describe BlockParameterName do
+    it "passes if block parameter name matches #allowed_names" do
+      subject.allowed_names.each do |name|
+        expect_no_issues subject, <<-CRYSTAL
+          %w[].each { |#{name}| }
+          CRYSTAL
+      end
+    end
+
+    it "fails if block parameter name doesn't match #allowed_names" do
+      expect_issue subject, <<-CRYSTAL
+        %w[].each { |x| }
+                   # ^ error: Disallowed block parameter name found
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#min_name_length" do
+        it "allows setting custom values" do
+          rule = BlockParameterName.new
+          rule.allowed_names = %w[a b c]
+
+          rule.min_name_length = 3
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.min_name_length = 1
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+
+      context "#allow_names_ending_in_numbers" do
+        it "allows setting custom values" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 1
+          rule.allowed_names = %w[]
+
+          rule.allow_names_ending_in_numbers = false
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x1| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.allow_names_ending_in_numbers = true
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x1| }
+            CRYSTAL
+        end
+      end
+
+      context "#allowed_names" do
+        it "allows setting custom names" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 3
+
+          rule.allowed_names = %w[a b c]
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.allowed_names = %w[x y z]
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+
+      context "#forbidden_names" do
+        it "allows setting custom names" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 1
+          rule.allowed_names = %w[]
+
+          rule.forbidden_names = %w[x y z]
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.forbidden_names = %w[a b c]
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/naming/constant_names_spec.cr

@@ -1,11 +1,11 @@
 require "../../../spec_helper"
 
 module Ameba
-  subject = Rule::Style::ConstantNames.new
+  subject = Rule::Naming::ConstantNames.new
 
   private def it_reports_constant(name, value, expected, *, file = __FILE__, line = __LINE__)
     it "reports constant name #{expected}", file, line do
-      rule = Rule::Style::ConstantNames.new
+      rule = Rule::Naming::ConstantNames.new
       expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line
           %{name} = #{value}
         # ^{name} error: Constant name should be screaming-cased: #{expected}, not #{name}
@@ -13,7 +13,7 @@ module Ameba
     end
   end
 
-  describe Rule::Style::ConstantNames do
+  describe Rule::Naming::ConstantNames do
     it "passes if type names are screaming-cased" do
       expect_no_issues subject, <<-CRYSTAL
         LUCKY_NUMBERS     = [3, 7, 11]

spec/ameba/rule/naming/filename_spec.cr

@@ -0,0 +1,19 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = Filename.new
+
+  describe Filename do
+    it "passes if filename is correct" do
+      expect_no_issues subject, code: "", path: "src/foo.cr"
+      expect_no_issues subject, code: "", path: "src/foo_bar.cr"
+    end
+
+    it "fails if filename is wrong" do
+      expect_issue subject, <<-CRYSTAL, path: "src/fooBar.cr"
+
+        # ^{} error: Filename should be underscore-cased: foo_bar.cr, not fooBar.cr
+        CRYSTAL
+    end
+  end
+end

spec/ameba/rule/naming/method_names_spec.cr

@@ -1,11 +1,11 @@
 require "../../../spec_helper"
 
 module Ameba
-  subject = Rule::Style::MethodNames.new
+  subject = Rule::Naming::MethodNames.new
 
   private def it_reports_method_name(name, expected, *, file = __FILE__, line = __LINE__)
     it "reports method name #{expected}", file, line do
-      rule = Rule::Style::MethodNames.new
+      rule = Rule::Naming::MethodNames.new
       expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line
         def %{name}; end
           # ^{name} error: Method name should be underscore-cased: #{expected}, not %{name}
@@ -13,7 +13,7 @@ module Ameba
     end
   end
 
-  describe Rule::Style::MethodNames do
+  describe Rule::Naming::MethodNames do
     it "passes if method names are underscore-cased" do
       expect_no_issues subject, <<-CRYSTAL
         class Person

spec/ameba/rule/naming/predicate_name_spec.cr

@@ -1,6 +1,6 @@
 require "../../../spec_helper"
 
-module Ameba::Rule::Style
+module Ameba::Rule::Naming
   subject = PredicateName.new
 
   describe PredicateName do
@@ -21,8 +21,18 @@ module Ameba::Rule::Style
 
     it "fails if predicate name is wrong" do
       expect_issue subject, <<-CRYSTAL
+        class Image
+          def self.is_valid?(x)
+                 # ^^^^^^^^^ error: Favour method name 'valid?' over 'is_valid?'
+          end
+        end
+
         def is_valid?(x)
-        # ^^^^^^^^^^^^^^ error: Favour method name 'valid?' over 'is_valid?'
+          # ^^^^^^^^^ error: Favour method name 'valid?' over 'is_valid?'
+        end
+
+        def is_valid(x)
+          # ^^^^^^^^ error: Favour method name 'valid?' over 'is_valid'
         end
         CRYSTAL
     end

spec/ameba/rule/naming/query_bool_methods_spec.cr

@@ -1,6 +1,6 @@
 require "../../../spec_helper"
 
-module Ameba::Rule::Style
+module Ameba::Rule::Naming
   subject = QueryBoolMethods.new
 
   describe QueryBoolMethods do

spec/ameba/rule/naming/rescued_exceptions_variable_name_spec.cr

@@ -0,0 +1,53 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = RescuedExceptionsVariableName.new
+
+  describe RescuedExceptionsVariableName do
+    it "passes if exception handler variable name matches #allowed_names" do
+      subject.allowed_names.each do |name|
+        expect_no_issues subject, <<-CRYSTAL
+          def foo
+            raise "foo"
+          rescue #{name}
+            nil
+          end
+          CRYSTAL
+      end
+    end
+
+    it "fails if exception handler variable name doesn't match #allowed_names" do
+      expect_issue subject, <<-CRYSTAL
+        def foo
+          raise "foo"
+        rescue wtf
+        # ^^^^^^^^ error: Disallowed variable name, use one of these instead: 'e', 'ex', 'exception', 'error'
+          nil
+        end
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#allowed_names" do
+        it "returns sensible defaults" do
+          rule = RescuedExceptionsVariableName.new
+          rule.allowed_names.should eq %w[e ex exception error]
+        end
+
+        it "allows setting custom names" do
+          rule = RescuedExceptionsVariableName.new
+          rule.allowed_names = %w[foo]
+
+          expect_issue rule, <<-CRYSTAL
+            def foo
+              raise "foo"
+            rescue e
+            # ^^^^^^ error: Disallowed variable name, use 'foo' instead
+              nil
+            end
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

spec/ameba/rule/naming/type_names_spec.cr

@@ -1,19 +1,19 @@
 require "../../../spec_helper"
 
 module Ameba
-  subject = Rule::Style::TypeNames.new
+  subject = Rule::Naming::TypeNames.new
 
   private def it_reports_name(type, name, expected, *, file = __FILE__, line = __LINE__)
     it "reports type name #{expected}", file, line do
-      rule = Rule::Style::TypeNames.new
+      rule = Rule::Naming::TypeNames.new
       expect_issue rule, <<-CRYSTAL, type: type, name: name, file: file, line: line
-        %{type} %{name}; end
-        # ^{type}^{name}^^^^ error: Type name should be camelcased: #{expected}, but it was %{name}
+        %{type}   %{name}; end
+        _{type} # ^{name} error: Type name should be camelcased: #{expected}, but it was %{name}
         CRYSTAL
     end
   end
 
-  describe Rule::Style::TypeNames do
+  describe Rule::Naming::TypeNames do
     it "passes if type names are camelcased" do
       expect_no_issues subject, <<-CRYSTAL
         class ParseError < Exception
@@ -46,7 +46,7 @@ module Ameba
     it "reports alias name" do
       expect_issue subject, <<-CRYSTAL
         alias Numeric_value = Int32
-        # ^{} error: Type name should be camelcased: NumericValue, but it was Numeric_value
+            # ^^^^^^^^^^^^^ error: Type name should be camelcased: NumericValue, but it was Numeric_value
         CRYSTAL
     end
   end

spec/ameba/rule/naming/variable_names_spec.cr

@@ -1,11 +1,11 @@
 require "../../../spec_helper"
 
 module Ameba
-  subject = Rule::Style::VariableNames.new
+  subject = Rule::Naming::VariableNames.new
 
   private def it_reports_var_name(name, value, expected, *, file = __FILE__, line = __LINE__)
     it "reports variable name #{expected}", file, line do
-      rule = Rule::Style::VariableNames.new
+      rule = Rule::Naming::VariableNames.new
       expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line
           %{name} = #{value}
         # ^{name} error: Var name should be underscore-cased: #{expected}, not %{name}
@@ -13,7 +13,7 @@ module Ameba
     end
   end
 
-  describe Rule::Style::VariableNames do
+  describe Rule::Naming::VariableNames do
     it "passes if var names are underscore-cased" do
       expect_no_issues subject, <<-CRYSTAL
         class Greeting

spec/ameba/rule/performance/any_after_filter_spec.cr

@@ -48,7 +48,7 @@ module Ameba::Rule::Performance
     context "properties" do
       it "#filter_names" do
         rule = AnyAfterFilter.new
-        rule.filter_names = %w(select)
+        rule.filter_names = %w[select]
 
         expect_no_issues rule, <<-CRYSTAL
           [1, 2, 3].reject { |e| e > 2 }.any?

spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr

@@ -46,7 +46,7 @@ module Ameba::Rule::Performance
     context "properties" do
       it "#call_names" do
         rule = ChainedCallWithNoBang.new
-        rule.call_names = %w(uniq)
+        rule.call_names = %w[uniq]
 
         expect_no_issues rule, <<-CRYSTAL
           [1, 2, 3].select { |e| e > 2 }.reverse

spec/ameba/rule/performance/excessive_allocations_spec.cr

@@ -0,0 +1,57 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+  subject = ExcessiveAllocations.new
+
+  describe ExcessiveAllocations do
+    it "passes if there is no potential performance improvements" do
+      expect_no_issues subject, <<-CRYSTAL
+        "Alice".chars.each(arg) { |c| puts c }
+        "Alice".chars(arg).each { |c| puts c }
+        "Alice\nBob".lines.each(arg) { |l| puts l }
+        "Alice\nBob".lines(arg).each { |l| puts l }
+        CRYSTAL
+    end
+
+    it "reports if there is a collection method followed by each" do
+      source = expect_issue subject, <<-CRYSTAL
+        "Alice".chars.each { |c| puts c }
+              # ^^^^^^^^^^ error: Use `each_char {...}` instead of `chars.each {...}` to avoid excessive allocation
+        "Alice\nBob".lines.each { |l| puts l }
+           # ^^^^^^^^^^ error: Use `each_line {...}` instead of `lines.each {...}` to avoid excessive allocation
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        "Alice".each_char { |c| puts c }
+        "Alice\nBob".each_line { |l| puts l }
+        CRYSTAL
+    end
+
+    it "does not report if source is a spec" do
+      expect_no_issues subject, <<-CRYSTAL, "source_spec.cr"
+        "Alice".chars.each { |c| puts c }
+        CRYSTAL
+    end
+
+    context "properties" do
+      it "#call_names" do
+        rule = ExcessiveAllocations.new
+        rule.call_names = {
+          "children" => "each_child",
+        }
+
+        expect_no_issues rule, <<-CRYSTAL
+          "Alice".chars.each { |c| puts c }
+          CRYSTAL
+      end
+    end
+
+    context "macro" do
+      it "doesn't report in macro scope" do
+        expect_no_issues subject, <<-CRYSTAL
+          {{ "Alice".chars.each { |c| puts c } }}
+          CRYSTAL
+      end
+    end
+  end
+end

spec/ameba/rule/performance/first_last_after_filter_spec.cr

@@ -64,7 +64,7 @@ module Ameba::Rule::Performance
     context "properties" do
       it "#filter_names" do
         rule = FirstLastAfterFilter.new
-        rule.filter_names = %w(reject)
+        rule.filter_names = %w[reject]
 
         expect_no_issues rule, <<-CRYSTAL
           [1, 2, 3].select { |e| e > 2 }.first

spec/ameba/rule/performance/minmax_after_map_spec.cr

@@ -0,0 +1,45 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+  subject = MinMaxAfterMap.new
+
+  describe MinMaxAfterMap do
+    it "passes if there are no potential performance improvements" do
+      expect_no_issues subject, <<-CRYSTAL
+        %w[Alice Bob].map { |name| name.size }.min(2)
+        %w[Alice Bob].map { |name| name.size }.max(2)
+        CRYSTAL
+    end
+
+    it "reports if there is a `min/max/minmax` call followed by `map`" do
+      source = expect_issue subject, <<-CRYSTAL
+        %w[Alice Bob].map { |name| name.size }.min
+                    # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `min_of {...}` instead of `map {...}.min`.
+        %w[Alice Bob].map(&.size).max.zero?
+                    # ^^^^^^^^^^^^^^^ error: Use `max_of {...}` instead of `map {...}.max`.
+        %w[Alice Bob].map(&.size).minmax?
+                    # ^^^^^^^^^^^^^^^^^^^ error: Use `minmax_of? {...}` instead of `map {...}.minmax?`.
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        %w[Alice Bob].min_of { |name| name.size }
+        %w[Alice Bob].max_of(&.size).zero?
+        %w[Alice Bob].minmax_of?(&.size)
+        CRYSTAL
+    end
+
+    it "does not report if source is a spec" do
+      expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL
+        %w[Alice Bob].map(&.size).min
+        CRYSTAL
+    end
+
+    context "macro" do
+      it "doesn't report in macro scope" do
+        expect_no_issues subject, <<-CRYSTAL
+          {{ %w[Alice Bob].map(&.size).min }}
+          CRYSTAL
+      end
+    end
+  end
+end

spec/ameba/rule/performance/size_after_filter_spec.cr

@@ -46,7 +46,7 @@ module Ameba::Rule::Performance
     context "properties" do
       it "#filter_names" do
         rule = SizeAfterFilter.new
-        rule.filter_names = %w(select)
+        rule.filter_names = %w[select]
 
         expect_no_issues rule, <<-CRYSTAL
           [1, 2, 3].reject(&.empty?).size

spec/ameba/rule/style/is_a_filter_spec.cr

@@ -44,7 +44,7 @@ module Ameba::Rule::Style
     context "properties" do
       it "#filter_names" do
         rule = IsAFilter.new
-        rule.filter_names = %w(select)
+        rule.filter_names = %w[select]
 
         expect_no_issues rule, <<-CRYSTAL
           [1, 2, nil].reject(&.nil?)

spec/ameba/rule/style/large_numbers_spec.cr

@@ -123,7 +123,7 @@ module Ameba
       it "#int_min_digits" do
         rule = Rule::Style::LargeNumbers.new
         rule.int_min_digits = 10
-        expect_no_issues rule, %q(1200000)
+        expect_no_issues rule, "1200000"
       end
     end
   end

spec/ameba/rule/style/parentheses_around_condition_spec.cr

@@ -4,7 +4,7 @@ module Ameba::Rule::Style
   subject = ParenthesesAroundCondition.new
 
   describe ParenthesesAroundCondition do
-    {% for keyword in %w(if unless while until) %}
+    {% for keyword in %w[if unless while until] %}
       context "{{ keyword.id }}" do
         it "reports if redundant parentheses are found" do
           source = expect_issue subject, <<-CRYSTAL, keyword: {{ keyword }}

spec/ameba/rule/style/verbose_block_spec.cr

@@ -25,6 +25,7 @@ module Ameba::Rule::Style
         (1..3).map { |l| l.to_i64 * l.to_i64 }
         (1..3).map { |m| m.to_s[start: m.to_i64, count: 3]? }
         (1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join }
+        (1..3).map { |o| o.foo = foos[o.abs]? || 0 }
         CRYSTAL
     end
 

spec/ameba/runner_spec.cr

@@ -95,7 +95,7 @@ module Ameba
         end
 
         it "does not run other rules" do
-          rules = [Rule::Lint::Syntax.new, Rule::Style::ConstantNames.new] of Rule::Base
+          rules = [Rule::Lint::Syntax.new, Rule::Naming::ConstantNames.new] of Rule::Base
           source = Source.new <<-CRYSTAL
             MyBadConstant = 1
 

spec/ameba/severity_spec.cr

@@ -45,7 +45,7 @@ module Ameba
     include YAML::Serializable
 
     @[YAML::Field(converter: Ameba::SeverityYamlConverter)]
-    property severity : Severity
+    property severity : Severity?
   end
 
   describe SeverityYamlConverter do

spec/ameba/tokenizer_spec.cr

@@ -22,23 +22,23 @@ module Ameba
         DELIMITER_START STRING INTERPOLATION_START NUMBER } DELIMITER_END EOF
       )
 
-      it_tokenizes %(%w(1 2)),
-        %w(STRING_ARRAY_START STRING STRING STRING_ARRAY_END EOF)
+      it_tokenizes %(%w[1 2]),
+        %w[STRING_ARRAY_START STRING STRING STRING_ARRAY_END EOF]
 
-      it_tokenizes %(%i(one two)),
-        %w(SYMBOL_ARRAY_START STRING STRING STRING_ARRAY_END EOF)
+      it_tokenizes %(%i[one two]),
+        %w[SYMBOL_ARRAY_START STRING STRING STRING_ARRAY_END EOF]
 
       it_tokenizes %(
-          class A
-            def method
-              puts "hello"
-            end
+        class A
+          def method
+            puts "hello"
           end
-      ), %w(
+        end
+      ), %w[
         NEWLINE SPACE IDENT SPACE CONST NEWLINE SPACE IDENT SPACE IDENT
         NEWLINE SPACE IDENT SPACE DELIMITER_START STRING DELIMITER_END
         NEWLINE SPACE IDENT NEWLINE SPACE IDENT NEWLINE SPACE EOF
-      )
+      ]
     end
   end
 end

spec/fixtures/config.yml

@@ -0,0 +1,2 @@
+Lint/ComparisonToBoolean:
+  Enabled: true

spec/spec_helper.cr

@@ -6,7 +6,7 @@ module Ameba
   # Dummy Rule which does nothing.
   class DummyRule < Rule::Base
     properties do
-      description : String = "Dummy rule that does nothing."
+      description "Dummy rule that does nothing."
       dummy true
     end
 
@@ -43,6 +43,9 @@ module Ameba
       description "Internal rule to test scopes"
     end
 
+    def test(source, node : Crystal::VisibilityModifier, scope : AST::Scope)
+    end
+
     def test(source, node : Crystal::ASTNode, scope : AST::Scope)
       @scopes << scope
     end
@@ -89,7 +92,7 @@ module Ameba
 
   class PerfRule < Rule::Performance::Base
     properties do
-      description : String = "Sample performance rule"
+      description "Sample performance rule"
     end
 
     def test(source)
@@ -256,6 +259,7 @@ module Ameba
       Crystal::MacroLiteral,
       Crystal::Expressions,
       Crystal::ControlExpression,
+      Crystal::Call,
     }
 
     def initialize(node)
@@ -279,6 +283,13 @@ module Ameba
   end
 end
 
+def with_presenter(klass, &)
+  io = IO::Memory.new
+  presenter = klass.new(io)
+
+  yield presenter, io
+end
+
 def as_node(source)
   Crystal::Parser.new(source).parse
 end

src/ameba.cr

@@ -3,6 +3,7 @@ require "./ameba/ast/**"
 require "./ameba/ext/**"
 require "./ameba/rule/**"
 require "./ameba/formatter/*"
+require "./ameba/presenter/*"
 require "./ameba/source/**"
 
 # Ameba's entry module.

src/ameba/ast/branch.cr

@@ -1,3 +1,5 @@
+require "./util"
+
 module Ameba::AST
   # Represents the branch in Crystal code.
   # Branch is a part of a branchable statement.
@@ -62,11 +64,13 @@ module Ameba::AST
     # Branch.of(assign_node, def_node)
     # ```
     def self.of(node : Crystal::ASTNode, parent_node : Crystal::ASTNode)
-      BranchVisitor.new(node).tap(&.accept parent_node).branch
+      BranchVisitor.new(node).tap(&.accept(parent_node)).branch
     end
 
     # :nodoc:
     private class BranchVisitor < Crystal::Visitor
+      include Util
+
       @current_branch : Crystal::ASTNode?
 
       property branchable : Branchable?
@@ -79,11 +83,12 @@ module Ameba::AST
         on_branchable_start(node, branches)
       end
 
-      private def on_branchable_start(node, branches : Array | Tuple)
+      private def on_branchable_start(node, branches : Enumerable)
         @branchable = Branchable.new(node, @branchable)
 
         branches.each do |branch_node|
           break if branch # branch found
+
           @current_branch = branch_node if branch_node && !branch_node.nop?
           branch_node.try &.accept(self)
         end
@@ -108,19 +113,11 @@ module Ameba::AST
         true
       end
 
-      def visit(node : Crystal::If)
+      def visit(node : Crystal::If | Crystal::Unless)
         on_branchable_start node, node.cond, node.then, node.else
       end
 
-      def end_visit(node : Crystal::If)
-        on_branchable_end node
-      end
-
-      def visit(node : Crystal::Unless)
-        on_branchable_start node, node.cond, node.then, node.else
-      end
-
-      def end_visit(node : Crystal::Unless)
+      def end_visit(node : Crystal::If | Crystal::Unless)
         on_branchable_end node
       end
 
@@ -140,19 +137,11 @@ module Ameba::AST
         on_branchable_end node
       end
 
-      def visit(node : Crystal::While)
+      def visit(node : Crystal::While | Crystal::Until)
         on_branchable_start node, node.cond, node.body
       end
 
-      def end_visit(node : Crystal::While)
-        on_branchable_end node
-      end
-
-      def visit(node : Crystal::Until)
-        on_branchable_start node, node.cond, node.body
-      end
-
-      def end_visit(node : Crystal::Until)
+      def end_visit(node : Crystal::While | Crystal::Until)
         on_branchable_end node
       end
 
@@ -187,6 +176,18 @@ module Ameba::AST
       def end_visit(node : Crystal::MacroFor)
         on_branchable_end node
       end
+
+      def visit(node : Crystal::Call)
+        if loop?(node) && (block = node.block)
+          on_branchable_start node, block.body
+        end
+      end
+
+      def end_visit(node : Crystal::Call)
+        if loop?(node) && node.block
+          on_branchable_end node
+        end
+      end
     end
   end
 end

src/ameba/ast/branchable.cr

@@ -15,14 +15,15 @@ module Ameba::AST
   class Branchable
     include Util
 
-    getter branches = [] of Crystal::ASTNode
-
-    # The actual Crystal node.
-    getter node : Crystal::ASTNode
-
     # Parent branchable (if any)
     getter parent : Branchable?
 
+    # Array of branches
+    getter branches = [] of Crystal::ASTNode
+
+    # The actual Crystal node
+    getter node : Crystal::ASTNode
+
     delegate to_s, to: @node
     delegate location, to: @node
     delegate end_location, to: @node

src/ameba/ast/flow_expression.cr

@@ -53,8 +53,11 @@ module Ameba::AST
       case current_node = node
       when Crystal::Expressions
         control_flow_found = false
+
         current_node.expressions.each do |exp|
-          unreachable_nodes << exp if control_flow_found
+          if control_flow_found
+            unreachable_nodes << exp
+          end
           control_flow_found ||= !loop?(exp) && flow_expression?(exp, in_loop?)
         end
       when Crystal::BinaryOp

src/ameba/ast/scope.cr

@@ -7,6 +7,9 @@ module Ameba::AST
     # Whether the scope yields.
     setter yields = false
 
+    # Scope visibility level
+    setter visibility : Crystal::Visibility?
+
     # Link to local variables
     getter variables = [] of Variable
 
@@ -31,9 +34,8 @@ module Ameba::AST
     # The actual AST node that represents a current scope.
     getter node : Crystal::ASTNode
 
-    delegate to_s, to: node
-    delegate location, to: node
-    delegate end_location, to: node
+    delegate location, end_location, to_s,
+      to: @node
 
     def_equals_and_hash node, location
 
@@ -43,7 +45,7 @@ module Ameba::AST
     # scope = Scope.new(class_node, nil)
     # ```
     def initialize(@node, @outer_scope = nil)
-      @outer_scope.try &.inner_scopes.<<(self)
+      @outer_scope.try &.inner_scopes.<< self
     end
 
     # Creates a new variable in the current scope.
@@ -94,7 +96,8 @@ module Ameba::AST
     # scope.find_variable("foo")
     # ```
     def find_variable(name : String)
-      variables.find(&.name.==(name)) || outer_scope.try &.find_variable(name)
+      variables.find(&.name.==(name)) ||
+        outer_scope.try &.find_variable(name)
     end
 
     # Creates a new assignment for the variable.
@@ -110,7 +113,8 @@ module Ameba::AST
     # Returns `true` if current scope represents a block (or proc),
     # `false` otherwise.
     def block?
-      node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral)
+      node.is_a?(Crystal::Block) ||
+        node.is_a?(Crystal::ProcLiteral)
     end
 
     # Returns `true` if current scope represents a spawn block, e. g.
@@ -121,15 +125,14 @@ module Ameba::AST
     # end
     # ```
     def spawn_block?
-      return false unless node.is_a?(Crystal::Block)
-
-      call = node.as(Crystal::Block).call
-      call.try(&.name) == "spawn"
+      node.as?(Crystal::Block).try(&.call).try(&.name) == "spawn"
     end
 
     # Returns `true` if current scope sits inside a macro.
     def in_macro?
-      (node.is_a?(Crystal::Macro) || node.is_a?(Crystal::MacroFor)) ||
+      (node.is_a?(Crystal::Macro) ||
+        node.is_a?(Crystal::MacroIf) ||
+        node.is_a?(Crystal::MacroFor)) ||
         !!outer_scope.try(&.in_macro?)
     end
 
@@ -141,7 +144,8 @@ module Ameba::AST
 
     # Returns `true` if type declaration variable is assigned in this scope.
     def assigns_type_dec?(name)
-      type_dec_variables.any?(&.name.== name) || !!outer_scope.try(&.assigns_type_dec?(name))
+      type_dec_variables.any?(&.name.== name) ||
+        !!outer_scope.try(&.assigns_type_dec?(name))
     end
 
     # Returns `true` if and only if current scope represents some
@@ -149,6 +153,7 @@ module Ameba::AST
     def type_definition?
       node.is_a?(Crystal::ClassDef) ||
         node.is_a?(Crystal::ModuleDef) ||
+        node.is_a?(Crystal::EnumDef) ||
         node.is_a?(Crystal::LibDef) ||
         node.is_a?(Crystal::FunDef) ||
         node.is_a?(Crystal::TypeDef) ||
@@ -159,27 +164,35 @@ module Ameba::AST
     # `false` otherwise.
     def references?(variable : Variable, check_inner_scopes = true)
       variable.references.any? do |reference|
-        return true if reference.scope == self
-        check_inner_scopes && inner_scopes.any?(&.references?(variable))
+        (reference.scope == self) ||
+          (check_inner_scopes && inner_scopes.any?(&.references?(variable)))
       end || variable.used_in_macro?
     end
 
     # Returns `true` if current scope (or any of inner scopes) yields,
     # `false` otherwise.
     def yields?(check_inner_scopes = true)
-      return true if @yields
-      return inner_scopes.any?(&.yields?) if check_inner_scopes
-      false
+      @yields || (check_inner_scopes && inner_scopes.any?(&.yields?))
     end
 
-    # Returns `true` if current scope is a def, `false` otherwise.
-    def def?
-      node.is_a?(Crystal::Def)
+    # Returns visibility of the current scope (could be inherited from the outer scope).
+    def visibility
+      @visibility || outer_scope.try(&.visibility)
     end
 
+    {% for type in %w[Def ClassDef ModuleDef EnumDef LibDef FunDef].map(&.id) %}
+      {% method_name = type.underscore %}
+      # Returns `true` if current scope is a {{ method_name[0..-5] }} def, `false` otherwise.
+      def {{ method_name }}?(*, check_outer_scopes = false)
+        node.is_a?(Crystal::{{ type }}) ||
+          !!(check_outer_scopes &&
+            outer_scope.try(&.{{ method_name }}?(check_outer_scopes: true)))
+      end
+    {% end %}
+
     # Returns `true` if this scope is a top level scope, `false` otherwise.
     def top_level?
-      outer_scope.nil? || type_definition?
+      outer_scope.nil?
     end
 
     # Returns `true` if var is an argument in current scope, `false` otherwise.

src/ameba/ast/util.cr

@@ -21,8 +21,8 @@ module Ameba::AST::Util
         static_literal?(node.to)}
     when Crystal::ArrayLiteral,
          Crystal::TupleLiteral
-      {true, node.elements.all? do |el|
-        static_literal?(el)
+      {true, node.elements.all? do |element|
+        static_literal?(element)
       end}
     when Crystal::HashLiteral
       {true, node.entries.all? do |entry|

src/ameba/ast/variabling/argument.cr

@@ -19,9 +19,8 @@ module Ameba::AST
     # Variable of this argument (may be the same node)
     getter variable : Variable
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     # Creates a new argument.
     #

src/ameba/ast/variabling/assignment.cr

@@ -19,9 +19,8 @@ module Ameba::AST
     # A scope assignment belongs to
     getter scope : Scope
 
-    delegate to_s, to: @node
-    delegate location, to: @node
-    delegate end_location, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     # Creates a new assignment.
     #
@@ -32,9 +31,7 @@ module Ameba::AST
       return unless scope = @variable.scope
 
       @branch = Branch.of(@node, scope)
-      @referenced = true if @variable.special? ||
-                            @variable.scope.type_definition? ||
-                            referenced_in_loop?
+      @referenced = true if @variable.special? || referenced_in_loop?
     end
 
     def referenced_in_loop?
@@ -75,31 +72,5 @@ module Ameba::AST
         node
       end
     end
-
-    # Indicates whether the node is a transformed assignment by the compiler.
-    # i.e.
-    #
-    # ```
-    # collection.each do |(a, b)|
-    #   puts b
-    # end
-    # ```
-    #
-    # is transformed to:
-    #
-    # ```
-    # collection.each do |__arg0|
-    #   a = __arg0[0]
-    #   b = __arg0[1]
-    #   puts(b)
-    # end
-    # ```
-    def transformed?
-      return false unless (assign = node).is_a?(Crystal::Assign)
-      return false unless (value = assign.value).is_a?(Crystal::Call)
-      return false unless (obj = value.obj).is_a?(Crystal::Var)
-
-      obj.name.starts_with? "__arg"
-    end
   end
 end

src/ameba/ast/variabling/ivariable.cr

@@ -2,10 +2,8 @@ module Ameba::AST
   class InstanceVariable
     getter node : Crystal::InstanceVar
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate name, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, name, to_s,
+      to: @node
 
     def initialize(@node)
     end

src/ameba/ast/variabling/type_dec_variable.cr

@@ -2,9 +2,8 @@ module Ameba::AST
   class TypeDecVariable
     getter node : Crystal::TypeDeclaration
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, to_s,
+      to: @node
 
     def initialize(@node)
     end

src/ameba/ast/variabling/variable.cr

@@ -17,10 +17,8 @@ module Ameba::AST
     # Node of the first assignment which can be available before any reference.
     getter assign_before_reference : Crystal::ASTNode?
 
-    delegate location, to: @node
-    delegate end_location, to: @node
-    delegate name, to: @node
-    delegate to_s, to: @node
+    delegate location, end_location, name, to_s,
+      to: @node
 
     # Creates a new variable(in the scope).
     #
@@ -54,7 +52,7 @@ module Ameba::AST
     #
     # ```
     # variable = Variable.new(node, scope)
-    # variable.reference(var_node)
+    # variable.reference(var_node, some_scope)
     # variable.referenced? # => true
     # ```
     def referenced?
@@ -74,6 +72,11 @@ module Ameba::AST
       end
     end
 
+    # :ditto:
+    def reference(scope : Scope)
+      reference(node, scope)
+    end
+
     # Reference variable's assignments.
     #
     # ```
@@ -136,7 +139,7 @@ module Ameba::AST
       case assign
       when Crystal::Assign           then eql?(assign.target)
       when Crystal::OpAssign         then eql?(assign.target)
-      when Crystal::MultiAssign      then assign.targets.any? { |t| eql?(t) }
+      when Crystal::MultiAssign      then assign.targets.any? { |target| eql?(target) }
       when Crystal::UninitializedVar then eql?(assign.var)
       else
         false
@@ -170,7 +173,7 @@ module Ameba::AST
     private class MacroReferenceFinder < Crystal::Visitor
       property? references = false
 
-      def initialize(node, @reference : String = reference)
+      def initialize(node, @reference : String)
         node.accept self
       end
 
@@ -179,10 +182,6 @@ module Ameba::AST
         val.to_s.includes?(@reference)
       end
 
-      def visit(node : Crystal::ASTNode)
-        true
-      end
-
       def visit(node : Crystal::MacroLiteral)
         !(@references ||= includes_reference?(node.value))
       end
@@ -201,14 +200,20 @@ module Ameba::AST
                           includes_reference?(node.then) ||
                           includes_reference?(node.else))
       end
+
+      def visit(node : Crystal::ASTNode)
+        true
+      end
     end
 
     private def update_assign_reference!
-      if @assign_before_reference.nil? &&
-         references.size <= assignments.size &&
-         assignments.none?(&.op_assign?)
-        @assign_before_reference = assignments.find { |ass| !ass.in_branch? }.try &.node
-      end
+      return if @assign_before_reference
+      return if references.size > assignments.size
+      return if assignments.any?(&.op_assign?)
+
+      @assign_before_reference = assignments
+        .find(&.in_branch?.!)
+        .try(&.node)
     end
   end
 end

src/ameba/ast/visitors/counting_visitor.cr

@@ -24,7 +24,7 @@ module Ameba::AST
     # Uses the same logic than rubocop. See
     # https://github.com/rubocop-hq/rubocop/blob/master/lib/rubocop/cop/metrics/cyclomatic_complexity.rb#L21
     # Except "for", because crystal doesn't have a "for" loop.
-    {% for node in %i(if while until rescue or and) %}
+    {% for node in %i[if while until rescue or and] %}
       # :nodoc:
       def visit(node : Crystal::{{ node.id.capitalize }})
         @complexity += 1 unless macro_condition?

src/ameba/ast/visitors/flow_expression_visitor.cr

@@ -8,11 +8,6 @@ module Ameba::AST
 
     @loop_stack = [] of Crystal::ASTNode
 
-    # Creates a new flow expression visitor.
-    def initialize(@rule, @source)
-      @source.ast.accept self
-    end
-
     # :nodoc:
     def visit(node)
       if flow_expression?(node, in_loop?)
@@ -22,12 +17,7 @@ module Ameba::AST
     end
 
     # :nodoc:
-    def visit(node : Crystal::While)
-      on_loop_started(node)
-    end
-
-    # :nodoc:
-    def visit(node : Crystal::Until)
+    def visit(node : Crystal::While | Crystal::Until)
       on_loop_started(node)
     end
 
@@ -37,12 +27,7 @@ module Ameba::AST
     end
 
     # :nodoc:
-    def end_visit(node : Crystal::While)
-      on_loop_ended(node)
-    end
-
-    # :nodoc:
-    def end_visit(node : Crystal::Until)
+    def end_visit(node : Crystal::While | Crystal::Until)
       on_loop_ended(node)
     end
 

src/ameba/ast/visitors/node_visitor.cr

@@ -1,34 +1,6 @@
 require "./base_visitor"
 
 module Ameba::AST
-  # List of nodes to be visited by Ameba's rules.
-  NODES = {
-    Alias,
-    IsA,
-    Assign,
-    Call,
-    Block,
-    Case,
-    ClassDef,
-    ClassVar,
-    Def,
-    EnumDef,
-    ExceptionHandler,
-    Expressions,
-    HashLiteral,
-    If,
-    InstanceVar,
-    LibDef,
-    ModuleDef,
-    NilLiteral,
-    StringInterpolation,
-    Unless,
-    Var,
-    When,
-    While,
-    Until,
-  }
-
   # An AST Visitor that traverses the source and allows all nodes
   # to be inspected by rules.
   #
@@ -36,13 +8,68 @@ module Ameba::AST
   # visitor = Ameba::AST::NodeVisitor.new(rule, source)
   # ```
   class NodeVisitor < BaseVisitor
+    @[Flags]
+    enum Category
+      Macro
+    end
+
+    # List of nodes to be visited by Ameba's rules.
+    NODES = {
+      Alias,
+      Assign,
+      Block,
+      Call,
+      Case,
+      ClassDef,
+      ClassVar,
+      Def,
+      EnumDef,
+      ExceptionHandler,
+      Expressions,
+      HashLiteral,
+      If,
+      InstanceVar,
+      IsA,
+      LibDef,
+      ModuleDef,
+      MultiAssign,
+      NilLiteral,
+      StringInterpolation,
+      Unless,
+      Until,
+      Var,
+      When,
+      While,
+    }
+
     @skip : Array(Crystal::ASTNode.class)?
 
-    def initialize(@rule, @source, skip = nil)
+    def self.category_to_node_classes(category : Category)
+      ([] of Crystal::ASTNode.class).tap do |classes|
+        classes.push(
+          Crystal::Macro,
+          Crystal::MacroExpression,
+          Crystal::MacroIf,
+          Crystal::MacroFor,
+        ) if category.macro?
+      end
+    end
+
+    def initialize(@rule, @source, *, skip : Category)
+      initialize @rule, @source,
+        skip: NodeVisitor.category_to_node_classes(skip)
+    end
+
+    def initialize(@rule, @source, *, skip : Array? = nil)
       @skip = skip.try &.map(&.as(Crystal::ASTNode.class))
       super @rule, @source
     end
 
+    def visit(node : Crystal::VisibilityModifier)
+      node.exp.visibility = node.modifier
+      true
+    end
+
     {% for name in NODES %}
       # A visit callback for `Crystal::{{ name }}` node.
       #

src/ameba/ast/visitors/redundant_control_expression_visitor.cr

@@ -43,7 +43,7 @@ module Ameba::AST
     end
 
     private def traverse_case(node)
-      node.whens.each { |n| traverse_node n.body }
+      node.whens.each { |when_node| traverse_node when_node.body }
       traverse_node(node.else)
     end
 
@@ -54,7 +54,7 @@ module Ameba::AST
     private def traverse_exception_handler(node)
       traverse_node node.body
       traverse_node node.else
-      node.rescues.try &.each { |n| traverse_node n.body }
+      node.rescues.try &.each { |rescue_node| traverse_node rescue_node.body }
     end
   end
 end

src/ameba/ast/visitors/scope_visitor.cr

@@ -16,35 +16,47 @@ module Ameba::AST
       ProcLiteral,
       Block,
       Macro,
+      MacroIf,
       MacroFor,
     }
 
     SPECIAL_NODE_NAMES = %w[super previous_def]
-    RECORD_NODE_NAME   = "record"
 
     @scope_queue = [] of Scope
     @current_scope : Scope
     @current_assign : Crystal::ASTNode?
+    @current_visibility : Crystal::Visibility?
     @skip : Array(Crystal::ASTNode.class)?
 
     def initialize(@rule, @source, skip = nil)
-      @skip = skip.try &.map(&.as(Crystal::ASTNode.class))
       @current_scope = Scope.new(@source.ast) # top level scope
-      @source.ast.accept self
-      @scope_queue.each { |scope| @rule.test @source, scope.node, scope }
+      @skip = skip.try &.map(&.as(Crystal::ASTNode.class))
+
+      super @rule, @source
+
+      @scope_queue.each do |scope|
+        @rule.test @source, scope.node, scope
+      end
     end
 
     private def on_scope_enter(node)
       return if skip?(node)
-      @current_scope = Scope.new(node, @current_scope)
+
+      scope = Scope.new(node, @current_scope)
+      scope.visibility = @current_visibility
+
+      @current_scope = scope
     end
 
     private def on_scope_end(node)
       @scope_queue << @current_scope
 
+      @current_visibility = nil
+
       # go up if this is not a top level scope
-      return unless outer_scope = @current_scope.outer_scope
-      @current_scope = outer_scope
+      if outer_scope = @current_scope.outer_scope
+        @current_scope = outer_scope
+      end
     end
 
     private def on_assign_end(target, node)
@@ -64,6 +76,12 @@ module Ameba::AST
       end
     {% end %}
 
+    # :nodoc:
+    def visit(node : Crystal::VisibilityModifier)
+      @current_visibility = node.exp.visibility = node.modifier
+      true
+    end
+
     # :nodoc:
     def visit(node : Crystal::Yield)
       @current_scope.yields = true
@@ -83,6 +101,7 @@ module Ameba::AST
     def end_visit(node : Crystal::Assign | Crystal::OpAssign)
       on_assign_end(node.target, node)
       @current_assign = nil
+
       on_scope_end(node) if @current_scope.eql?(node)
     end
 
@@ -90,6 +109,7 @@ module Ameba::AST
     def end_visit(node : Crystal::MultiAssign)
       node.targets.each { |target| on_assign_end(target, node) }
       @current_assign = nil
+
       on_scope_end(node) if @current_scope.eql?(node)
     end
 
@@ -97,6 +117,7 @@ module Ameba::AST
     def end_visit(node : Crystal::UninitializedVar)
       on_assign_end(node.var, node)
       @current_assign = nil
+
       on_scope_end(node) if @current_scope.eql?(node)
     end
 
@@ -106,14 +127,17 @@ module Ameba::AST
 
       @current_scope.add_variable(var)
       @current_scope.add_type_dec_variable(node)
-      @current_assign = node.value unless node.value.nil?
+
+      @current_assign = node.value if node.value
     end
 
+    # :nodoc:
     def end_visit(node : Crystal::TypeDeclaration)
       return unless (var = node.var).is_a?(Crystal::Var)
 
       on_assign_end(var, node)
       @current_assign = nil
+
       on_scope_end(node) if @current_scope.eql?(node)
     end
 
@@ -129,7 +153,7 @@ module Ameba::AST
 
     # :nodoc:
     def visit(node : Crystal::Var)
-      variable = @current_scope.find_variable node.name
+      variable = @current_scope.find_variable(node.name)
 
       case
       when @current_scope.arg?(node) # node is an argument
@@ -137,7 +161,7 @@ module Ameba::AST
       when variable.nil? && @current_assign # node is a variable
         @current_scope.add_variable(node)
       when variable # node is a reference
-        reference = variable.reference node, @current_scope
+        reference = variable.reference(node, @current_scope)
         if @current_assign.is_a?(Crystal::OpAssign) || !reference.target_of?(@current_assign)
           variable.reference_assignments!
         end
@@ -146,24 +170,39 @@ module Ameba::AST
 
     # :nodoc:
     def visit(node : Crystal::Call)
+      scope = @current_scope
+
       case
-      when @current_scope.def?
-        if node.name.in?(SPECIAL_NODE_NAMES) && node.args.empty?
-          @current_scope.arguments.each do |arg|
-            variable = arg.variable
-            variable.reference(variable.node, @current_scope).explicit = false
-          end
+      when (scope.top_level? || scope.type_definition?) && record_macro?(node)
+        return false
+      when scope.type_definition? && accessor_macro?(node)
+        return false
+      when scope.def? && special_node?(node)
+        scope.arguments.each do |arg|
+          ref = arg.variable.reference(scope)
+          ref.explicit = false
         end
-        true
-      when @current_scope.top_level? && record_macro?(node)
-        false
-      else
-        true
       end
+      true
+    end
+
+    private def special_node?(node)
+      node.name.in?(SPECIAL_NODE_NAMES) && node.args.empty?
+    end
+
+    private def accessor_macro?(node)
+      node.name.matches? /^(class_)?(getter[?!]?|setter|property[?!]?)$/
     end
 
     private def record_macro?(node)
-      node.name == RECORD_NODE_NAME && node.args.first?.is_a?(Crystal::Path)
+      return false unless node.name == "record"
+
+      case node.args.first?
+      when Crystal::Path, Crystal::Generic
+        true
+      else
+        false
+      end
     end
 
     private def skip?(node)

src/ameba/cli/cmd.cr

@@ -28,7 +28,14 @@ module Ameba::Cli
     configure_rules(config, opts)
 
     if opts.rules?
-      print_rules(config)
+      print_rules(config.rules)
+    end
+
+    if describe_rule_name = opts.describe_rule
+      unless rule = config.rules.find(&.name.== describe_rule_name)
+        raise "Unknown rule"
+      end
+      describe_rule(rule)
     end
 
     runner = Ameba.run(config)
@@ -49,6 +56,7 @@ module Ameba::Cli
     property globs : Array(String)?
     property only : Array(String)?
     property except : Array(String)?
+    property describe_rule : String?
     property location_to_explain : NamedTuple(file: String, line: Int32, column: Int32)?
     property fail_level : Severity?
     property? skip_reading_config = false
@@ -67,11 +75,11 @@ module Ameba::Cli
       parser.on("-h", "--help", "Show this help") { print_help(parser) }
       parser.on("-r", "--rules", "Show all available rules") { opts.rules = true }
       parser.on("-s", "--silent", "Disable output") { opts.formatter = :silent }
-      parser.unknown_args do |f|
-        if f.size == 1 && f.first =~ /.+:\d+:\d+/
-          configure_explain_opts(f.first, opts)
+      parser.unknown_args do |arr|
+        if arr.size == 1 && arr.first.matches?(/.+:\d+:\d+/)
+          configure_explain_opts(arr.first, opts)
         else
-          opts.globs = f unless f.empty?
+          opts.globs = arr unless arr.empty?
         end
       end
 
@@ -119,6 +127,11 @@ module Ameba::Cli
         configure_explain_opts(loc, opts)
       end
 
+      parser.on("-d", "--describe Category/Rule",
+        "Describe a rule with specified name") do |rule_name|
+        configure_describe_opts(rule_name, opts)
+      end
+
       parser.on("--without-affected-code",
         "Stop showing affected code while using a default formatter") do
         opts.without_affected_code = true
@@ -152,6 +165,11 @@ module Ameba::Cli
       opts.without_affected_code?
   end
 
+  private def configure_describe_opts(rule_name, opts)
+    opts.describe_rule = rule_name.presence
+    opts.formatter = :silent
+  end
+
   private def configure_explain_opts(loc, opts)
     location_to_explain = parse_explain_location(loc)
     opts.location_to_explain = location_to_explain
@@ -183,14 +201,13 @@ module Ameba::Cli
     exit 0
   end
 
-  private def print_rules(config)
-    config.rules.each do |rule|
-      puts "%s [%s] - %s" % {
-        rule.name.colorize(:white),
-        rule.severity.symbol.to_s.colorize(:green),
-        rule.description.colorize(:dark_gray),
-      }
-    end
+  private def describe_rule(rule)
+    Presenter::RulePresenter.new.run(rule)
+    exit 0
+  end
+
+  private def print_rules(rules)
+    Presenter::RuleCollectionPresenter.new.run(rules)
     exit 0
   end
 end

src/ameba/config.cr

@@ -97,8 +97,9 @@ class Ameba::Config
     @excluded = load_array_section(config, "Excluded")
     @globs = load_array_section(config, "Globs", DEFAULT_GLOBS)
 
-    return unless formatter_name = load_formatter_name(config)
-    self.formatter = formatter_name
+    if formatter_name = load_formatter_name(config)
+      self.formatter = formatter_name
+    end
   end
 
   # Loads YAML configuration file by `path`.
@@ -115,12 +116,13 @@ class Ameba::Config
               end
     Config.new YAML.parse(content)
   rescue e
-    raise "Config file is invalid: #{e.message}"
+    raise "Unable to load config file: #{e.message}"
   end
 
   protected def self.read_config(path = nil)
     if path
-      return File.exists?(path) ? File.read(path) : nil
+      return File.read(path) if File.exists?(path)
+      raise "Config file does not exist"
     end
     each_config_path do |config_path|
       return File.read(config_path) if File.exists?(config_path)
@@ -202,13 +204,13 @@ class Ameba::Config
   #
   # ```
   # config = Ameba::Config.load
-  # config.update_rules %w(Rule1 Rule2), enabled: true
+  # config.update_rules %w[Rule1 Rule2], enabled: true
   # ```
   #
   # also it allows to update groups of rules:
   #
   # ```
-  # config.update_rules %w(Group1 Group2), enabled: true
+  # config.update_rules %w[Group1 Group2], enabled: true
   # ```
   def update_rules(names, enabled = true, excluded = nil)
     names.try &.each do |name|
@@ -240,22 +242,23 @@ class Ameba::Config
 
   # :nodoc:
   module RuleConfig
+    # Define rule properties
     macro properties(&block)
       {% definitions = [] of NamedTuple %}
-      {% if block.body.is_a? Assign %}
-        {% definitions << {var: block.body.target, value: block.body.value} %}
-      {% elsif block.body.is_a? Call %}
-          {% definitions << {var: block.body.name, value: block.body.args.first} %}
-      {% elsif block.body.is_a? TypeDeclaration %}
-        {% definitions << {var: block.body.var, value: block.body.value, type: block.body.type} %}
+      {% if (prop = block.body).is_a? Call %}
+        {% if (named_args = prop.named_args) && (type = named_args.select(&.name.== "as".id).first) %}
+          {% definitions << {var: prop.name, value: prop.args.first, type: type.value} %}
+        {% else %}
+          {% definitions << {var: prop.name, value: prop.args.first} %}
+        {% end %}
       {% elsif block.body.is_a? Expressions %}
         {% for prop in block.body.expressions %}
-          {% if prop.is_a? Assign %}
-            {% definitions << {var: prop.target, value: prop.value} %}
-          {% elsif prop.is_a? Call %}
-            {% definitions << {var: prop.name, value: prop.args.first} %}
-          {% elsif prop.is_a? TypeDeclaration %}
-            {% definitions << {var: prop.var, value: prop.value, type: prop.type} %}
+          {% if prop.is_a? Call %}
+            {% if (named_args = prop.named_args) && (type = named_args.select(&.name.== "as".id).first) %}
+              {% definitions << {var: prop.name, value: prop.args.first, type: type.value} %}
+            {% else %}
+              {% definitions << {var: prop.name, value: prop.args.first} %}
+            {% end %}
           {% end %}
         {% end %}
       {% end %}
@@ -273,7 +276,7 @@ class Ameba::Config
           {% converter = SeverityYamlConverter %}
         {% end %}
 
-        {% if type == nil %}
+        {% unless type %}
           {% if value.is_a? BoolLiteral %}
             {% type = Bool %}
           {% elsif value.is_a? StringLiteral %}
@@ -283,23 +286,23 @@ class Ameba::Config
               {% type = Int32 %}
             {% elsif value.kind == :i64 %}
               {% type = Int64 %}
+            {% elsif value.kind == :i128 %}
+              {% type = Int128 %}
             {% elsif value.kind == :f32 %}
               {% type = Float32 %}
             {% elsif value.kind == :f64 %}
               {% type = Float64 %}
             {% end %}
           {% end %}
-
-          {% type = Nil if type == nil %}
         {% end %}
 
         {% properties[name] = {key: key, default: value, type: type, converter: converter} %}
 
-        @[YAML::Field(key: {{ key }}, converter: {{ converter }}, type: {{ type }})]
+        @[YAML::Field(key: {{ key }}, converter: {{ converter }})]
         {% if type == Bool %}
-          property? {{ name }} : {{ type }} = {{ value }}
+          property? {{ name }}{{ " : #{type}".id if type }} = {{ value }}
         {% else %}
-          property {{ name }} : {{ type }} = {{ value }}
+          property {{ name }}{{ " : #{type}".id if type }} = {{ value }}
         {% end %}
       {% end %}
 
@@ -321,9 +324,10 @@ class Ameba::Config
 
     macro included
       GROUP_SEVERITY = {
-        Lint:        Ameba::Severity::Warning,
-        Metrics:     Ameba::Severity::Warning,
-        Performance: Ameba::Severity::Warning,
+        Documentation: Ameba::Severity::Warning,
+        Lint:          Ameba::Severity::Warning,
+        Metrics:       Ameba::Severity::Warning,
+        Performance:   Ameba::Severity::Warning,
       }
 
       class_getter default_severity : Ameba::Severity do

src/ameba/formatter/base_formatter.cr

@@ -17,13 +17,13 @@ module Ameba::Formatter
     # A list of sources to inspect is passed as an argument.
     def started(sources); end
 
-    # Callback that indicates when source inspection is finished.
+    # Callback that indicates when source inspection is started.
     # A corresponding source is passed as an argument.
-    def source_finished(source : Source); end
+    def source_started(source : Source); end
 
     # Callback that indicates when source inspection is finished.
     # A corresponding source is passed as an argument.
-    def source_started(source : Source); end
+    def source_finished(source : Source); end
 
     # Callback that indicates when inspection is finished.
     # A list of inspected sources is passed as an argument.

src/ameba/formatter/explain_formatter.cr

@@ -4,8 +4,6 @@ module Ameba::Formatter
   # A formatter that shows the detailed explanation of the issue at
   # a specific location.
   class ExplainFormatter
-    HEADING_MARKER = "## "
-
     include Util
 
     getter output : IO::FileDescriptor | IO::Memory
@@ -64,9 +62,8 @@ module Ameba::Formatter
         rule.name.colorize(:magenta),
         rule.severity.to_s.colorize(rule.severity.color),
       }
-
-      if rule.responds_to?(:description)
-        output_paragraph rule.description
+      if rule_description = colorize_code_fences(rule.description)
+        output_paragraph rule_description
       end
 
       rule_doc = colorize_code_fences(rule.class.parsed_doc)
@@ -84,7 +81,7 @@ module Ameba::Formatter
     end
 
     private def output_title(title)
-      output << HEADING_MARKER.colorize(:yellow)
+      output << "### ".colorize(:yellow)
       output << title.upcase.colorize(:yellow)
       output << "\n\n"
     end
@@ -95,7 +92,7 @@ module Ameba::Formatter
 
     private def output_paragraph(paragraph : Array)
       paragraph.each do |line|
-        output << ' ' << line << '\n'
+        output << "    " << line << '\n'
       end
       output << '\n'
     end

src/ameba/formatter/todo_formatter.cr

@@ -3,7 +3,7 @@ module Ameba::Formatter
   # Basically, it takes all issues reported and disables corresponding rules
   # or excludes failed sources from these rules.
   class TODOFormatter < DotFormatter
-    def initialize(@output = STDOUT)
+    def initialize(@output = STDOUT, @config_path : Path = Config::DEFAULT_PATH)
     end
 
     def finished(sources)
@@ -26,25 +26,30 @@ module Ameba::Formatter
     end
 
     private def generate_todo_config(issues)
-      file = File.new(Config::DEFAULT_PATH, mode: "w")
-      file << header
-      rule_issues_map(issues).each do |rule, rule_issues|
-        file << "\n# Problems found: #{rule_issues.size}"
-        file << "\n# Run `ameba --only #{rule.name}` for details"
-        file << rule_todo(rule, rule_issues).gsub("---", "")
+      File.open(@config_path, mode: "w") do |file|
+        file << header
+
+        rule_issues_map(issues).each do |rule, rule_issues|
+          rule_todo = rule_todo(rule, rule_issues)
+          rule_todo =
+            {rule_todo.name => rule_todo}
+              .to_yaml.gsub("---", "")
+
+          file << "\n# Problems found: #{rule_issues.size}"
+          file << "\n# Run `ameba --only #{rule.name}` for details"
+          file << rule_todo
+        end
+        file
       end
-      file
-    ensure
-      file.close if file
     end
 
     private def rule_issues_map(issues)
-      Hash(Rule::Base, Array(Issue)).new.tap do |h|
+      Hash(Rule::Base, Array(Issue)).new.tap do |hash|
         issues.each do |issue|
           next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax)
           next if issue.correctable? && config[:autocorrect]?
 
-          (h[issue.rule] ||= Array(Issue).new) << issue
+          (hash[issue.rule] ||= Array(Issue).new) << issue
         end
       end
     end
@@ -60,11 +65,11 @@ module Ameba::Formatter
     end
 
     private def rule_todo(rule, issues)
-      rule.excluded = issues
-        .compact_map(&.location.try &.filename.try &.to_s)
-        .uniq!
-
-      {rule.name => rule}.to_yaml
+      rule.dup.tap do |rule_todo|
+        rule_todo.excluded = issues
+          .compact_map(&.location.try &.filename.try &.to_s)
+          .uniq!
+      end
     end
   end
 end

src/ameba/formatter/util.cr

@@ -1,5 +1,7 @@
 module Ameba::Formatter
   module Util
+    extend self
+
     def deansify(message : String?) : String?
       message.try &.gsub(/\x1b[^m]*m/, "").presence
     end

src/ameba/glob_utils.cr

@@ -1,6 +1,8 @@
 module Ameba
   # Helper module that is utilizes helpers for working with globs.
   module GlobUtils
+    extend self
+
     # Returns all files that match specified globs.
     # Globs can have wildcards or be rejected:
     #
@@ -20,10 +22,13 @@ module Ameba
     # expand(["spec/*.cr", "src"]) # => all files in src folder + first level specs
     # ```
     def expand(globs)
-      globs.flat_map do |glob|
-        glob += "/**/*.cr" if File.directory?(glob)
-        Dir[glob]
-      end.uniq!
+      globs
+        .flat_map do |glob|
+          glob += "/**/*.cr" if File.directory?(glob)
+          Dir[glob]
+        end
+        .uniq!
+        .select! { |path| File.file?(path) }
     end
 
     private def rejected_globs(globs)

src/ameba/presenter/base_presenter.cr

@@ -0,0 +1,12 @@
+module Ameba::Presenter
+  private ENABLED_MARK  = "✓".colorize(:green)
+  private DISABLED_MARK = "x".colorize(:red)
+
+  class BasePresenter
+    # TODO: allow other IOs
+    getter output : IO::FileDescriptor | IO::Memory
+
+    def initialize(@output = STDOUT)
+    end
+  end
+end

src/ameba/presenter/rule_collection_presenter.cr

@@ -0,0 +1,34 @@
+module Ameba::Presenter
+  class RuleCollectionPresenter < BasePresenter
+    def run(rules)
+      rules = rules.to_h do |rule|
+        name = rule.name.split('/')
+        name = "%s/%s" % {
+          name[0...-1].join('/').colorize(:light_gray),
+          name.last.colorize(:white),
+        }
+        {name, rule}
+      end
+      longest_name = rules.max_of(&.first.size)
+
+      rules.group_by(&.last.group).each do |group, group_rules|
+        output.puts "— %s" % group.colorize(:light_blue).underline
+        output.puts
+        group_rules.each do |name, rule|
+          output.puts "  %s  [%s]    %s    %s" % {
+            rule.enabled? ? ENABLED_MARK : DISABLED_MARK,
+            rule.severity.symbol.to_s.colorize(:green),
+            name.ljust(longest_name),
+            rule.description.colorize(:dark_gray),
+          }
+        end
+        output.puts
+      end
+
+      output.puts "Total rules: %s / %s enabled" % {
+        rules.size.to_s.colorize(:light_blue),
+        rules.count(&.last.enabled?).to_s.colorize(:light_blue),
+      }
+    end
+  end
+end

src/ameba/presenter/rule_presenter.cr

@@ -0,0 +1,43 @@
+module Ameba::Presenter
+  class RulePresenter < BasePresenter
+    def run(rule)
+      output.puts
+      output_title "Rule info"
+      output_paragraph "%s of a %s severity [enabled: %s]" % {
+        rule.name.colorize(:magenta),
+        rule.severity.to_s.colorize(rule.severity.color),
+        rule.enabled? ? ENABLED_MARK : DISABLED_MARK,
+      }
+      if rule_description = colorize_code_fences(rule.description)
+        output_paragraph rule_description
+      end
+
+      if rule_doc = colorize_code_fences(rule.class.parsed_doc)
+        output_title "Detailed description"
+        output_paragraph rule_doc
+      end
+    end
+
+    private def output_title(title)
+      output.print "### %s\n\n" % title.upcase.colorize(:yellow)
+    end
+
+    private def output_paragraph(paragraph : String)
+      output_paragraph(paragraph.lines)
+    end
+
+    private def output_paragraph(paragraph : Array)
+      paragraph.each do |line|
+        output.puts "    #{line}"
+      end
+      output.puts
+    end
+
+    private def colorize_code_fences(string)
+      return unless string
+      string
+        .gsub(/```(.+?)```/m, &.colorize(:dark_gray))
+        .gsub(/`(?!`)(.+?)`/, &.colorize(:dark_gray))
+    end
+  end
+end

src/ameba/reportable.cr

@@ -1,6 +1,10 @@
+require "./ast/util"
+
 module Ameba
   # Represents a module used to report issues.
   module Reportable
+    include AST::Util
+
     # List of reported issues.
     getter issues = [] of Issue
 
@@ -30,13 +34,19 @@ module Ameba
     end
 
     # Adds a new issue for Crystal AST *node*.
-    def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil) : Issue
-      add_issue rule, node.location, node.end_location, message, status, block
+    def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, block : (Source::Corrector ->)? = nil, *, prefer_name_location = false) : Issue
+      location = name_location(node) if prefer_name_location
+      location ||= node.location
+
+      end_location = name_end_location(node) if prefer_name_location
+      end_location ||= node.end_location
+
+      add_issue rule, location, end_location, message, status, block
     end
 
     # :ditto:
-    def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, &block : Source::Corrector ->) : Issue
-      add_issue rule, node, message, status, block
+    def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil, *, prefer_name_location = false, &block : Source::Corrector ->) : Issue
+      add_issue rule, node, message, status, block, prefer_name_location: prefer_name_location
     end
 
     # Adds a new issue for Crystal *token*.

src/ameba/rule/base.cr

@@ -32,14 +32,15 @@ module Ameba::Rule
     # This method is designed to test the source passed in. If source has issues
     # that are tested by this rule, it should add an issue.
     #
-    # Be default it uses a node visitor to traverse all the nodes in the source.
+    # By default it uses a node visitor to traverse all the nodes in the source.
+    #
     # NOTE: Must be overridden for other type of rules.
     def test(source : Source)
       AST::NodeVisitor.new self, source
     end
 
+    # NOTE: Can't be abstract
     def test(source : Source, node : Crystal::ASTNode, *opts)
-      # can't be abstract
     end
 
     # A convenient addition to `#test` method that does the same
@@ -112,8 +113,9 @@ module Ameba::Rule
       name.hash
     end
 
+    # Adds an issue to the *source*
     macro issue_for(*args, **kwargs, &block)
-      source.add_issue(self, {{ *args }}, {{ **kwargs }}) {{ block }}
+      source.add_issue(self, {{ args.splat }}, {{ kwargs.double_splat }}) {{ block }}
     end
 
     protected def self.rule_name

src/ameba/rule/documentation/documentation.cr

@@ -0,0 +1,80 @@
+module Ameba::Rule::Documentation
+  # A rule that enforces documentation for public types:
+  # modules, classes, enums, methods and macros.
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Documentation/Documentation:
+  #   Enabled: true
+  #   IgnoreClasses: false
+  #   IgnoreModules: true
+  #   IgnoreEnums: false
+  #   IgnoreDefs: true
+  #   IgnoreMacros: false
+  #   IgnoreMacroHooks: true
+  # ```
+  class Documentation < Base
+    properties do
+      enabled false
+      description "Enforces public types to be documented"
+
+      ignore_classes false
+      ignore_modules true
+      ignore_enums false
+      ignore_defs true
+      ignore_macros false
+      ignore_macro_hooks true
+    end
+
+    MSG = "Missing documentation"
+
+    MACRO_HOOK_NAMES = %w[
+      inherited
+      included extended
+      method_missing method_added
+      finished
+    ]
+
+    def test(source)
+      AST::ScopeVisitor.new self, source
+    end
+
+    def test(source, node : Crystal::ClassDef, scope : AST::Scope)
+      ignore_classes? || check_missing_doc(source, node, scope)
+    end
+
+    def test(source, node : Crystal::ModuleDef, scope : AST::Scope)
+      ignore_modules? || check_missing_doc(source, node, scope)
+    end
+
+    def test(source, node : Crystal::EnumDef, scope : AST::Scope)
+      ignore_enums? || check_missing_doc(source, node, scope)
+    end
+
+    def test(source, node : Crystal::Def, scope : AST::Scope)
+      ignore_defs? || check_missing_doc(source, node, scope)
+    end
+
+    def test(source, node : Crystal::Macro, scope : AST::Scope)
+      return if ignore_macro_hooks? && node.name.in?(MACRO_HOOK_NAMES)
+
+      ignore_macros? || check_missing_doc(source, node, scope)
+    end
+
+    private def check_missing_doc(source, node, scope)
+      # bail out if the node is not public,
+      # i.e. `private def foo`
+      return if !node.visibility.public?
+
+      # bail out if the scope is not public,
+      # i.e. `def bar` inside `private class Foo`
+      return if (visibility = scope.visibility) && !visibility.public?
+
+      # bail out if the node has the documentation present
+      return if node.doc.presence
+
+      issue_for(node, MSG)
+    end
+  end
+end

src/ameba/rule/documentation/documentation_admonition.cr

@@ -0,0 +1,96 @@
+module Ameba::Rule::Documentation
+  # A rule that reports documentation admonitions.
+  #
+  # Optionally, these can fail at an appropriate time.
+  #
+  # ```
+  # def get_user(id)
+  #   # TODO(2024-04-24) Fix this hack when the database migration is complete
+  #   if id < 1_000_000
+  #     v1_api_call(id)
+  #   else
+  #     v2_api_call(id)
+  #   end
+  # end
+  # ```
+  #
+  # `TODO` comments are used to remind yourself of source code related things.
+  #
+  # The premise here is that `TODO` should be dealt with in the near future
+  # and are therefore reported by Ameba.
+  #
+  # `FIXME` comments are used to indicate places where source code needs fixing.
+  #
+  # The premise here is that `FIXME` should indeed be fixed as soon as possible
+  # and are therefore reported by Ameba.
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Documentation/DocumentationAdmonition:
+  #   Enabled: true
+  #   Admonitions: [TODO, FIXME, BUG]
+  #   Timezone: UTC
+  # ```
+  class DocumentationAdmonition < Base
+    properties do
+      description "Reports documentation admonitions"
+      admonitions %w[TODO FIXME BUG]
+      timezone "UTC"
+    end
+
+    MSG      = "Found a %s admonition in a comment"
+    MSG_LATE = "Found a %s admonition in a comment (%s)"
+    MSG_ERR  = "%s admonition error: %s"
+
+    @[YAML::Field(ignore: true)]
+    private getter location : Time::Location {
+      Time::Location.load(self.timezone)
+    }
+
+    def test(source)
+      Tokenizer.new(source).run do |token|
+        next unless token.type.comment?
+        next unless doc = token.value.to_s
+
+        pattern =
+          /^#\s*(?<admonition>#{Regex.union(admonitions)})(?:\((?<context>.+?)\))?(?:\W+|$)/m
+
+        matches = doc.scan(pattern)
+        matches.each do |match|
+          admonition = match["admonition"]
+          begin
+            case expr = match["context"]?.presence
+            when /\A\d{4}-\d{2}-\d{2}\Z/ # date
+              # ameba:disable Lint/NotNil
+              date = Time.parse(expr.not_nil!, "%F", location)
+              issue_for_date source, token, admonition, date
+            when /\A\d{4}-\d{2}-\d{2} \d{2}:\d{2}(:\d{2})?\Z/ # date + time (no tz)
+              # ameba:disable Lint/NotNil
+              date = Time.parse(expr.not_nil!, "%F #{$1?.presence ? "%T" : "%R"}", location)
+              issue_for_date source, token, admonition, date
+            else
+              issue_for token, MSG % admonition
+            end
+          rescue ex
+            issue_for token, MSG_ERR % {admonition, "#{ex}: #{expr.inspect}"}
+          end
+        end
+      end
+    end
+
+    private def issue_for_date(source, node, admonition, date)
+      diff = Time.utc - date.to_utc
+
+      return if diff.negative?
+
+      past = case diff
+             when 0.seconds..1.day then "today is the day!"
+             when 1.day..2.days    then "1 day past"
+             else                       "#{diff.total_days.to_i} days past"
+             end
+
+      issue_for node, MSG_LATE % {admonition, past}
+    end
+  end
+end

src/ameba/rule/lint/comparison_to_boolean.cr

@@ -28,7 +28,7 @@ module Ameba::Rule::Lint
     end
 
     MSG      = "Comparison to a boolean is pointless"
-    OP_NAMES = %w(== != ===)
+    OP_NAMES = %w[== != ===]
 
     def test(source, node : Crystal::Call)
       return unless node.name.in?(OP_NAMES)

src/ameba/rule/lint/debug_calls.cr

@@ -18,7 +18,7 @@ module Ameba::Rule::Lint
   class DebugCalls < Base
     properties do
       description "Disallows debug-related calls"
-      method_names : Array(String) = %w(p p! pp pp!)
+      method_names %w[p p! pp pp!]
     end
 
     MSG = "Possibly forgotten debug-related `%s` call detected"

src/ameba/rule/lint/empty_expression.cr

@@ -28,8 +28,6 @@ module Ameba::Rule::Lint
   #   Enabled: true
   # ```
   class EmptyExpression < Base
-    include AST::Util
-
     properties do
       description "Disallows empty expressions"
     end

src/ameba/rule/lint/literals_comparison.cr

@@ -23,7 +23,7 @@ module Ameba::Rule::Lint
       description "Identifies comparisons between literals"
     end
 
-    OP_NAMES = %w(=== == !=)
+    OP_NAMES = %w[=== == !=]
 
     MSG        = "Comparison always evaluates to %s"
     MSG_LIKELY = "Comparison most likely evaluates to %s"
@@ -36,14 +36,15 @@ module Ameba::Rule::Lint
       arg_is_literal, arg_is_static = literal_kind?(arg)
 
       return unless obj_is_literal && arg_is_literal
+      return unless obj.to_s == arg.to_s
 
       is_dynamic = !obj_is_static || !arg_is_static
 
       what =
         case node.name
         when "===" then "the same"
-        when "=="  then (obj.to_s == arg.to_s).to_s
-        when "!="  then (obj.to_s != arg.to_s).to_s
+        when "=="  then "true"
+        when "!="  then "false"
         end
 
       issue_for node, (is_dynamic ? MSG_LIKELY : MSG) % what

src/ameba/rule/lint/missing_block_argument.cr

@@ -20,8 +20,6 @@ module Ameba::Rule::Lint
   #   Enabled: true
   # ```
   class MissingBlockArgument < Base
-    include AST::Util
-
     properties do
       description "Disallows yielding method definitions without block argument"
     end
@@ -36,10 +34,7 @@ module Ameba::Rule::Lint
     def test(source, node : Crystal::Def, scope : AST::Scope)
       return if !scope.yields? || node.block_arg
 
-      return unless location = node.name_location
-      end_location = name_end_location(node)
-
-      issue_for location, end_location, MSG
+      issue_for node, MSG, prefer_name_location: true
     end
   end
 end

src/ameba/rule/lint/not_nil.cr

@@ -26,32 +26,21 @@ module Ameba::Rule::Lint
   #   Enabled: true
   # ```
   class NotNil < Base
-    include AST::Util
-
     properties do
       description "Identifies usage of `not_nil!` calls"
     end
 
-    NOT_NIL_NAME = "not_nil!"
-    MSG          = "Avoid using `not_nil!`"
+    MSG = "Avoid using `not_nil!`"
 
     def test(source)
-      AST::NodeVisitor.new self, source, skip: [
-        Crystal::Macro,
-        Crystal::MacroExpression,
-        Crystal::MacroIf,
-        Crystal::MacroFor,
-      ]
+      AST::NodeVisitor.new self, source, skip: :macro
     end
 
     def test(source, node : Crystal::Call)
-      return unless node.name == NOT_NIL_NAME
+      return unless node.name == "not_nil!"
       return unless node.obj && node.args.empty?
 
-      return unless name_location = node.name_location
-      return unless end_location = name_end_location(node)
-
-      issue_for name_location, end_location, MSG
+      issue_for node, MSG, prefer_name_location: true
     end
   end
 end