Set timezone to `UTC` in CI

Fixes the case where formatter specs were deleting project's `.ameba.yml` file

.ameba.yml

@@ -0,0 +1,3 @@
+Lint/DocumentationAdmonition:
+  Timezone: UTC
+  Admonitions: [FIXME, BUG]

.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@v5
         with:
           context: .
           push: true

.github/workflows/ci.yml

@@ -18,13 +18,16 @@ jobs:
     runs-on: ${{ matrix.os }}
 
     steps:
+      - name: Set timezone to UTC
+        uses: szenius/set-timezone@v1.2
+
       - 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

.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

Makefile

@@ -30,9 +30,5 @@ bin: build
 	mkdir -p $(SHARD_BIN)
 	cp ./bin/ameba $(SHARD_BIN)
 
-.PHONY: run_file
-run_file:
-	cp -n ./bin/ameba.cr $(SHARD_BIN) || true
-
 .PHONY: test
 test: spec lint

shard.yml

@@ -1,5 +1,5 @@
 name: ameba
-version: 1.4.3
+version: 1.5.0
 
 authors:
   - Vitalii Elenhaupt <velenhaupt@gmail.com>
@@ -10,11 +10,12 @@ targets:
 
 scripts:
   # TODO: remove pre-compiled executable in future releases
-  postinstall: make bin && make run_file
+  postinstall: shards build -Dpreview_mt
 
 executables:
   - ameba
+  - ameba.cr
 
-crystal: "~> 1.7.0"
+crystal: "~> 1.9.0"
 
 license: MIT

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/cli/cmd_spec.cr

@@ -5,7 +5,7 @@ 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 -c spec/fixtures/config.yml spec/fixtures/source.cr)
         r.should be_nil
       end
     end
@@ -43,12 +43,12 @@ module Ameba::Cli
       end
 
       it "defaults rules? flag to false" do
-        c = Cli.parse_args %w(file.cr)
+        c = Cli.parse_args %w(spec/fixtures/source.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(spec/fixtures/source.cr)
         c.skip_reading_config?.should be_false
       end
 
@@ -58,7 +58,7 @@ module Ameba::Cli
       end
 
       it "defaults all? flag to false" do
-        c = Cli.parse_args %w(file.cr)
+        c = Cli.parse_args %w(spec/fixtures/source.cr)
         c.all?.should be_false
       end
 
@@ -95,35 +95,35 @@ module Ameba::Cli
 
       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 spec/fixtures/source.cr:3:5)
 
           location_to_explain = c.location_to_explain.should_not be_nil
-          location_to_explain[:file].should eq "src/file.cr"
+          location_to_explain[:file].should eq "spec/fixtures/source.cr"
           location_to_explain[:line].should eq 3
           location_to_explain[:column].should eq 5
         end
 
         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 spec/fixtures/source.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 spec/fixtures/source.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 spec/fixtures/source.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 spec/fixtures/source.cr)
           end
         end
       end

spec/ameba/config_spec.cr

@@ -2,17 +2,28 @@ 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
     end
 
     describe ".new" do
-      it "loads default globs when config is empty" do
-        yml = YAML.parse "{}"
-        config = Config.new(yml)
-        config.globs.should eq Config::DEFAULT_GLOBS
+      context "when config is empty" do
+        it "loads default globs" do
+          yml = YAML.parse "{}"
+          config = Config.new(yml)
+          config.globs.should eq ["**/*.cr"]
+        end
+
+        it "sets !lib as a default glob when there are .cr files in lib" do
+          File.touch "lib/shard.cr"
+          yml = YAML.parse "{}"
+          config = Config.new(yml)
+          config.globs.should eq ["**/*.cr", "!lib"]
+        ensure
+          File.delete "lib/shard.cr"
+        end
       end
 
       it "initializes globs as string" do
@@ -84,6 +95,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 foo.yml") do
+          Config.load "foo.yml"
+        end
+      end
+
       it "loads default config" do
         config = Config.load
         config.should_not be_nil
@@ -96,7 +113,7 @@ module Ameba
       config = Config.load config_sample
 
       it "holds source globs" do
-        config.globs.should eq Config::DEFAULT_GLOBS
+        config.globs.should eq ["**/*.cr"]
       end
 
       it "allows to set globs" do

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

@@ -45,6 +45,30 @@ module Ameba
         subject.expand(["**/#{current_file_basename}", "**/#{current_file_basename}"])
           .should eq [current_file_path]
       end
+
+      it "raises an ArgumentError when the glob doesn't match any files" do
+        expect_raises(ArgumentError, "No files found matching foo/*") do
+          subject.expand(["foo/*"])
+        end
+      end
+
+      it "raises an ArgumentError when given a missing file" do
+        expect_raises(ArgumentError, "No files found matching foo.cr") do
+          subject.expand(["foo.cr"])
+        end
+      end
+
+      it "raises an ArgumentError when given a missing directory" do
+        expect_raises(ArgumentError, "No files found matching foo/") do
+          subject.expand(["foo/"])
+        end
+      end
+
+      it "raises an ArgumentError when given multiple arguments, one of which is missing" do
+        expect_raises(ArgumentError, "No files found matching foo.cr") do
+          subject.expand(["**/#{current_file_basename}", "foo.cr"])
+        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/lint/documentation_admonition_spec.cr

@@ -0,0 +1,113 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  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/lint/shadowing_outer_local_var_spec.cr

@@ -44,7 +44,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 +69,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/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/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/lint/useless_assign_spec.cr

@@ -233,7 +233,7 @@ module Ameba::Rule::Lint
       subject.catch(s).should be_valid
     end
 
-    context "when transformed" do
+    context "block unpacking" do
       it "does not report if the first arg is transformed and not used" do
         s = Source.new %(
           collection.each do |(a, b)|

spec/ameba/rule/performance/excessive_allocations_spec.cr

@@ -0,0 +1,58 @@
+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
+        "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/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/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/fixtures/config.yml

@@ -0,0 +1,4 @@
+Ameba/PerfRule:
+  Enabled: false
+Ameba/ErrorRule:
+  Enabled: false

spec/spec_helper.cr

@@ -282,6 +282,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/variabling/assignment.cr

@@ -76,6 +76,9 @@ module Ameba::AST
       end
     end
 
+    # TODO: Remove in a next release. BC for crystal <= 1.9.
+    # refs https://github.com/crystal-ameba/ameba/pull/407
+    #
     # Indicates whether the node is a transformed assignment by the compiler.
     # i.e.
     #

src/ameba/ast/visitors/scope_visitor.cr

@@ -21,7 +21,6 @@ module Ameba::AST
     }
 
     SPECIAL_NODE_NAMES = %w[super previous_def]
-    RECORD_NODE_NAME   = "record"
 
     @scope_queue = [] of Scope
     @current_scope : Scope
@@ -190,7 +189,7 @@ module Ameba::AST
     end
 
     private def record_macro?(node)
-      node.name == RECORD_NODE_NAME && node.args.first?.is_a?(Crystal::Path)
+      node.name == "record" && node.args.first?.is_a?(Crystal::Path)
     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
@@ -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

@@ -55,10 +55,7 @@ class Ameba::Config
     Path[XDG_CONFIG_HOME] / "ameba/config.yml",
   }
 
-  DEFAULT_GLOBS = %w(
-    **/*.cr
-    !lib
-  )
+  SOURCES_GLOB = "**/*.cr"
 
   getter rules : Array(Rule::Base)
   property severity = Severity::Convention
@@ -95,7 +92,7 @@ class Ameba::Config
     @rules = Rule.rules.map &.new(config).as(Rule::Base)
     @rule_groups = @rules.group_by &.group
     @excluded = load_array_section(config, "Excluded")
-    @globs = load_array_section(config, "Globs", DEFAULT_GLOBS)
+    @globs = load_array_section(config, "Globs", default_globs)
 
     return unless formatter_name = load_formatter_name(config)
     self.formatter = formatter_name
@@ -115,12 +112,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
+      raise ArgumentError.new("Config file does not exist #{path}") unless File.exists?(path)
+      return File.read(path)
     end
     each_config_path do |config_path|
       return File.read(config_path) if File.exists?(config_path)
@@ -238,6 +236,12 @@ class Ameba::Config
     end
   end
 
+  private def default_globs
+    [SOURCES_GLOB].tap do |globs|
+      globs.push("!lib") unless Dir["lib/**/*.cr"].empty?
+    end
+  end
+
   # :nodoc:
   module RuleConfig
     # Define rule properties

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,7 +26,7 @@ module Ameba::Formatter
     end
 
     private def generate_todo_config(issues)
-      file = File.new(Config::DEFAULT_PATH, mode: "w")
+      file = File.new(@config_path, mode: "w")
       file << header
       rule_issues_map(issues).each do |rule, rule_issues|
         file << "\n# Problems found: #{rule_issues.size}"

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

@@ -22,6 +22,7 @@ module Ameba
     def expand(globs)
       globs.flat_map do |glob|
         glob += "/**/*.cr" if File.directory?(glob)
+        raise ArgumentError.new("No files found matching #{glob}") if Dir[glob].empty?
         Dir[glob]
       end.uniq!
     end

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/rule/lint/documentation_admonition.cr

@@ -0,0 +1,96 @@
+module Ameba::Rule::Lint
+  # 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:
+  #
+  # ```
+  # Lint/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/not_nil.cr

@@ -32,15 +32,14 @@ module Ameba::Rule::Lint
       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: :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

src/ameba/rule/lint/not_nil_after_no_bang.cr

@@ -30,15 +30,14 @@ module Ameba::Rule::Lint
     BLOCK_CALL_NAMES = %w(index rindex find)
     CALL_NAMES       = %w(index rindex)
 
-    NOT_NIL_NAME = "not_nil!"
-    MSG          = "Use `%s! {...}` instead of `%s {...}.not_nil!`"
+    MSG = "Use `%s! {...}` instead of `%s {...}.not_nil!`"
 
     def test(source)
       AST::NodeVisitor.new self, source, skip: :macro
     end
 
     def test(source, node : Crystal::Call)
-      return unless node.name == NOT_NIL_NAME && node.args.empty?
+      return unless node.name == "not_nil!" && node.args.empty?
       return unless (obj = node.obj).is_a?(Crystal::Call)
       return unless obj.name.in?(obj.block ? BLOCK_CALL_NAMES : CALL_NAMES)
 

src/ameba/rule/performance/any_after_filter.cr

@@ -32,11 +32,10 @@ module Ameba::Rule::Performance
       filter_names %w(select reject)
     end
 
-    ANY_NAME = "any?"
-    MSG      = "Use `any? {...}` instead of `%s {...}.any?`"
+    MSG = "Use `any? {...}` instead of `%s {...}.any?`"
 
     def test(source, node : Crystal::Call)
-      return unless node.name == ANY_NAME && (obj = node.obj)
+      return unless node.name == "any?" && (obj = node.obj)
       return unless obj.is_a?(Crystal::Call) && obj.block && node.block.nil?
       return unless obj.name.in?(filter_names)
 

src/ameba/rule/performance/any_instead_of_empty.cr

@@ -34,11 +34,10 @@ module Ameba::Rule::Performance
       description "Identifies usage of arg-less `any?` calls"
     end
 
-    ANY_NAME = "any?"
-    MSG      = "Use `!{...}.empty?` instead of `{...}.any?`"
+    MSG = "Use `!{...}.empty?` instead of `{...}.any?`"
 
     def test(source, node : Crystal::Call)
-      return unless node.name == ANY_NAME
+      return unless node.name == "any?"
       return unless node.block.nil? && node.args.empty?
       return unless node.obj
 

src/ameba/rule/performance/compact_after_map.cr

@@ -26,18 +26,16 @@ module Ameba::Rule::Performance
       description "Identifies usage of `compact` calls that follow `map`"
     end
 
-    COMPACT_NAME = "compact"
-    MAP_NAME     = "map"
-    MSG          = "Use `compact_map {...}` instead of `map {...}.compact`"
+    MSG = "Use `compact_map {...}` instead of `map {...}.compact`"
 
     def test(source)
       AST::NodeVisitor.new self, source, skip: :macro
     end
 
     def test(source, node : Crystal::Call)
-      return unless node.name == COMPACT_NAME && (obj = node.obj)
+      return unless node.name == "compact" && (obj = node.obj)
       return unless obj.is_a?(Crystal::Call) && obj.block
-      return unless obj.name == MAP_NAME
+      return unless obj.name == "map"
 
       issue_for obj.name_location, node.name_end_location, MSG
     end

src/ameba/rule/performance/excessive_allocations.cr

@@ -0,0 +1,70 @@
+require "./base"
+
+module Ameba::Rule::Performance
+  # This rule is used to identify excessive collection allocations,
+  # that can be avoided by using `each_<member>` instead of `<collection>.each`.
+  #
+  # For example, this is considered inefficient:
+  #
+  # ```
+  # "Alice".chars.each { |c| puts c }
+  # "Alice\nBob".lines.each { |l| puts l }
+  # ```
+  #
+  # And can be written as this:
+  #
+  # ```
+  # "Alice".each_char { |c| puts c }
+  # "Alice\nBob".each_line { |l| puts l }
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Performance/ExcessiveAllocations:
+  #   Enabled: true
+  #   CallNames:
+  #     codepoints: each_codepoint
+  #     graphemes: each_grapheme
+  #     chars: each_char
+  #     lines: each_line
+  # ```
+  class ExcessiveAllocations < Base
+    include AST::Util
+
+    properties do
+      description "Identifies usage of excessive collection allocations"
+      call_names({
+        "codepoints" => "each_codepoint",
+        "graphemes"  => "each_grapheme",
+        "chars"      => "each_char",
+        "lines"      => "each_line",
+        # "keys"       => "each_key",
+        # "values"     => "each_value",
+        # "children"   => "each_child",
+      })
+    end
+
+    MSG = "Use `%s {...}` instead of `%s.each {...}` to avoid excessive allocation"
+
+    def test(source)
+      AST::NodeVisitor.new self, source, skip: :macro
+    end
+
+    def test(source, node : Crystal::Call)
+      return unless node.name == "each" && node.args.empty? && node.block
+      return unless (obj = node.obj).is_a?(Crystal::Call)
+      return unless obj.args.empty? && obj.block.nil?
+      return unless method = call_names[obj.name]?
+
+      return unless name_location = obj.name_location
+      return unless end_location = name_end_location(node)
+
+      msg = MSG % {method, obj.name}
+
+      issue_for name_location, end_location, msg do |corrector|
+        corrector.replace(name_location, end_location, method)
+      end
+    end
+  end
+end

src/ameba/rule/performance/flatten_after_map.cr

@@ -26,18 +26,16 @@ module Ameba::Rule::Performance
       description "Identifies usage of `flatten` calls that follow `map`"
     end
 
-    FLATTEN_NAME = "flatten"
-    MAP_NAME     = "map"
-    MSG          = "Use `flat_map {...}` instead of `map {...}.flatten`"
+    MSG = "Use `flat_map {...}` instead of `map {...}.flatten`"
 
     def test(source)
       AST::NodeVisitor.new self, source, skip: :macro
     end
 
     def test(source, node : Crystal::Call)
-      return unless node.name == FLATTEN_NAME && (obj = node.obj)
+      return unless node.name == "flatten" && (obj = node.obj)
       return unless obj.is_a?(Crystal::Call) && obj.block
-      return unless obj.name == MAP_NAME
+      return unless obj.name == "map"
 
       issue_for obj.name_location, node.name_end_location, MSG
     end

src/ameba/rule/performance/minmax_after_map.cr

@@ -0,0 +1,67 @@
+require "./base"
+
+module Ameba::Rule::Performance
+  # This rule is used to identify usage of `min/max/minmax` calls that follow `map`.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # %w[Alice Bob].map(&.size).min
+  # %w[Alice Bob].map(&.size).max
+  # %w[Alice Bob].map(&.size).minmax
+  # ```
+  #
+  # And it should be written as this:
+  #
+  # ```
+  # %w[Alice Bob].min_of(&.size)
+  # %w[Alice Bob].max_of(&.size)
+  # %w[Alice Bob].minmax_of(&.size)
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Performance/MinMaxAfterMap:
+  #   Enabled: true
+  # ```
+  class MinMaxAfterMap < Base
+    properties do
+      description "Identifies usage of `min/max/minmax` calls that follow `map`"
+    end
+
+    MSG        = "Use `%s {...}` instead of `map {...}.%s`."
+    CALL_NAMES = %w[min min? max max? minmax minmax?]
+
+    def test(source)
+      AST::NodeVisitor.new self, source, skip: :macro
+    end
+
+    def test(source, node : Crystal::Call)
+      return unless node.name.in?(CALL_NAMES) && node.block.nil? && node.args.empty?
+      return unless (obj = node.obj) && obj.is_a?(Crystal::Call)
+      return unless obj.name == "map" && obj.block && obj.args.empty?
+
+      return unless name_location = obj.name_location
+      return unless end_location = node.name_end_location
+
+      of_name = node.name.sub(/(.+?)(\?)?$/, "\\1_of\\2")
+      message = MSG % {of_name, node.name}
+
+      issue_for name_location, end_location, message do |corrector|
+        next unless node_name_location = node.name_location
+
+        # TODO: switching the order of the below calls breaks the corrector
+        corrector.replace(
+          name_location,
+          name_location.adjust(column_number: {{ "map".size - 1 }}),
+          of_name
+        )
+        corrector.remove(
+          node_name_location.adjust(column_number: -1),
+          end_location
+        )
+      end
+    end
+  end
+end

src/ameba/rule/performance/size_after_filter.cr

@@ -38,15 +38,14 @@ module Ameba::Rule::Performance
       filter_names %w(select reject)
     end
 
-    SIZE_NAME = "size"
-    MSG       = "Use `count {...}` instead of `%s {...}.size`."
+    MSG = "Use `count {...}` instead of `%s {...}.size`."
 
     def test(source)
       AST::NodeVisitor.new self, source, skip: :macro
     end
 
     def test(source, node : Crystal::Call)
-      return unless node.name == SIZE_NAME && (obj = node.obj)
+      return unless node.name == "size" && (obj = node.obj)
       return unless obj.is_a?(Crystal::Call) && obj.block
       return unless obj.name.in?(filter_names)
 

src/ameba/rule/style/verbose_block.cr

@@ -99,6 +99,9 @@ module Ameba::Rule::Style
         node.named_args.try &.each do |arg|
           i += reference_count(arg.value, obj)
         end
+      when Crystal::BinaryOp
+        i += reference_count(node.left, obj)
+        i += reference_count(node.right, obj)
       when Crystal::Block
         i += reference_count(node.body, obj)
       when Crystal::Var
@@ -230,7 +233,7 @@ module Ameba::Rule::Style
       # we filter out the blocks that are of call type - `i.to_i64.odd?`
       return unless (body = block.body).is_a?(Crystal::Call)
 
-      # we need to "unwind" the chain challs, so the final receiver object
+      # we need to "unwind" the chain calls, so the final receiver object
       # ends up being a variable - `i`
       obj = body.obj
       while obj.is_a?(Crystal::Call)

src/ameba/spec/expect_issue.cr

@@ -109,7 +109,7 @@ module Ameba::Spec::ExpectIssue
     code = lines.join('\n')
 
     if code == annotated_code
-      raise "Use `report_no_issues` to assert that no issues are found"
+      raise "Use `expect_no_issues` to assert that no issues are found"
     end
 
     source, actual_annotations = actual_annotations(rules, code, path, lines)