diff --git a/README.md b/README.md
index b901259a1..fd5d184ea 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@
Code style linter for Crystal
- (a single-celled animal that catches food and moves about by extending fingerlike projections of protoplasm)
+ (a single-celled animal that catches food and moves about by extending fingerlike projections of protoplasm)
@@ -35,7 +35,7 @@
## About
Ameba is a static code analysis tool for the Crystal language.
-It enforces a consistent [Crystal code style](https://crystal-lang.org/docs/conventions/coding_style.html),
+It enforces a consistent [Crystal code style](https://crystal-lang.org/reference/conventions/coding_style.html),
also catches code smells and wrong code constructions.
See also [Roadmap](https://github.com/crystal-ameba/ameba/wiki).
@@ -46,7 +46,7 @@ Run `ameba` binary within your project directory to catch code issues:
```sh
$ ameba
-Inspecting 107 files.
+Inspecting 107 files
...............F.....................F....................................................................
@@ -61,9 +61,7 @@ src/ameba/formatter/base_formatter.cr:12:7
^
Finished in 542.64 milliseconds
-
-129 inspected, 2 failures.
-
+129 inspected, 2 failures
```
### Run in parallel
diff --git a/shard.yml b/shard.yml
index e40c86dcd..e64b2a966 100644
--- a/shard.yml
+++ b/shard.yml
@@ -1,5 +1,5 @@
name: ameba
-version: 0.13.4
+version: 0.14.0
authors:
- Vitalii Elenhaupt
diff --git a/spec/ameba/ast/scope_spec.cr b/spec/ameba/ast/scope_spec.cr
index 5a629619d..2e97ab6c7 100644
--- a/spec/ameba/ast/scope_spec.cr
+++ b/spec/ameba/ast/scope_spec.cr
@@ -113,7 +113,7 @@ module Ameba::AST
it "adds a new variable to the scope" do
scope = Scope.new as_node("")
scope.add_variable(Crystal::Var.new "foo")
- scope.variables.any?.should be_true
+ scope.variables.empty?.should be_false
end
end
diff --git a/spec/ameba/ast/variabling/variable_spec.cr b/spec/ameba/ast/variabling/variable_spec.cr
index fc39ef0d1..66b1a1882 100644
--- a/spec/ameba/ast/variabling/variable_spec.cr
+++ b/spec/ameba/ast/variabling/variable_spec.cr
@@ -48,7 +48,7 @@ module Ameba::AST
it "assigns the variable (creates a new assignment)" do
variable = Variable.new(var_node, scope)
variable.assign(assign_node, scope)
- variable.assignments.any?.should be_true
+ variable.assignments.empty?.should be_false
end
it "can create multiple assignments" do
@@ -64,7 +64,7 @@ module Ameba::AST
variable = Variable.new(var_node, scope)
variable.assign(as_node("foo=1"), scope)
variable.reference(var_node, scope)
- variable.references.any?.should be_true
+ variable.references.empty?.should be_false
end
it "adds a reference to the scope" do
diff --git a/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr b/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr
new file mode 100644
index 000000000..0575ff001
--- /dev/null
+++ b/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr
@@ -0,0 +1,17 @@
+require "../../../spec_helper"
+
+module Ameba::AST
+ describe TopLevelNodesVisitor do
+ describe "#require_nodes" do
+ it "returns require node" do
+ source = Source.new %(
+ require "foo"
+ def bar; end
+ )
+ visitor = TopLevelNodesVisitor.new(source.ast)
+ visitor.require_nodes.size.should eq 1
+ visitor.require_nodes.first.to_s.should eq %q(require "foo")
+ end
+ end
+ end
+end
diff --git a/spec/ameba/cli/cmd_spec.cr b/spec/ameba/cli/cmd_spec.cr
index f44ff6c6b..def5b6bff 100644
--- a/spec/ameba/cli/cmd_spec.cr
+++ b/spec/ameba/cli/cmd_spec.cr
@@ -42,6 +42,16 @@ module Ameba::Cli
c.except.should eq %w(RULE1 RULE2)
end
+ it "defaults rules? flag to false" do
+ c = Cli.parse_args %w(file.cr)
+ c.rules?.should eq false
+ end
+
+ it "accepts --rules flag" do
+ 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.all?.should eq false
diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr
index c92768737..d7547a2a9 100644
--- a/spec/ameba/config_spec.cr
+++ b/spec/ameba/config_spec.cr
@@ -120,7 +120,7 @@ module Ameba
it "returns list of sources" do
config.sources.size.should be > 0
config.sources.first.should be_a Source
- config.sources.any? { |s| s.fullpath == __FILE__ }.should be_true
+ config.sources.any?(&.fullpath.==(__FILE__)).should be_true
end
it "returns a list of sources mathing globs" do
@@ -130,7 +130,7 @@ module Ameba
it "returns a lisf of sources excluding 'Excluded'" do
config.excluded = %w(**/config_spec.cr)
- config.sources.any? { |s| s.fullpath == __FILE__ }.should be_false
+ config.sources.any?(&.fullpath.==(__FILE__)).should be_false
end
end
diff --git a/spec/ameba/formatter/dot_formatter_spec.cr b/spec/ameba/formatter/dot_formatter_spec.cr
index 444426228..3ba909838 100644
--- a/spec/ameba/formatter/dot_formatter_spec.cr
+++ b/spec/ameba/formatter/dot_formatter_spec.cr
@@ -8,7 +8,7 @@ module Ameba::Formatter
describe "#started" do
it "writes started message" do
subject.started [Source.new ""]
- output.to_s.should eq "Inspecting 1 file.\n\n"
+ output.to_s.should eq "Inspecting 1 file\n\n"
end
end
@@ -29,7 +29,7 @@ module Ameba::Formatter
describe "#finished" do
it "writes a final message" do
subject.finished [Source.new ""]
- output.to_s.should contain "1 inspected, 0 failures."
+ output.to_s.should contain "1 inspected, 0 failures"
end
it "writes the elapsed time" do
@@ -45,7 +45,7 @@ module Ameba::Formatter
end
subject.finished [s]
log = output.to_s
- log.should contain "1 inspected, 2 failures."
+ log.should contain "1 inspected, 2 failures"
log.should contain "DummyRuleError"
log.should contain "NamedRuleError"
end
@@ -60,7 +60,7 @@ module Ameba::Formatter
end
subject.finished [s]
log = output.to_s
- log.should contain "> a = 22"
+ log.should contain "> \e[97ma = 22"
log.should contain " \e[33m^\e[0m"
end
@@ -99,7 +99,7 @@ module Ameba::Formatter
s.add_issue(DummyRule.new, location: {1, 1},
message: "DummyRuleError", status: :disabled)
subject.finished [s]
- output.to_s.should contain "1 inspected, 0 failures."
+ output.to_s.should contain "1 inspected, 0 failures"
end
end
end
diff --git a/spec/ameba/formatter/util_spec.cr b/spec/ameba/formatter/util_spec.cr
index e2f95423d..9087228f5 100644
--- a/spec/ameba/formatter/util_spec.cr
+++ b/spec/ameba/formatter/util_spec.cr
@@ -8,6 +8,65 @@ module Ameba::Formatter
subject = Subject.new
describe Util do
+ describe "#deansify" do
+ it "returns given string without ANSI codes" do
+ str = String.build do |io|
+ io << "foo".colorize.green.underline
+ io << '-'
+ io << "bar".colorize.red.underline
+ end
+ subject.deansify("foo-bar").should eq "foo-bar"
+ subject.deansify(str).should eq "foo-bar"
+ end
+ end
+
+ describe "#trim" do
+ it "trims string longer than :max_length" do
+ subject.trim(("+" * 300), 1).should eq "+"
+ subject.trim(("+" * 300), 3).should eq "+++"
+ subject.trim(("+" * 300), 5).should eq "+ ..."
+ subject.trim(("+" * 300), 7).should eq "+++ ..."
+ end
+
+ it "leaves intact string shorter than :max_length" do
+ subject.trim(("+" * 3), 100).should eq "+++"
+ end
+
+ it "allows to use custom ellipsis" do
+ subject.trim(("+" * 300), 3, "…").should eq "++…"
+ end
+ end
+
+ describe "#context" do
+ it "returns correct pre/post context lines" do
+ source = Source.new <<-EOF
+ # pre:1
+ # pre:2
+ # pre:3
+ # pre:4
+ # pre:5
+ a = 1
+ # post:1
+ # post:2
+ # post:3
+ # post:4
+ # post:5
+ EOF
+
+ subject.context(source.lines, lineno: 6, context_lines: 3)
+ .should eq({<<-PRE.lines, <<-POST.lines
+ # pre:3
+ # pre:4
+ # pre:5
+ PRE
+ # post:1
+ # post:2
+ # post:3
+ POST
+ })
+ end
+ end
+
describe "#affected_code" do
it "returns nil if there is no such a line number" do
source = Source.new %(
@@ -22,7 +81,38 @@ module Ameba::Formatter
a = 1
)
location = Crystal::Location.new("filename", 1, 1)
- subject.affected_code(source, location).should eq "> a = 1\n \e[33m^\e[0m"
+ subject.deansify(subject.affected_code(source, location))
+ .should eq "> a = 1\n ^\n"
+ end
+
+ it "returns correct line if it is found" do
+ source = Source.new <<-EOF
+ # pre:1
+ # pre:2
+ # pre:3
+ # pre:4
+ # pre:5
+ a = 1
+ # post:1
+ # post:2
+ # post:3
+ # post:4
+ # post:5
+ EOF
+
+ location = Crystal::Location.new("filename", 6, 1)
+ subject.deansify(subject.affected_code(source, location, context_lines: 3))
+ .should eq <<-STR
+ > # pre:3
+ > # pre:4
+ > # pre:5
+ > a = 1
+ ^
+ > # post:1
+ > # post:2
+ > # post:3
+
+ STR
end
end
end
diff --git a/spec/ameba/issue_spec.cr b/spec/ameba/issue_spec.cr
index 652f52473..b6488552e 100644
--- a/spec/ameba/issue_spec.cr
+++ b/spec/ameba/issue_spec.cr
@@ -42,9 +42,22 @@ module Ameba
location: nil,
end_location: nil,
message: "",
- status: :enabled
+ status: :disabled
- issue.status.should eq :enabled
+ issue.status.should eq Issue::Status::Disabled
+ issue.disabled?.should be_true
+ issue.enabled?.should be_false
+ end
+
+ it "sets status to :enabled by default" do
+ issue = Issue.new rule: DummyRule.new,
+ location: nil,
+ end_location: nil,
+ message: ""
+
+ issue.status.should eq Issue::Status::Enabled
+ issue.enabled?.should be_true
+ issue.disabled?.should be_false
end
end
end
diff --git a/spec/ameba/rule/lint/duplicated_require_spec.cr b/spec/ameba/rule/lint/duplicated_require_spec.cr
new file mode 100644
index 000000000..94122d0e7
--- /dev/null
+++ b/spec/ameba/rule/lint/duplicated_require_spec.cr
@@ -0,0 +1,48 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+ subject = DuplicatedRequire.new
+
+ describe DuplicatedRequire do
+ it "passes if there are no duplicated requires" do
+ source = Source.new %(
+ require "math"
+ require "big"
+ require "big/big_decimal"
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there are a duplicated requires" do
+ source = Source.new %(
+ require "big"
+ require "math"
+ require "big"
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports rule, pos and message" do
+ source = Source.new %(
+ require "./thing"
+ require "./thing"
+ require "./another_thing"
+ require "./another_thing"
+ ), "source.cr"
+
+ subject.catch(source).should_not be_valid
+
+ issue = source.issues.first
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:2:1"
+ issue.end_location.to_s.should eq ""
+ issue.message.should eq "Duplicated require of `./thing`"
+
+ issue = source.issues.last
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:4:1"
+ issue.end_location.to_s.should eq ""
+ issue.message.should eq "Duplicated require of `./another_thing`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/lint/redundant_with_object_spec.cr b/spec/ameba/rule/lint/redundant_with_object_spec.cr
index f6adc130e..bc3a9baf6 100644
--- a/spec/ameba/rule/lint/redundant_with_object_spec.cr
+++ b/spec/ameba/rule/lint/redundant_with_object_spec.cr
@@ -77,7 +77,7 @@ module Ameba::Rule::Lint
issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:2:14"
issue.end_location.to_s.should eq "source.cr:2:30"
- issue.message.should eq "Use each instead of each_with_object"
+ issue.message.should eq "Use `each` instead of `each_with_object`"
end
end
end
diff --git a/spec/ameba/rule/lint/spec_focus_spec.cr b/spec/ameba/rule/lint/spec_focus_spec.cr
new file mode 100644
index 000000000..328d6f066
--- /dev/null
+++ b/spec/ameba/rule/lint/spec_focus_spec.cr
@@ -0,0 +1,127 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+ describe SpecFocus do
+ subject = SpecFocus.new
+
+ it "does not report if spec is not focused" do
+ s = Source.new %(
+ context "context" {}
+ describe "describe" {}
+ it "it" {}
+ pending "pending" {}
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should be_valid
+ end
+
+ it "reports if there is a focused context" do
+ s = Source.new %(
+ context "context", focus: true do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+ end
+
+ it "reports if there is a focused describe block" do
+ s = Source.new %(
+ describe "describe", focus: true do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+ end
+
+ it "reports if there is a focused it block" do
+ s = Source.new %(
+ it "it", focus: true do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+ end
+
+ it "reports if there is a focused pending block" do
+ s = Source.new %(
+ pending "pending", focus: true do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+ end
+
+ it "reports if there is a spec item with `focus: false`" do
+ s = Source.new %(
+ it "it", focus: false do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+ end
+
+ it "does not report if there is non spec block with :focus" do
+ s = Source.new %(
+ some_method "foo", focus: true do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should be_valid
+ end
+
+ it "does not report if there is a tagged item with :focus" do
+ s = Source.new %(
+ it "foo", tags: "focus" do
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should be_valid
+ end
+
+ it "does not report if there are focused spec items without blocks" do
+ s = Source.new %(
+ describe "foo", focus: true
+ context "foo", focus: true
+ it "foo", focus: true
+ pending "foo", focus: true
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should be_valid
+ end
+
+ it "does not report if there are focused items out of spec file" do
+ s = Source.new %(
+ describe "foo", focus: true {}
+ context "foo", focus: true {}
+ it "foo", focus: true {}
+ pending "foo", focus: true {}
+ )
+
+ subject.catch(s).should be_valid
+ end
+
+ it "reports rule, pos and message" do
+ s = Source.new %(
+ it "foo", focus: true do
+ it "bar", focus: true {}
+ end
+ ), path: "source_spec.cr"
+
+ subject.catch(s).should_not be_valid
+
+ s.issues.size.should eq(2)
+
+ first, second = s.issues
+
+ first.rule.should_not be_nil
+ first.location.to_s.should eq "source_spec.cr:1:11"
+ first.end_location.to_s.should eq ""
+ first.message.should eq "Focused spec item detected"
+
+ second.rule.should_not be_nil
+ second.location.to_s.should eq "source_spec.cr:2:13"
+ second.end_location.to_s.should eq ""
+ second.message.should eq "Focused spec item detected"
+ end
+ end
+end
diff --git a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr
new file mode 100644
index 000000000..2a5f82276
--- /dev/null
+++ b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr
@@ -0,0 +1,46 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+ subject = AnyInsteadOfEmpty.new
+
+ describe AnyInsteadOfEmpty do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ [1, 2, 3].any?(&.zero?)
+ [1, 2, 3].any?(String)
+ [1, 2, 3].any?(1..3)
+ [1, 2, 3].any? { |e| e > 1 }
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is any? call without a block nor argument" do
+ source = Source.new %(
+ [1, 2, 3].any?
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "macro" do
+ it "reports in macro scope" do
+ source = Source.new %(
+ {{ [1, 2, 3].any? }}
+ )
+ subject.catch(source).should_not be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ source = Source.new path: "source.cr", code: %(
+ [1, 2, 3].any?
+ )
+ subject.catch(source).should_not be_valid
+ issue = source.issues.first
+
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:11"
+ issue.end_location.to_s.should eq "source.cr:1:15"
+ issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr
new file mode 100644
index 000000000..32504ce59
--- /dev/null
+++ b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr
@@ -0,0 +1,69 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+ subject = ChainedCallWithNoBang.new
+
+ describe ChainedCallWithNoBang do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ (1..3).select { |e| e > 1 }.sort!
+ (1..3).select { |e| e > 1 }.sort_by!(&.itself)
+ (1..3).select { |e| e > 1 }.uniq!
+ (1..3).select { |e| e > 1 }.shuffle!
+ (1..3).select { |e| e > 1 }.reverse!
+ (1..3).select { |e| e > 1 }.rotate!
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is select followed by reverse" do
+ source = Source.new %(
+ [1, 2, 3].select { |e| e > 1 }.reverse
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is select followed by reverse followed by other call" do
+ source = Source.new %(
+ [1, 2, 3].select { |e| e > 2 }.reverse.size
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "properties" do
+ it "allows to configure `call_names`" do
+ source = Source.new %(
+ [1, 2, 3].select { |e| e > 2 }.reverse
+ )
+ rule = ChainedCallWithNoBang.new
+ rule.call_names = %w(uniq)
+ rule.catch(source).should be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ source = Source.new path: "source.cr", code: <<-CODE
+ [1, 2, 3].select { |e| e > 1 }.reverse
+ CODE
+
+ subject.catch(source).should_not be_valid
+ source.issues.size.should eq 1
+
+ issue = source.issues.first
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:32"
+ issue.end_location.to_s.should eq "source.cr:1:39"
+
+ issue.message.should eq "Use bang method variant `reverse!` after chained `select` call"
+ end
+
+ context "macro" do
+ it "doesn't report in macro scope" do
+ source = Source.new %(
+ {{ [1, 2, 3].select { |e| e > 2 }.reverse }}
+ )
+ subject.catch(source).should be_valid
+ end
+ end
+ end
+end
diff --git a/spec/ameba/rule/performance/compact_after_map_spec.cr b/spec/ameba/rule/performance/compact_after_map_spec.cr
new file mode 100644
index 000000000..ad34c79d1
--- /dev/null
+++ b/spec/ameba/rule/performance/compact_after_map_spec.cr
@@ -0,0 +1,50 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+ subject = CompactAfterMap.new
+
+ describe CompactAfterMap do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ (1..3).compact_map(&.itself)
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "passes if there is map followed by a bang call" do
+ source = Source.new %(
+ (1..3).map(&.itself).compact!
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is map followed by compact call" do
+ source = Source.new %(
+ (1..3).map(&.itself).compact
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "macro" do
+ it "doesn't report in macro scope" do
+ source = Source.new %(
+ {{ [1, 2, 3].map(&.to_s).compact }}
+ )
+ subject.catch(source).should be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ s = Source.new %(
+ (1..3).map(&.itself).compact
+ ), "source.cr"
+ subject.catch(s).should_not be_valid
+ issue = s.issues.first
+
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:8"
+ issue.end_location.to_s.should eq "source.cr:1:29"
+ issue.message.should eq "Use `compact_map {...}` instead of `map {...}.compact`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/performance/flatten_after_map_spec.cr b/spec/ameba/rule/performance/flatten_after_map_spec.cr
new file mode 100644
index 000000000..71666c316
--- /dev/null
+++ b/spec/ameba/rule/performance/flatten_after_map_spec.cr
@@ -0,0 +1,43 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+ subject = FlattenAfterMap.new
+
+ describe FlattenAfterMap do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ %w[Alice Bob].flat_map(&.chars)
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is map followed by flatten call" do
+ source = Source.new %(
+ %w[Alice Bob].map(&.chars).flatten
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "macro" do
+ it "doesn't report in macro scope" do
+ source = Source.new %(
+ {{ %w[Alice Bob].map(&.chars).flatten }}
+ )
+ subject.catch(source).should be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ s = Source.new %(
+ %w[Alice Bob].map(&.chars).flatten
+ ), "source.cr"
+ subject.catch(s).should_not be_valid
+ issue = s.issues.first
+
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:15"
+ issue.end_location.to_s.should eq "source.cr:1:35"
+ issue.message.should eq "Use `flat_map {...}` instead of `map {...}.flatten`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/performance/map_instead_of_block_spec.cr b/spec/ameba/rule/performance/map_instead_of_block_spec.cr
new file mode 100644
index 000000000..6a8c468a7
--- /dev/null
+++ b/spec/ameba/rule/performance/map_instead_of_block_spec.cr
@@ -0,0 +1,59 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Performance
+ subject = MapInsteadOfBlock.new
+
+ describe MapInsteadOfBlock do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ (1..3).join(&.to_s)
+ (1..3).sum(&.*(2))
+ (1..3).product(&.*(2))
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is map followed by join without a block" do
+ source = Source.new %(
+ (1..3).map(&.to_s).join
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is map followed by join without a block (with argument)" do
+ source = Source.new %(
+ (1..3).map(&.to_s).join('.')
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is map followed by join with a block" do
+ source = Source.new %(
+ (1..3).map(&.to_s).join(&.itself)
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "macro" do
+ it "doesn't report in macro scope" do
+ source = Source.new %(
+ {{ [1, 2, 3].map(&.to_s).join }}
+ )
+ subject.catch(source).should be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ s = Source.new %(
+ (1..3).map(&.to_s).join
+ ), "source.cr"
+ subject.catch(s).should_not be_valid
+ issue = s.issues.first
+
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:8"
+ issue.end_location.to_s.should eq "source.cr:1:24"
+ issue.message.should eq "Use `join {...}` instead of `map {...}.join`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/style/is_a_filter_spec.cr b/spec/ameba/rule/style/is_a_filter_spec.cr
new file mode 100644
index 000000000..1a49c2d17
--- /dev/null
+++ b/spec/ameba/rule/style/is_a_filter_spec.cr
@@ -0,0 +1,64 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+ subject = IsAFilter.new
+
+ describe IsAFilter do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ [1, 2, nil].select(Int32)
+ [1, 2, nil].reject(Nil)
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is .is_a? call within select" do
+ source = Source.new %(
+ [1, 2, nil].select(&.is_a?(Int32))
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is .nil? call within reject" do
+ source = Source.new %(
+ [1, 2, nil].reject(&.nil?)
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "properties" do
+ it "allows to configure filter_names" do
+ source = Source.new %(
+ [1, 2, nil].reject(&.nil?)
+ )
+ rule = IsAFilter.new
+ rule.filter_names = %w(select)
+ rule.catch(source).should be_valid
+ end
+ end
+
+ context "macro" do
+ it "reports in macro scope" do
+ source = Source.new %(
+ {{ [1, 2, nil].reject(&.nil?) }}
+ )
+ subject.catch(source).should_not be_valid
+ end
+ end
+
+ it "reports rule, pos and message" do
+ source = Source.new path: "source.cr", code: %(
+ [1, 2, nil].reject(&.nil?)
+ )
+ subject.catch(source).should_not be_valid
+ source.issues.size.should eq 1
+
+ issue = source.issues.first
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:13"
+ issue.end_location.to_s.should eq "source.cr:1:26"
+
+ issue.message.should eq "Use `reject(Nil)` instead of `reject {...}`"
+ end
+ end
+end
diff --git a/spec/ameba/rule/style/verbose_block_spec.cr b/spec/ameba/rule/style/verbose_block_spec.cr
new file mode 100644
index 000000000..3689cecb9
--- /dev/null
+++ b/spec/ameba/rule/style/verbose_block_spec.cr
@@ -0,0 +1,214 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+ subject = VerboseBlock.new
+
+ describe VerboseBlock do
+ it "passes if there is no potential performance improvements" do
+ source = Source.new %(
+ (1..3).any?(&.odd?)
+ (1..3).join('.', &.to_s)
+ (1..3).each_with_index { |i, idx| i * idx }
+ (1..3).map { |i| typeof(i) }
+ (1..3).map { |i| i || 0 }
+ (1..3).map { |i| :foo }
+ (1..3).map { |i| :foo.to_s.split.join('.') }
+ (1..3).map { :foo }
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "passes if the block argument is used within the body" do
+ source = Source.new %(
+ (1..3).map { |i| i * i }
+ (1..3).map { |j| j * j.to_i64 }
+ (1..3).map { |k| k.to_i64 * k }
+ (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 }
+ )
+ subject.catch(source).should be_valid
+ end
+
+ it "reports if there is a call with a collapsible block" do
+ source = Source.new %(
+ (1..3).any? { |i| i.odd? }
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is a call with an argument + collapsible block" do
+ source = Source.new %(
+ (1..3).join('.') { |i| i.to_s }
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ it "reports if there is a call with a collapsible block (with chained call)" do
+ source = Source.new %(
+ (1..3).map { |i| i.to_s.split.reverse.join.strip }
+ )
+ subject.catch(source).should_not be_valid
+ end
+
+ context "properties" do
+ it "#exclude_calls_with_block" do
+ source = Source.new %(
+ (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.exclude_calls_with_block = true)
+ .catch(source).should be_valid
+ rule
+ .tap(&.exclude_calls_with_block = false)
+ .catch(source).should_not be_valid
+ end
+
+ it "#exclude_multiple_line_blocks" do
+ source = Source.new %(
+ (1..3).any? do |i|
+ i.odd?
+ end
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.exclude_multiple_line_blocks = true)
+ .catch(source).should be_valid
+ rule
+ .tap(&.exclude_multiple_line_blocks = false)
+ .catch(source).should_not be_valid
+ end
+
+ it "#exclude_prefix_operators" do
+ source = Source.new %(
+ (1..3).sum { |i| +i }
+ (1..3).sum { |i| -i }
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.exclude_prefix_operators = true)
+ .catch(source).should be_valid
+ rule
+ .tap(&.exclude_prefix_operators = false)
+ .catch(source).should_not be_valid
+ end
+
+ it "#exclude_operators" do
+ source = Source.new %(
+ (1..3).sum { |i| i * 2 }
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.exclude_operators = true)
+ .catch(source).should be_valid
+ rule
+ .tap(&.exclude_operators = false)
+ .catch(source).should_not be_valid
+ end
+
+ it "#exclude_setters" do
+ source = Source.new %(
+ Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.exclude_setters = true)
+ .catch(source).should be_valid
+ rule
+ .tap(&.exclude_setters = false)
+ .catch(source).should_not be_valid
+ end
+
+ it "#max_line_length" do
+ source = Source.new %(
+ (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
+ i.to_s.reverse.strip.blank?
+ end
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.max_line_length = 60)
+ .catch(source).should be_valid
+ rule
+ .tap(&.max_line_length = nil)
+ .catch(source).should_not be_valid
+ end
+
+ it "#max_length" do
+ source = Source.new %(
+ (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
+ )
+ rule = VerboseBlock.new
+ rule
+ .tap(&.max_length = 30)
+ .catch(source).should be_valid
+ rule
+ .tap(&.max_length = nil)
+ .catch(source).should_not be_valid
+ end
+ end
+
+ context "macro" do
+ it "reports in macro scope" do
+ source = Source.new %(
+ {{ (1..3).any? { |i| i.odd? } }}
+ )
+ subject.catch(source).should_not be_valid
+ end
+ end
+
+ it "reports call args and named_args" do
+ short_block_variants = {
+ %|map(&.to_s.[start: 0.to_i64, count: 3]?)|,
+ %|map(&.to_s.[0.to_i64, count: 3]?)|,
+ %|map(&.to_s.[0.to_i64, 3]?)|,
+ %|map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))|,
+ %|map(&.to_s.[0.to_i64, count: 3]=("foo"))|,
+ %|map(&.to_s.[0.to_i64, 3]=("foo"))|,
+ %|map(&.to_s.camelcase(lower: true))|,
+ %|map(&.to_s.camelcase)|,
+ %|map(&.to_s.gsub('_', '-'))|,
+ %|map(&.in?(*{1, 2, 3}, **{foo: :bar}))|,
+ %|map(&.in?(1, *foo, 3, **bar))|,
+ %|join(separator: '.', &.to_s)|,
+ }
+
+ source = Source.new path: "source.cr", code: %(
+ (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? }
+ (1..3).map { |i| i.to_s[0.to_i64, count: 3]? }
+ (1..3).map { |i| i.to_s[0.to_i64, 3]? }
+ (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3] = "foo" }
+ (1..3).map { |i| i.to_s[0.to_i64, count: 3] = "foo" }
+ (1..3).map { |i| i.to_s[0.to_i64, 3] = "foo" }
+ (1..3).map { |i| i.to_s.camelcase(lower: true) }
+ (1..3).map { |i| i.to_s.camelcase }
+ (1..3).map { |i| i.to_s.gsub('_', '-') }
+ (1..3).map { |i| i.in?(*{1, 2, 3}, **{foo: :bar}) }
+ (1..3).map { |i| i.in?(1, *foo, 3, **bar) }
+ (1..3).join(separator: '.') { |i| i.to_s }
+ )
+ subject.catch(source).should_not be_valid
+ source.issues.size.should eq(short_block_variants.size)
+
+ source.issues.each_with_index do |issue, i|
+ issue.message.should eq(VerboseBlock::MSG % short_block_variants[i])
+ end
+ end
+
+ it "reports rule, pos and message" do
+ source = Source.new path: "source.cr", code: %(
+ (1..3).any? { |i| i.odd? }
+ )
+ subject.catch(source).should_not be_valid
+ source.issues.size.should eq 1
+
+ issue = source.issues.first
+ issue.rule.should_not be_nil
+ issue.location.to_s.should eq "source.cr:1:8"
+ issue.end_location.to_s.should eq "source.cr:1:26"
+
+ issue.message.should eq "Use short block notation instead: `any?(&.odd?)`"
+ end
+ end
+end
diff --git a/spec/ameba/severity_spec.cr b/spec/ameba/severity_spec.cr
index 94f3b7304..cf63c1334 100644
--- a/spec/ameba/severity_spec.cr
+++ b/spec/ameba/severity_spec.cr
@@ -4,9 +4,7 @@ module Ameba
describe Severity do
describe "#symbol" do
it "returns the symbol for each severity in the enum" do
- Severity.values.each do |severity|
- severity.symbol.should_not be_nil
- end
+ Severity.values.each(&.symbol.should_not(be_nil))
end
it "returns symbol for Error" do
diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr
index cd8af8759..fd4781fa4 100644
--- a/spec/ameba/source_spec.cr
+++ b/spec/ameba/source_spec.cr
@@ -23,6 +23,18 @@ module Ameba
end
end
+ describe "#spec?" do
+ it "returns true if the source is a spec file" do
+ s = Source.new "", "./source_spec.cr"
+ s.spec?.should be_true
+ end
+
+ it "returns false if the source is not a spec file" do
+ s = Source.new "", "./source.cr"
+ s.spec?.should be_false
+ end
+ end
+
describe "#matches_path?" do
it "returns true if source's path is matched" do
s = Source.new "", "source.cr"
diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr
index a1739becc..3f106efbb 100644
--- a/spec/spec_helper.cr
+++ b/spec/spec_helper.cr
@@ -3,7 +3,7 @@ require "../src/ameba"
module Ameba
# Dummy Rule which does nothing.
- struct DummyRule < Rule::Base
+ class DummyRule < Rule::Base
properties do
description : String = "Dummy rule that does nothing."
end
@@ -35,7 +35,7 @@ module Ameba
end
end
- struct NamedRule < Rule::Base
+ class NamedRule < Rule::Base
properties do
description "A rule with a custom name."
end
@@ -45,7 +45,7 @@ module Ameba
end
end
- struct ErrorRule < Rule::Base
+ class ErrorRule < Rule::Base
properties do
description "Always adds an error at 1:1"
end
@@ -55,7 +55,7 @@ module Ameba
end
end
- struct ScopeRule < Rule::Base
+ class ScopeRule < Rule::Base
@[YAML::Field(ignore: true)]
getter scopes = [] of AST::Scope
@@ -68,7 +68,7 @@ module Ameba
end
end
- struct FlowExpressionRule < Rule::Base
+ class FlowExpressionRule < Rule::Base
@[YAML::Field(ignore: true)]
getter expressions = [] of AST::FlowExpression
@@ -81,7 +81,7 @@ module Ameba
end
end
- struct RedundantControlExpressionRule < Rule::Base
+ class RedundantControlExpressionRule < Rule::Base
@[YAML::Field(ignore: true)]
getter nodes = [] of Crystal::ASTNode
@@ -95,7 +95,7 @@ module Ameba
end
# A rule that always raises an error
- struct RaiseRule < Rule::Base
+ class RaiseRule < Rule::Base
property should_raise = false
properties do
diff --git a/src/ameba.cr b/src/ameba.cr
index 4649b00c1..f16c62e60 100644
--- a/src/ameba.cr
+++ b/src/ameba.cr
@@ -20,7 +20,6 @@ require "./ameba/formatter/*"
#
# Ameba.run config
# ```
-#
module Ameba
extend self
@@ -35,7 +34,6 @@ module Ameba
# Ameba.run
# Ameba.run config
# ```
- #
def run(config = Config.load)
Runner.new(config).run
end
diff --git a/src/ameba/ast/branch.cr b/src/ameba/ast/branch.cr
index dcca21aa8..647ebafd5 100644
--- a/src/ameba/ast/branch.cr
+++ b/src/ameba/ast/branch.cr
@@ -44,7 +44,6 @@ module Ameba::AST
# end
# end
# ```
- #
def in_loop?
@parent.loop?
end
diff --git a/src/ameba/ast/branchable.cr b/src/ameba/ast/branchable.cr
index 2495dbb2a..4bf9819ef 100644
--- a/src/ameba/ast/branchable.cr
+++ b/src/ameba/ast/branchable.cr
@@ -37,8 +37,7 @@ module Ameba::AST
# Returns true if this node or one of the parent branchables is a loop, false otherwise.
def loop?
- return true if loop?(node)
- parent.try(&.loop?) || false
+ loop?(node) || parent.try(&.loop?) || false
end
end
end
diff --git a/src/ameba/ast/flow_expression.cr b/src/ameba/ast/flow_expression.cr
index 6b2149622..8c80eb531 100644
--- a/src/ameba/ast/flow_expression.cr
+++ b/src/ameba/ast/flow_expression.cr
@@ -59,8 +59,6 @@ module Ameba::AST
end
when Crystal::BinaryOp
unreachable_nodes << current_node.right if flow_expression?(current_node.left, in_loop?)
- else
- # nop
end
unreachable_nodes
diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr
index e1922b6e4..bacf42585 100644
--- a/src/ameba/ast/scope.cr
+++ b/src/ameba/ast/scope.cr
@@ -78,7 +78,7 @@ module Ameba::AST
# scope.find_variable("foo")
# ```
def find_variable(name : String)
- variables.find { |v| v.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.
@@ -117,8 +117,8 @@ module Ameba::AST
# Returns true instance variable assinged in this scope.
def assigns_ivar?(name)
- arguments.find { |arg| arg.name == name } &&
- ivariables.find { |var| var.name == "@#{name}" }
+ arguments.find(&.name.== name) &&
+ ivariables.find(&.name.== "@#{name}")
end
# Returns true if and only if current scope represents some
@@ -143,7 +143,7 @@ module Ameba::AST
# Returns true if current scope is a def, false if not.
def def?
- node.is_a? Crystal::Def
+ node.is_a?(Crystal::Def)
end
# Returns true if this scope is a top level scope, false if not.
@@ -173,7 +173,7 @@ module Ameba::AST
# the same Crystal node as `@node`.
def eql?(node)
node == @node &&
- !node.location.nil? &&
+ node.location &&
node.location == @node.location
end
end
diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr
index 28f55dd0a..0690e31b1 100644
--- a/src/ameba/ast/util.cr
+++ b/src/ameba/ast/util.cr
@@ -97,7 +97,6 @@ module Ameba::AST::Util
# ```
#
# That's because not all branches return(i.e. `else` is missing).
- #
def flow_expression?(node, in_loop = false)
return true if flow_command? node, in_loop
diff --git a/src/ameba/ast/variabling/assignment.cr b/src/ameba/ast/variabling/assignment.cr
index 6908572a2..e8bf04365 100644
--- a/src/ameba/ast/variabling/assignment.cr
+++ b/src/ameba/ast/variabling/assignment.cr
@@ -28,7 +28,6 @@ module Ameba::AST
# ```
# Assignment.new(node, variable, scope)
# ```
- #
def initialize(@node, @variable, @scope)
if scope = @variable.scope
@branch = Branch.of(@node, scope)
@@ -49,7 +48,7 @@ module Ameba::AST
# a ||= 1
# ```
def op_assign?
- node.is_a? Crystal::OpAssign
+ node.is_a?(Crystal::OpAssign)
end
# Returns true if this assignment is in a branch, false if not.
@@ -95,7 +94,6 @@ module Ameba::AST
# puts(b)
# end
# ```
- #
def transformed?
return false unless (assign = node).is_a?(Crystal::Assign)
return false unless (value = assign.value).is_a?(Crystal::Call)
diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr
index 59648c3d1..bb45877b4 100644
--- a/src/ameba/ast/variabling/variable.cr
+++ b/src/ameba/ast/variabling/variable.cr
@@ -27,7 +27,6 @@ module Ameba::AST
# ```
# Variable.new(node, scope)
# ```
- #
def initialize(@node, @scope)
end
@@ -45,7 +44,6 @@ module Ameba::AST
# variable.assign(node2)
# variable.assignment.size # => 2
# ```
- #
def assign(node, scope)
assignments << Assignment.new(node, self, scope)
@@ -60,7 +58,7 @@ module Ameba::AST
# variable.referenced? # => true
# ```
def referenced?
- references.any?
+ !references.empty?
end
# Creates a reference to this variable in some scope.
@@ -69,7 +67,6 @@ module Ameba::AST
# variable = Variable.new(node, scope)
# variable.reference(var_node, some_scope)
# ```
- #
def reference(node : Crystal::Var, scope : Scope)
Reference.new(node, scope).tap do |reference|
references << reference
@@ -91,8 +88,8 @@ module Ameba::AST
next if consumed_branches.includes?(assignment.branch)
assignment.referenced = true
- break unless assignment.branch
- consumed_branches << assignment.branch.not_nil!
+ break unless branch = assignment.branch
+ consumed_branches << branch
end
end
@@ -175,6 +172,10 @@ module Ameba::AST
node.accept self
end
+ def references?(node : Crystal::Var)
+ @macro_literals.any?(&.value.includes?(node.name))
+ end
+
def visit(node : Crystal::ASTNode)
true
end
@@ -203,7 +204,7 @@ module Ameba::AST
private def update_assign_reference!
if @assign_before_reference.nil? &&
references.size <= assignments.size &&
- assignments.none? { |ass| ass.op_assign? }
+ assignments.none?(&.op_assign?)
@assign_before_reference = assignments.find { |ass| !ass.in_branch? }.try &.node
end
end
diff --git a/src/ameba/ast/visitors/base_visitor.cr b/src/ameba/ast/visitors/base_visitor.cr
index 20a2ef5af..297bbe5db 100644
--- a/src/ameba/ast/visitors/base_visitor.cr
+++ b/src/ameba/ast/visitors/base_visitor.cr
@@ -15,7 +15,6 @@ module Ameba::AST
# ```
# visitor = Ameba::AST::NodeVisitor.new(rule, source)
# ```
- #
def initialize(@rule, @source)
@source.ast.accept self
end
diff --git a/src/ameba/ast/visitors/flow_expression_visitor.cr b/src/ameba/ast/visitors/flow_expression_visitor.cr
index f75e0d647..69ec1e25f 100644
--- a/src/ameba/ast/visitors/flow_expression_visitor.cr
+++ b/src/ameba/ast/visitors/flow_expression_visitor.cr
@@ -18,7 +18,6 @@ module Ameba::AST
if flow_expression?(node, in_loop?)
@rule.test @source, node, FlowExpression.new(node, in_loop?)
end
-
true
end
@@ -61,7 +60,7 @@ module Ameba::AST
end
private def in_loop?
- @loop_stack.any?
+ !@loop_stack.empty?
end
end
end
diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr
index 47bb343da..100c2bb49 100644
--- a/src/ameba/ast/visitors/node_visitor.cr
+++ b/src/ameba/ast/visitors/node_visitor.cr
@@ -40,7 +40,7 @@ module Ameba::AST
@skip : Array(Crystal::ASTNode.class)?
def initialize(@rule, @source, skip = nil)
- @skip = skip.try &.map { |el| el.as(Crystal::ASTNode.class) }
+ @skip = skip.try &.map(&.as(Crystal::ASTNode.class))
super @rule, @source
end
@@ -54,7 +54,7 @@ module Ameba::AST
{% end %}
def visit(node)
- return true unless (skip = @skip)
+ return true unless skip = @skip
!skip.includes?(node.class)
end
end
diff --git a/src/ameba/ast/visitors/redundant_control_expression_visitor.cr b/src/ameba/ast/visitors/redundant_control_expression_visitor.cr
index a1ef1c3df..100f7f21d 100644
--- a/src/ameba/ast/visitors/redundant_control_expression_visitor.cr
+++ b/src/ameba/ast/visitors/redundant_control_expression_visitor.cr
@@ -28,8 +28,6 @@ module Ameba::AST
when Crystal::Case then traverse_case node
when Crystal::BinaryOp then traverse_binary_op node
when Crystal::ExceptionHandler then traverse_exception_handler node
- else
- # ok
end
end
diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr
index 084e83aea..619278800 100644
--- a/src/ameba/ast/visitors/scope_visitor.cr
+++ b/src/ameba/ast/visitors/scope_visitor.cr
@@ -7,7 +7,6 @@ module Ameba::AST
RECORD_NODE_NAME = "record"
@scope_queue = [] of Scope
-
@current_scope : Scope
def initialize(@rule, @source)
@@ -169,7 +168,6 @@ module Ameba::AST
variable.reference(variable.node, @current_scope).explicit = false
end
end
-
true
when @current_scope.top_level? && record_macro?(node)
false
diff --git a/src/ameba/ast/visitors/top_level_nodes_visitor.cr b/src/ameba/ast/visitors/top_level_nodes_visitor.cr
new file mode 100644
index 000000000..6e606cae8
--- /dev/null
+++ b/src/ameba/ast/visitors/top_level_nodes_visitor.cr
@@ -0,0 +1,28 @@
+module Ameba::AST
+ # AST Visitor that visits certain nodes at a top level, which
+ # can characterize the source (i.e. require statements, modules etc.)
+ class TopLevelNodesVisitor < Crystal::Visitor
+ getter require_nodes = [] of Crystal::Require
+
+ # Creates a new instance of visitor
+ def initialize(@scope : Crystal::ASTNode)
+ @scope.accept(self)
+ end
+
+ # :nodoc:
+ def visit(node : Crystal::Require)
+ require_nodes << node
+ end
+
+ # If a top level node is Crystal::Expressions traverse the children.
+ def visit(node : Crystal::Expressions)
+ true
+ end
+
+ # A general visit method for rest of the nodes.
+ # Returns false meaning all child nodes will not be traversed.
+ def visit(node : Crystal::ASTNode)
+ false
+ end
+ end
+end
diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr
index 42778da28..821f2845c 100644
--- a/src/ameba/cli/cmd.cr
+++ b/src/ameba/cli/cmd.cr
@@ -8,12 +8,21 @@ module Ameba::Cli
def run(args = ARGV)
opts = parse_args args
config = Config.load opts.config, opts.colors?
- config.globs = opts.globs.not_nil! if opts.globs
- config.severity = opts.fail_level.not_nil! if opts.fail_level
+
+ if globs = opts.globs
+ config.globs = globs
+ end
+ if fail_level = opts.fail_level
+ config.severity = fail_level
+ end
configure_formatter(config, opts)
configure_rules(config, opts)
+ if opts.rules?
+ print_rules(config)
+ end
+
runner = Ameba.run(config)
if location = opts.location_to_explain
@@ -34,6 +43,7 @@ module Ameba::Cli
property except : Array(String)?
property location_to_explain : NamedTuple(file: String, line: Int32, column: Int32)?
property fail_level : Severity?
+ property? rules = false
property? all = false
property? colors = true
property? without_affected_code = false
@@ -44,13 +54,14 @@ module Ameba::Cli
parser.banner = "Usage: ameba [options] [file1 file2 ...]"
parser.on("-v", "--version", "Print version") { print_version }
- parser.on("-h", "--help", "Show this help") { show_help parser }
+ 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)
else
- opts.globs = f if f.any?
+ opts.globs = f unless f.empty?
end
end
@@ -66,12 +77,12 @@ module Ameba::Cli
parser.on("--only RULE1,RULE2,...",
"Run only given rules (or groups)") do |rules|
- opts.only = rules.split ","
+ opts.only = rules.split(',')
end
parser.on("--except RULE1,RULE2,...",
"Disable the given rules (or groups)") do |rules|
- opts.except = rules.split ","
+ opts.except = rules.split(',')
end
parser.on("--all", "Enables all available rules") do
@@ -84,7 +95,8 @@ module Ameba::Cli
opts.config = ""
end
- parser.on("--fail-level SEVERITY", "Change the level of failure to exit. Defaults to Convention") do |level|
+ parser.on("--fail-level SEVERITY",
+ "Change the level of failure to exit. Defaults to Convention") do |level|
opts.fail_level = Severity.parse(level)
end
@@ -107,13 +119,13 @@ module Ameba::Cli
end
private def configure_rules(config, opts)
- if only = opts.only
- config.rules.map! { |r| r.enabled = false; r }
+ case
+ when only = opts.only
+ config.rules.each(&.enabled = false)
config.update_rules(only, enabled: true)
- elsif opts.all?
- config.rules.map! { |r| r.enabled = true; r }
+ when opts.all?
+ config.rules.each(&.enabled = true)
end
-
config.update_rules(opts.except, enabled: false)
end
@@ -121,7 +133,8 @@ module Ameba::Cli
if name = opts.formatter
config.formatter = name
end
- config.formatter.config[:without_affected_code] = opts.without_affected_code?
+ config.formatter.config[:without_affected_code] =
+ opts.without_affected_code?
end
private def configure_explain_opts(loc, opts)
@@ -132,10 +145,15 @@ module Ameba::Cli
end
private def parse_explain_location(arg)
- location = arg.split(":", remove_empty: true).map &.strip
+ location = arg.split(':', remove_empty: true).map! &.strip
raise ArgumentError.new unless location.size === 3
+
file, line, column = location
- {file: file, line: line.to_i, column: column.to_i}
+ {
+ file: file,
+ line: line.to_i,
+ column: column.to_i,
+ }
rescue
raise "location should have PATH:line:column format"
end
@@ -145,8 +163,18 @@ module Ameba::Cli
exit 0
end
- private def show_help(parser)
+ private def print_help(parser)
puts parser
exit 0
end
+
+ private def print_rules(config)
+ config.rules.each do |rule|
+ puts \
+ "#{rule.name.colorize(:white)} " \
+ "[#{rule.severity.symbol.to_s.colorize(:green)}] - " \
+ "#{rule.description.colorize(:dark_gray)}"
+ end
+ exit 0
+ end
end
diff --git a/src/ameba/config.cr b/src/ameba/config.cr
index 1ecbd3a8c..381f87255 100644
--- a/src/ameba/config.cr
+++ b/src/ameba/config.cr
@@ -31,7 +31,6 @@ class Ameba::Config
!lib
)
- setter formatter : Formatter::BaseFormatter?
getter rules : Array(Rule::Base)
property severity = Severity::Convention
@@ -66,7 +65,9 @@ class Ameba::Config
@excluded = load_array_section(config, "Excluded")
@globs = load_array_section(config, "Globs", DEFAULT_GLOBS)
- self.formatter = load_formatter_name(config)
+ if formatter_name = load_formatter_name(config)
+ self.formatter = formatter_name
+ end
end
# Loads YAML configuration file by `path`.
@@ -74,7 +75,6 @@ class Ameba::Config
# ```
# config = Ameba::Config.load
# ```
- #
def self.load(path = PATH, colors = true)
Colorize.enabled = colors
content = File.exists?(path) ? File.read path : "{}"
@@ -84,7 +84,7 @@ class Ameba::Config
end
def self.formatter_names
- AVAILABLE_FORMATTERS.keys.join("|")
+ AVAILABLE_FORMATTERS.keys.join('|')
end
# Returns a list of sources matching globs and excluded sections.
@@ -96,7 +96,6 @@ class Ameba::Config
# config.excluded = ["spec"]
# config.sources # => list of sources pointing to files found by the wildcards
# ```
- #
def sources
(find_files_by_globs(globs) - find_files_by_globs(excluded))
.map { |path| Source.new File.read(path), path }
@@ -111,8 +110,8 @@ class Ameba::Config
# config.formatter
# ```
#
- def formatter
- @formatter ||= Formatter::DotFormatter.new
+ property formatter : Formatter::BaseFormatter do
+ Formatter::DotFormatter.new
end
# Sets formatter by name.
@@ -121,7 +120,6 @@ class Ameba::Config
# config = Ameba::Config.load
# config.formatter = :progress
# ```
- #
def formatter=(name : String | Symbol)
if f = AVAILABLE_FORMATTERS[name]?
@formatter = f.new
@@ -136,15 +134,13 @@ class Ameba::Config
# config = Ameba::Config.load
# config.update_rule "MyRuleName", enabled: false
# ```
- #
def update_rule(name, enabled = true, excluded = nil)
- index = @rules.index { |r| r.name == name }
- raise ArgumentError.new("Rule `#{name}` does not exist") unless index
+ rule = @rules.find(&.name.==(name))
+ raise ArgumentError.new("Rule `#{name}` does not exist") unless rule
- rule = @rules[index]
- rule.enabled = enabled
- rule.excluded = excluded
- @rules[index] = rule
+ rule
+ .tap(&.enabled = enabled)
+ .tap(&.excluded = excluded)
end
# Updates rules properties.
@@ -159,20 +155,22 @@ class Ameba::Config
# ```
# config.update_rules %w(Group1 Group2), enabled: true
# ```
- #
- def update_rules(names, **args)
+ def update_rules(names, enabled = true, excluded = nil)
names.try &.each do |name|
- if group = @rule_groups[name]?
- group.each { |rule| update_rule(rule.name, **args) }
+ if rules = @rule_groups[name]?
+ rules.each do |rule|
+ rule.enabled = enabled
+ rule.excluded = excluded
+ end
else
- update_rule name, **args
+ update_rule name, enabled, excluded
end
end
end
private def load_formatter_name(config)
name = config["Formatter"]?.try &.["Name"]?
- name ? name.to_s : nil
+ name.try(&.to_s)
end
private def load_array_section(config, section_name, default = [] of String)
@@ -269,7 +267,7 @@ class Ameba::Config
include YAML::Serializable::Strict
def self.new(config = nil)
- if (raw = config.try &.raw).is_a? Hash
+ if (raw = config.try &.raw).is_a?(Hash)
yaml = raw[rule_name]?.try &.to_yaml
end
from_yaml yaml || "{}"
diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr
index 396069e53..121ea1cf3 100644
--- a/src/ameba/formatter/dot_formatter.cr
+++ b/src/ameba/formatter/dot_formatter.cr
@@ -6,14 +6,15 @@ module Ameba::Formatter
class DotFormatter < BaseFormatter
include Util
- @started_at : Time?
+ @started_at : Time::Span?
@mutex = Thread::Mutex.new
# Reports a message when inspection is started.
def started(sources)
- @started_at = Time.utc # Time.monotonic
+ @started_at = Time.monotonic
- output << started_message(sources.size)
+ output.puts started_message(sources.size)
+ output.puts
end
# Reports a result of the inspection of a corresponding source.
@@ -35,32 +36,35 @@ module Ameba::Formatter
next if issue.disabled?
next if (location = issue.location).nil?
- output << "#{location}\n".colorize(:cyan)
- output << "[#{issue.rule.severity.symbol}] #{issue.rule.name}: #{issue.message}\n".colorize(:red)
+ output.puts location.colorize(:cyan)
+ output.puts \
+ "[#{issue.rule.severity.symbol}] " \
+ "#{issue.rule.name}: " \
+ "#{issue.message}".colorize(:red)
- if show_affected_code && (code = affected_code(source, location))
+ if show_affected_code && (code = affected_code(source, location, issue.end_location))
output << code.colorize(:default)
end
- output << "\n"
+ output.puts
end
end
- output << finished_in_message(@started_at, Time.utc) # Time.monotonic
- output << final_message(sources, failed_sources)
+ output.puts finished_in_message(@started_at, Time.monotonic)
+ output.puts final_message(sources, failed_sources)
end
private def started_message(size)
if size == 1
- "Inspecting 1 file.\n\n".colorize(:default)
+ "Inspecting 1 file".colorize(:default)
else
- "Inspecting #{size} files.\n\n".colorize(:default)
+ "Inspecting #{size} files".colorize(:default)
end
end
private def finished_in_message(started, finished)
if started && finished
- "Finished in #{to_human(finished - started)} \n\n".colorize(:default)
+ "Finished in #{to_human(finished - started)}".colorize(:default)
end
end
@@ -86,11 +90,11 @@ module Ameba::Formatter
private def final_message(sources, failed_sources)
total = sources.size
- failures = failed_sources.map { |f| f.issues.size }.sum
+ failures = failed_sources.sum(&.issues.size)
color = failures == 0 ? :green : :red
s = failures != 1 ? "s" : ""
- "#{total} inspected, #{failures} failure#{s}.\n".colorize color
+ "#{total} inspected, #{failures} failure#{s}".colorize(color)
end
end
end
diff --git a/src/ameba/formatter/explain_formatter.cr b/src/ameba/formatter/explain_formatter.cr
index 6670245ba..f8fe87c15 100644
--- a/src/ameba/formatter/explain_formatter.cr
+++ b/src/ameba/formatter/explain_formatter.cr
@@ -4,9 +4,8 @@ module Ameba::Formatter
# A formatter that shows the detailed explanation of the issue at
# a specific location.
class ExplainFormatter
- LINE_BREAK = "\n"
- HEADING = "## "
- PREFIX = " "
+ HEADING = "## "
+ PREFIX = " "
include Util
@@ -21,36 +20,36 @@ module Ameba::Formatter
# ExplainFormatter.new output,
# {file: path, line: line_number, column: column_number}
# ```
- #
- def initialize(@output, loc)
- @location = Crystal::Location.new(loc[:file], loc[:line], loc[:column])
+ def initialize(@output, location)
+ @location = Crystal::Location.new(location[:file], location[:line], location[:column])
end
# Reports the explainations at the *@location*.
def finished(sources)
- source = sources.find { |s| s.path == @location.filename }
-
+ source = sources.find(&.path.==(@location.filename))
return unless source
- source.issues.each do |issue|
- if (location = issue.location) &&
- location.line_number == @location.line_number &&
- location.column_number == @location.column_number
- explain(source, issue)
- end
- end
+ issue = source.issues.find(&.location.==(@location))
+ return unless issue
+
+ explain(source, issue)
end
private def explain(source, issue)
rule = issue.rule
+ location, end_location =
+ issue.location, issue.end_location
+
+ return unless location
+
output_title "ISSUE INFO"
output_paragraph [
issue.message.colorize(:red).to_s,
- @location.to_s.colorize(:cyan).to_s,
+ location.to_s.colorize(:cyan).to_s,
]
- if affected_code = affected_code(source, @location)
+ if affected_code = affected_code(source, location, end_location, context_lines: 3)
output_title "AFFECTED CODE"
output_paragraph affected_code
end
@@ -65,19 +64,19 @@ module Ameba::Formatter
end
private def output_title(title)
- output << HEADING.colorize(:yellow) << title.colorize(:yellow) << LINE_BREAK
- output << LINE_BREAK
+ output << HEADING.colorize(:yellow) << title.colorize(:yellow) << '\n'
+ output << '\n'
end
private def output_paragraph(paragraph : String)
- output_paragraph(paragraph.split(LINE_BREAK))
+ output_paragraph(paragraph.lines)
end
private def output_paragraph(paragraph : Array(String))
paragraph.each do |line|
- output << PREFIX << line << LINE_BREAK
+ output << PREFIX << line << '\n'
end
- output << LINE_BREAK
+ output << '\n'
end
end
end
diff --git a/src/ameba/formatter/flycheck_formatter.cr b/src/ameba/formatter/flycheck_formatter.cr
index 89423ff39..50d5de6ca 100644
--- a/src/ameba/formatter/flycheck_formatter.cr
+++ b/src/ameba/formatter/flycheck_formatter.cr
@@ -9,7 +9,7 @@ module Ameba::Formatter
@mutex.synchronize do
output.printf "%s:%d:%d: %s: [%s] %s\n",
source.path, loc.line_number, loc.column_number, e.rule.severity.symbol,
- e.rule.name, e.message.gsub("\n", " ")
+ e.rule.name, e.message.gsub('\n', " ")
end
end
end
diff --git a/src/ameba/formatter/todo_formatter.cr b/src/ameba/formatter/todo_formatter.cr
index fd0b81000..473144074 100644
--- a/src/ameba/formatter/todo_formatter.cr
+++ b/src/ameba/formatter/todo_formatter.cr
@@ -8,19 +8,20 @@ module Ameba::Formatter
def finished(sources)
super
- issues = sources.map(&.issues).flatten
+
+ issues = sources.flat_map(&.issues)
unless issues.any? { |issue| !issue.disabled? }
- @output << "No issues found. File is not generated.\n"
+ @output.puts "No issues found. File is not generated."
return
end
- if issues.any? { |issue| issue.syntax? }
- @output << "Unable to generate TODO file. Please fix syntax issues.\n"
+ if issues.any?(&.syntax?)
+ @output.puts "Unable to generate TODO file. Please fix syntax issues."
return
end
file = generate_todo_config issues
- @output << "Created #{file.path}\n"
+ @output.puts "Created #{file.path}"
file
end
@@ -40,7 +41,7 @@ module Ameba::Formatter
private def rule_issues_map(issues)
Hash(Rule::Base, Array(Issue)).new.tap do |h|
issues.each do |issue|
- next if issue.disabled? || issue.rule.is_a? Rule::Lint::Syntax
+ next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax)
(h[issue.rule] ||= Array(Issue).new) << issue
end
end
@@ -57,10 +58,9 @@ module Ameba::Formatter
end
private def rule_todo(rule, issues)
- rule.excluded =
- issues.map(&.location.try &.filename.try &.to_s)
- .compact
- .uniq!
+ rule.excluded = issues
+ .compact_map(&.location.try &.filename.try &.to_s)
+ .uniq!
{rule.name => rule}.to_yaml
end
diff --git a/src/ameba/formatter/util.cr b/src/ameba/formatter/util.cr
index 7219a67bb..d6c411c86 100644
--- a/src/ameba/formatter/util.cr
+++ b/src/ameba/formatter/util.cr
@@ -1,22 +1,109 @@
module Ameba::Formatter
module Util
- def affected_code(source, location, max_length = 100, placeholder = " ...", prompt = "> ")
- line, column = location.line_number, location.column_number
- affected_line = source.lines[line - 1]?
+ def deansify(message : String?) : String?
+ message.try &.gsub(/\x1b[^m]*m/, "").presence
+ end
+
+ def trim(str, max_length = 120, ellipsis = " ...")
+ if (str.size - ellipsis.size) > max_length
+ str = str[0, max_length]
+ if str.size > ellipsis.size
+ str = str[0...-ellipsis.size] + ellipsis
+ end
+ end
+ str
+ end
+
+ def context(lines, lineno, context_lines = 3, remove_empty = true)
+ pre_context, post_context = %w[], %w[]
+
+ lines.each_with_index do |line, i|
+ case i + 1
+ when lineno - context_lines...lineno
+ pre_context << line
+ when lineno + 1..lineno + context_lines
+ post_context << line
+ end
+ end
+
+ if remove_empty
+ # remove empty lines at the beginning ...
+ while pre_context.first?.try(&.blank?)
+ pre_context.shift
+ end
+ # ... and the end
+ while post_context.last?.try(&.blank?)
+ post_context.pop
+ end
+ end
+
+ {pre_context, post_context}
+ end
+
+ def affected_code(source, location, end_location = nil, context_lines = 0, max_length = 120, ellipsis = " ...", prompt = "> ")
+ lines = source.lines
+ lineno, column =
+ location.line_number, location.column_number
- return if affected_line.nil? || affected_line.strip.empty?
+ return unless affected_line = lines[lineno - 1]?.presence
- if affected_line.size > max_length && column < max_length
- affected_line = affected_line[0, max_length - placeholder.size - 1] + placeholder
+ if column < max_length
+ affected_line = trim(affected_line, max_length, ellipsis)
end
- stripped = affected_line.lstrip
- position = column - (affected_line.size - stripped.size) + prompt.size
+ show_context = context_lines > 0
+
+ if show_context
+ pre_context, post_context =
+ context(lines, lineno, context_lines)
+
+ position = prompt.size + column
+ position -= 1
+ else
+ affected_line_size, affected_line =
+ affected_line.size, affected_line.lstrip
+
+ position = column - (affected_line_size - affected_line.size) + prompt.size
+ position -= 1
+ end
String.build do |str|
- str << prompt << stripped << "\n"
- str << " " * (position - 1)
+ if show_context
+ pre_context.try &.each do |line|
+ line = trim(line, max_length, ellipsis)
+ str << prompt
+ str.puts(line.colorize(:dark_gray))
+ end
+ end
+
+ str << prompt
+ str.puts(affected_line.colorize(:white))
+
+ str << (" " * position)
str << "^".colorize(:yellow)
+
+ if end_location
+ end_lineno = end_location.line_number
+ end_column = end_location.column_number
+
+ if end_lineno == lineno && end_column > column
+ end_position = end_column - column
+ end_position -= 1
+
+ str << ("-" * end_position).colorize(:dark_gray)
+ str << "^".colorize(:yellow)
+ end
+ end
+
+ str.puts
+
+ if show_context
+ post_context.try &.each do |line|
+ line = trim(line, max_length, ellipsis)
+ str << prompt
+ str.puts(line.colorize(:dark_gray))
+ end
+ end
end
end
end
diff --git a/src/ameba/glob_utils.cr b/src/ameba/glob_utils.cr
index a714f8851..71cd0cb5f 100644
--- a/src/ameba/glob_utils.cr
+++ b/src/ameba/glob_utils.cr
@@ -7,12 +7,11 @@ module Ameba
# ```
# find_files_by_globs(["**/*.cr", "!lib"])
# ```
- #
def find_files_by_globs(globs)
rejected = rejected_globs(globs)
selected = globs - rejected
- expand(selected) - expand(rejected.map! { |p| p[1..-1] })
+ expand(selected) - expand(rejected.map!(&.[1..-1]))
end
# Expands globs. Globs can point to files or even directories.
@@ -20,12 +19,11 @@ module Ameba
# ```
# expand(["spec/*.cr", "src"]) # => all files in src folder + first level specs
# ```
- #
def expand(globs)
- globs.map do |glob|
+ globs.flat_map do |glob|
glob += "/**/*.cr" if File.directory?(glob)
Dir[glob]
- end.flatten.uniq!
+ end.uniq!
end
private def rejected_globs(globs)
diff --git a/src/ameba/inline_comments.cr b/src/ameba/inline_comments.cr
index cece57fec..65f70a248 100644
--- a/src/ameba/inline_comments.cr
+++ b/src/ameba/inline_comments.cr
@@ -36,9 +36,8 @@ module Ameba
# Time.epoch(1483859302)
# end
# ```
- #
def location_disabled?(location, rule)
- return false if Rule::SPECIAL.includes?(rule.name)
+ return false if rule.name.in?(Rule::SPECIAL)
return false unless line_number = location.try &.line_number.try &.- 1
return false unless line = lines[line_number]?
@@ -65,7 +64,6 @@ module Ameba
# line = "# # ameba:disable Rule1, Rule2"
# parse_inline_directive(line) # => nil
# ```
- #
def parse_inline_directive(line)
if directive = COMMENT_DIRECTIVE_REGEX.match(line)
return if commented_out?(line.gsub(directive[0], ""))
@@ -89,8 +87,10 @@ module Ameba
private def line_disabled?(line, rule)
return false unless directive = parse_inline_directive(line)
- Action.parse?(directive[:action]).try(&.disable?) &&
- (directive[:rules].includes?(rule.name) || directive[:rules].includes?(rule.group))
+ return false unless Action.parse?(directive[:action]).try(&.disable?)
+
+ directive[:rules].includes?(rule.name) ||
+ directive[:rules].includes?(rule.group)
end
private def commented_out?(line)
diff --git a/src/ameba/issue.cr b/src/ameba/issue.cr
index 03e95165b..b913eac3b 100644
--- a/src/ameba/issue.cr
+++ b/src/ameba/issue.cr
@@ -1,22 +1,31 @@
module Ameba
# Represents an issue reported by Ameba.
- record Issue,
+ struct Issue
+ enum Status
+ Enabled
+ Disabled
+ end
+
# A rule that triggers this issue.
- rule : Rule::Base,
+ getter rule : Rule::Base
# Location of the issue.
- location : Crystal::Location?,
+ getter location : Crystal::Location?
# End location of the issue.
- end_location : Crystal::Location?,
+ getter end_location : Crystal::Location?
# Issue message.
- message : String,
+ getter message : String
# Issue status.
- status : Symbol? do
- def disabled?
- status == :disabled
+ getter status : Status
+
+ delegate :enabled?, :disabled?,
+ to: status
+
+ def initialize(@rule, @location, @end_location, @message, status : Status? = nil)
+ @status = status || Status::Enabled
end
def syntax?
diff --git a/src/ameba/reportable.cr b/src/ameba/reportable.cr
index e31ef3a74..4632fa915 100644
--- a/src/ameba/reportable.cr
+++ b/src/ameba/reportable.cr
@@ -5,37 +5,46 @@ module Ameba
getter issues = [] of Issue
# Adds a new issue to the list of issues.
- def add_issue(rule, location : Crystal::Location?, end_location : Crystal::Location?, message, status = nil)
- status ||= :disabled if location_disabled?(location, rule)
- issues << Issue.new rule, location, end_location, message, status
+ def add_issue(rule, location, end_location, message, status : Issue::Status? = nil) : Issue
+ status ||=
+ Issue::Status::Disabled if location_disabled?(location, rule)
+
+ Issue.new(rule, location, end_location, message, status).tap do |issue|
+ issues << issue
+ end
end
- # Adds a new issue for AST *node*.
- def add_issue(rule, node : Crystal::ASTNode, message, **args)
- add_issue rule, node.location, node.end_location, message, **args
+ # Adds a new issue for Crystal AST *node*.
+ def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil) : Issue
+ add_issue rule, node.location, node.end_location, message, status
end
# Adds a new issue for Crystal *token*.
- def add_issue(rule, token : Crystal::Token, message, **args)
- add_issue rule, token.location, nil, message, **args
+ def add_issue(rule, token : Crystal::Token, message, status : Issue::Status? = nil) : Issue
+ add_issue rule, token.location, nil, message, status
end
# Adds a new issue for *location* defined by line and column numbers.
- def add_issue(rule, location : Tuple(Int32, Int32), message, **args)
- location = Crystal::Location.new path, *location
- add_issue rule, location, nil, message, **args
+ def add_issue(rule, location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue
+ location =
+ Crystal::Location.new(path, *location)
+
+ add_issue rule, location, nil, message, status
end
# Adds a new issue for *location* and *end_location* defined by line and column numbers.
- def add_issue(rule, location : Tuple(Int32, Int32), end_location : Tuple(Int32, Int32), message, **args)
- location = Crystal::Location.new path, *location
- end_location = Crystal::Location.new path, *end_location
- add_issue rule, location, end_location, message, **args
+ def add_issue(rule, location : {Int32, Int32}, end_location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue
+ location =
+ Crystal::Location.new(path, *location)
+ end_location =
+ Crystal::Location.new(path, *end_location)
+
+ add_issue rule, location, end_location, message, status
end
- # Returns true if the list of not disabled issues is empty, false otherwise.
+ # Returns `true` if the list of not disabled issues is empty, `false` otherwise.
def valid?
- issues.reject(&.disabled?).empty?
+ issues.none?(&.enabled?)
end
end
end
diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr
index c895499a2..414c469ab 100644
--- a/src/ameba/rule/base.cr
+++ b/src/ameba/rule/base.cr
@@ -10,7 +10,7 @@ module Ameba::Rule
# inherits from this struct:
#
# ```
- # struct MyRule < Ameba::Rule::Base
+ # class MyRule < Ameba::Rule::Base
# def test(source)
# if invalid?(source)
# issue_for line, column, "Something wrong."
@@ -26,8 +26,7 @@ module Ameba::Rule
# Enforces rules to implement an abstract `#test` method which
# is designed to test the source passed in. If source has issues
# that are tested by this rule, it should add an issue.
- #
- abstract struct Base
+ abstract class Base
include Config::RuleConfig
# This method is designed to test the source passed in. If source has issues
@@ -50,22 +49,20 @@ module Ameba::Rule
# source = MyRule.new.catch(source)
# source.valid?
# ```
- #
def catch(source : Source)
- source.tap { |s| test s }
+ source.tap { test source }
end
# Returns a name of this rule, which is basically a class name.
#
# ```
- # struct MyRule < Ameba::Rule::Base
+ # class MyRule < Ameba::Rule::Base
# def test(source)
# end
# end
#
# MyRule.new.name # => "MyRule"
# ```
- #
def name
{{@type}}.rule_name
end
@@ -73,13 +70,12 @@ module Ameba::Rule
# Returns a group this rule belong to.
#
# ```
- # struct MyGroup::MyRule < Ameba::Rule::Base
+ # class MyGroup::MyRule < Ameba::Rule::Base
# # ...
# end
#
# MyGroup::MyRule.new.group # => "MyGroup"
# ```
- #
def group
{{@type}}.group_name
end
@@ -91,11 +87,10 @@ module Ameba::Rule
# ```
# my_rule.excluded?(source) # => true or false
# ```
- #
def excluded?(source)
excluded.try &.any? do |path|
source.matches_path?(path) ||
- Dir.glob(path).any? { |glob| source.matches_path? glob }
+ Dir.glob(path).any? { |glob| source.matches_path?(glob) }
end
end
@@ -105,13 +100,12 @@ module Ameba::Rule
# ```
# my_rule.special? # => true or false
# ```
- #
def special?
- SPECIAL.includes? name
+ name.in?(SPECIAL)
end
def ==(other)
- name == other.try &.name
+ name == other.try(&.name)
end
def hash
@@ -123,11 +117,11 @@ module Ameba::Rule
end
protected def self.rule_name
- name.gsub("Ameba::Rule::", "").gsub("::", "/")
+ name.gsub("Ameba::Rule::", "").gsub("::", '/')
end
protected def self.group_name
- rule_name.split("/")[0...-1].join("/")
+ rule_name.split('/')[0...-1].join('/')
end
protected def self.subclasses
@@ -146,7 +140,7 @@ module Ameba::Rule
# module Ameba
# # This is a test rule.
# # Does nothing.
- # struct MyRule < Ameba::Rule::Base
+ # class MyRule < Ameba::Rule::Base
# def test(source)
# end
# end
@@ -156,8 +150,11 @@ module Ameba::Rule
# ```
def self.parsed_doc
source = File.read(path_to_source_file)
- nodes = Crystal::Parser.new(source).tap(&.wants_doc = true).parse
- type_name = rule_name.split("/").last?
+ nodes = Crystal::Parser.new(source)
+ .tap(&.wants_doc = true)
+ .parse
+ type_name = rule_name.split('/').last?
+
DocFinder.new(nodes, type_name).doc
end
@@ -190,7 +187,6 @@ module Ameba::Rule
# ```
# Ameba::Rule.rules # => [Rule1, Rule2, ....]
# ```
- #
def self.rules
Base.subclasses
end
diff --git a/src/ameba/rule/layout/line_length.cr b/src/ameba/rule/layout/line_length.cr
index a4c24d37d..41eb76fa4 100644
--- a/src/ameba/rule/layout/line_length.cr
+++ b/src/ameba/rule/layout/line_length.cr
@@ -8,8 +8,7 @@ module Ameba::Rule::Layout
# Enabled: true
# MaxLength: 100
# ```
- #
- struct LineLength < Base
+ class LineLength < Base
properties do
enabled false
description "Disallows lines longer than `MaxLength` number of symbols"
@@ -20,9 +19,7 @@ module Ameba::Rule::Layout
def test(source)
source.lines.each_with_index do |line, index|
- next unless line.size > max_length
-
- issue_for({index + 1, max_length + 1}, MSG)
+ issue_for({index + 1, max_length + 1}, MSG) if line.size > max_length
end
end
end
diff --git a/src/ameba/rule/layout/trailing_blank_lines.cr b/src/ameba/rule/layout/trailing_blank_lines.cr
index 24499e9c0..a133ebb8c 100644
--- a/src/ameba/rule/layout/trailing_blank_lines.cr
+++ b/src/ameba/rule/layout/trailing_blank_lines.cr
@@ -7,8 +7,7 @@ module Ameba::Rule::Layout
# Layout/TrailingBlankLines:
# Enabled: true
# ```
- #
- struct TrailingBlankLines < Base
+ class TrailingBlankLines < Base
properties do
description "Disallows trailing blank lines"
end
@@ -24,9 +23,9 @@ module Ameba::Rule::Layout
source_lines_size = source_lines.size
return if source_lines_size == 1 && last_source_line.empty?
- last_line_not_empty = !last_source_line.empty?
- if source_lines_size >= 1 && (source_lines.last(2).join.strip.empty? || last_line_not_empty)
- issue_for({source_lines_size - 1, 1}, last_line_not_empty ? MSG_FINAL_NEWLINE : MSG)
+ last_line_empty = last_source_line.empty?
+ if source_lines_size >= 1 && (source_lines.last(2).join.blank? || !last_line_empty)
+ issue_for({source_lines_size - 1, 1}, last_line_empty ? MSG : MSG_FINAL_NEWLINE)
end
end
end
diff --git a/src/ameba/rule/layout/trailing_whitespace.cr b/src/ameba/rule/layout/trailing_whitespace.cr
index a4df797ee..a80a56ec1 100644
--- a/src/ameba/rule/layout/trailing_whitespace.cr
+++ b/src/ameba/rule/layout/trailing_whitespace.cr
@@ -7,8 +7,7 @@ module Ameba::Rule::Layout
# Layout/TrailingWhitespace:
# Enabled: true
# ```
- #
- struct TrailingWhitespace < Base
+ class TrailingWhitespace < Base
properties do
description "Disallows trailing whitespaces"
end
@@ -17,8 +16,7 @@ module Ameba::Rule::Layout
def test(source)
source.lines.each_with_index do |line, index|
- next unless line =~ /\s$/
- issue_for({index + 1, line.size}, MSG)
+ issue_for({index + 1, line.size}, MSG) if line =~ /\s$/
end
end
end
diff --git a/src/ameba/rule/lint/bad_directive.cr b/src/ameba/rule/lint/bad_directive.cr
index 76b6d3589..3d70c2d50 100644
--- a/src/ameba/rule/lint/bad_directive.cr
+++ b/src/ameba/rule/lint/bad_directive.cr
@@ -17,8 +17,7 @@ module Ameba::Rule::Lint
# Lint/BadDirective:
# Enabled: true
# ```
- #
- struct BadDirective < Base
+ class BadDirective < Base
properties do
description "Reports bad comment directives"
end
@@ -48,7 +47,9 @@ module Ameba::Rule::Lint
private def check_rules(source, token, rules)
bad_names = rules - ALL_RULE_NAMES - ALL_GROUP_NAMES
- issue_for token, "Such rules do not exist: %s" % bad_names.join(", ") unless bad_names.empty?
+ return if bad_names.empty?
+
+ issue_for token, "Such rules do not exist: %s" % bad_names.join(", ")
end
end
end
diff --git a/src/ameba/rule/lint/comparison_to_boolean.cr b/src/ameba/rule/lint/comparison_to_boolean.cr
index 313db5a22..d95d68d70 100644
--- a/src/ameba/rule/lint/comparison_to_boolean.cr
+++ b/src/ameba/rule/lint/comparison_to_boolean.cr
@@ -19,23 +19,21 @@ module Ameba::Rule::Lint
# Lint/ComparisonToBoolean:
# Enabled: true
# ```
- #
- struct ComparisonToBoolean < Base
+ class ComparisonToBoolean < Base
properties do
enabled false
description "Disallows comparison to booleans"
end
- MSG = "Comparison to a boolean is pointless"
+ MSG = "Comparison to a boolean is pointless"
+ OP_NAMES = %w(== != ===)
def test(source, node : Crystal::Call)
- comparison = %w(== != ===).includes?(node.name)
- to_boolean = node.args.first?.try &.is_a?(Crystal::BoolLiteral) ||
+ comparison = node.name.in?(OP_NAMES)
+ to_boolean = node.args.first?.try(&.is_a?(Crystal::BoolLiteral)) ||
node.obj.is_a?(Crystal::BoolLiteral)
- return unless comparison && to_boolean
-
- issue_for node, MSG
+ issue_for node, MSG if comparison && to_boolean
end
end
end
diff --git a/src/ameba/rule/lint/debugger_statement.cr b/src/ameba/rule/lint/debugger_statement.cr
index 976744901..da478cbb4 100644
--- a/src/ameba/rule/lint/debugger_statement.cr
+++ b/src/ameba/rule/lint/debugger_statement.cr
@@ -10,8 +10,7 @@ module Ameba::Rule::Lint
# Lint/DebuggerStatement:
# Enabled: true
# ```
- #
- struct DebuggerStatement < Base
+ class DebuggerStatement < Base
properties do
description "Disallows calls to debugger"
end
diff --git a/src/ameba/rule/lint/duplicated_require.cr b/src/ameba/rule/lint/duplicated_require.cr
new file mode 100644
index 000000000..0e33dbb03
--- /dev/null
+++ b/src/ameba/rule/lint/duplicated_require.cr
@@ -0,0 +1,31 @@
+module Ameba::Rule::Lint
+ # A rule that reports duplicated require statements.
+ #
+ # ```
+ # require "./thing"
+ # require "./stuff"
+ # require "./thing" # duplicated require
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Lint/DuplicatedRequire:
+ # Enabled: true
+ # ```
+ class DuplicatedRequire < Base
+ properties do
+ description "Reports duplicated require statements"
+ end
+
+ MSG = "Duplicated require of `%s`"
+
+ def test(source)
+ nodes = AST::TopLevelNodesVisitor.new(source.ast).require_nodes
+ nodes.each_with_object([] of String) do |node, processed_require_strings|
+ issue_for(node, MSG % node.string) if processed_require_strings.includes?(node.string)
+ processed_require_strings << node.string
+ end
+ end
+ end
+end
diff --git a/src/ameba/rule/lint/empty_ensure.cr b/src/ameba/rule/lint/empty_ensure.cr
index f8fdeb48e..e363f824b 100644
--- a/src/ameba/rule/lint/empty_ensure.cr
+++ b/src/ameba/rule/lint/empty_ensure.cr
@@ -38,8 +38,7 @@ module Ameba::Rule::Lint
# Lint/EmptyEnsure
# Enabled: true
# ```
- #
- struct EmptyEnsure < Base
+ class EmptyEnsure < Base
properties do
description "Disallows empty ensure statement"
end
diff --git a/src/ameba/rule/lint/empty_expression.cr b/src/ameba/rule/lint/empty_expression.cr
index 682ed8a35..89ca5fdcf 100644
--- a/src/ameba/rule/lint/empty_expression.cr
+++ b/src/ameba/rule/lint/empty_expression.cr
@@ -27,13 +27,12 @@ module Ameba::Rule::Lint
# Lint/EmptyExpression:
# Enabled: true
# ```
- #
- struct EmptyExpression < Base
+ class EmptyExpression < Base
include AST::Util
properties do
- description "Disallows empty expressions"
enabled false
+ description "Disallows empty expressions"
end
MSG = "Avoid empty expression %s"
@@ -41,8 +40,7 @@ module Ameba::Rule::Lint
def test(source, node : Crystal::NilLiteral)
exp = node_source(node, source.lines).try &.join
-
- return if exp.nil? || exp == "nil"
+ return if exp.in?(nil, "nil")
issue_for node, MSG % exp
end
diff --git a/src/ameba/rule/lint/empty_loop.cr b/src/ameba/rule/lint/empty_loop.cr
index 19eafd2fe..b58e23664 100644
--- a/src/ameba/rule/lint/empty_loop.cr
+++ b/src/ameba/rule/lint/empty_loop.cr
@@ -37,7 +37,7 @@ module Ameba::Rule::Lint
# Lint/EmptyLoop:
# Enabled: true
# ```
- struct EmptyLoop < Base
+ class EmptyLoop < Base
include AST::Util
properties do
diff --git a/src/ameba/rule/lint/hash_duplicated_key.cr b/src/ameba/rule/lint/hash_duplicated_key.cr
index c9de6a788..c8ffcb4e6 100644
--- a/src/ameba/rule/lint/hash_duplicated_key.cr
+++ b/src/ameba/rule/lint/hash_duplicated_key.cr
@@ -19,8 +19,7 @@ module Ameba::Rule::Lint
# Lint/HashDuplicatedKey:
# Enabled: true
# ```
- #
- struct HashDuplicatedKey < Base
+ class HashDuplicatedKey < Base
properties do
description "Disallows duplicated keys in hash literals"
end
@@ -28,7 +27,7 @@ module Ameba::Rule::Lint
MSG = "Duplicated keys in hash literal: %s"
def test(source, node : Crystal::HashLiteral)
- return unless (keys = duplicated_keys(node.entries)).any?
+ return if (keys = duplicated_keys(node.entries)).empty?
issue_for node, MSG % keys.join(", ")
end
@@ -36,7 +35,7 @@ module Ameba::Rule::Lint
private def duplicated_keys(entries)
entries.map(&.key)
.group_by(&.itself)
- .select { |_, v| v.size > 1 }
+ .tap(&.select! { |_, v| v.size > 1 })
.map { |k, _| k }
end
end
diff --git a/src/ameba/rule/lint/literal_in_condition.cr b/src/ameba/rule/lint/literal_in_condition.cr
index 2dc2e6004..fc3ce23a2 100644
--- a/src/ameba/rule/lint/literal_in_condition.cr
+++ b/src/ameba/rule/lint/literal_in_condition.cr
@@ -19,8 +19,7 @@ module Ameba::Rule::Lint
# Lint/LiteralInCondition:
# Enabled: true
# ```
- #
- struct LiteralInCondition < Base
+ class LiteralInCondition < Base
include AST::Util
properties do
@@ -31,8 +30,7 @@ module Ameba::Rule::Lint
MSG = "Literal value found in conditional"
def check_node(source, node)
- return unless literal?(node.cond)
- issue_for node, MSG
+ issue_for node, MSG if literal?(node.cond)
end
def test(source, node : Crystal::If)
diff --git a/src/ameba/rule/lint/literal_in_interpolation.cr b/src/ameba/rule/lint/literal_in_interpolation.cr
index df674f74f..a8683d128 100644
--- a/src/ameba/rule/lint/literal_in_interpolation.cr
+++ b/src/ameba/rule/lint/literal_in_interpolation.cr
@@ -15,8 +15,7 @@ module Ameba::Rule::Lint
# Lint/LiteralInInterpolation
# Enabled: true
# ```
- #
- struct LiteralInInterpolation < Base
+ class LiteralInInterpolation < Base
include AST::Util
properties do
diff --git a/src/ameba/rule/lint/percent_array.cr b/src/ameba/rule/lint/percent_array.cr
index 0f1d3de82..3109fa151 100644
--- a/src/ameba/rule/lint/percent_array.cr
+++ b/src/ameba/rule/lint/percent_array.cr
@@ -23,12 +23,12 @@ module Ameba::Rule::Lint
# StringArrayUnwantedSymbols: ',"'
# SymbolArrayUnwantedSymbols: ',:'
# ```
- #
- struct PercentArrays < Base
+ class PercentArrays < Base
properties do
description "Disallows some unwanted symbols in percent array literals"
- string_array_unwanted_symbols ",\""
- symbol_array_unwanted_symbols ",:"
+
+ string_array_unwanted_symbols %(,")
+ symbol_array_unwanted_symbols %(,:)
end
MSG = "Symbols `%s` may be unwanted in %s array literals"
@@ -49,8 +49,6 @@ module Ameba::Rule::Lint
issue_for start_token.not_nil!, issue.not_nil!
end
issue = start_token = nil
- else
- # nop
end
end
end
@@ -61,14 +59,11 @@ module Ameba::Rule::Lint
check_array_entry entry, string_array_unwanted_symbols, "%w"
when .starts_with? "%i"
check_array_entry entry, symbol_array_unwanted_symbols, "%i"
- else
- # nop
end
end
private def check_array_entry(entry, symbols, literal)
- return unless entry =~ /[#{symbols}]/
- MSG % {symbols, literal}
+ MSG % {symbols, literal} if entry =~ /[#{symbols}]/
end
end
end
diff --git a/src/ameba/rule/lint/rand_zero.cr b/src/ameba/rule/lint/rand_zero.cr
index 6c2b70f28..565450bf0 100644
--- a/src/ameba/rule/lint/rand_zero.cr
+++ b/src/ameba/rule/lint/rand_zero.cr
@@ -22,8 +22,7 @@ module Ameba::Rule::Lint
# Lint/RandZero:
# Enabled: true
# ```
- #
- struct RandZero < Base
+ class RandZero < Base
properties do
description "Disallows rand zero calls"
end
@@ -34,9 +33,9 @@ module Ameba::Rule::Lint
return unless node.name == "rand" &&
node.args.size == 1 &&
(arg = node.args.first) &&
- (arg.is_a? Crystal::NumberLiteral) &&
+ arg.is_a?(Crystal::NumberLiteral) &&
(value = arg.value) &&
- (value == "0" || value == "1")
+ value.in?("0", "1")
issue_for node, MSG % node
end
diff --git a/src/ameba/rule/lint/redundant_string_coercion.cr b/src/ameba/rule/lint/redundant_string_coercion.cr
index f21484a22..0273e6d9b 100644
--- a/src/ameba/rule/lint/redundant_string_coercion.cr
+++ b/src/ameba/rule/lint/redundant_string_coercion.cr
@@ -20,8 +20,7 @@ module Ameba::Rule::Lint
# Lint/RedundantStringCoersion
# Enabled: true
# ```
- #
- struct RedundantStringCoercion < Base
+ class RedundantStringCoercion < Base
include AST::Util
properties do
@@ -31,7 +30,9 @@ module Ameba::Rule::Lint
MSG = "Redundant use of `Object#to_s` in interpolation"
def test(source, node : Crystal::StringInterpolation)
- string_coercion_nodes(node).each { |n| issue_for n.name_location, n.end_location, MSG }
+ string_coercion_nodes(node).each do |n|
+ issue_for n.name_location, n.end_location, MSG
+ end
end
private def string_coercion_nodes(node)
diff --git a/src/ameba/rule/lint/redundant_with_index.cr b/src/ameba/rule/lint/redundant_with_index.cr
index f67766d6d..6f6229861 100644
--- a/src/ameba/rule/lint/redundant_with_index.cr
+++ b/src/ameba/rule/lint/redundant_with_index.cr
@@ -26,8 +26,7 @@ module Ameba::Rule::Lint
# Lint/RedundantWithIndex:
# Enabled: true
# ```
- #
- struct RedundantWithIndex < Base
+ class RedundantWithIndex < Base
properties do
description "Disallows redundant `with_index` calls"
end
@@ -35,15 +34,14 @@ module Ameba::Rule::Lint
def test(source, node : Crystal::Call)
args, block = node.args, node.block
- return if args.size > 1 || block.nil? || with_index_arg?(block.not_nil!)
+ return if block.nil? || args.size > 1
+ return if with_index_arg?(block)
case node.name
when "with_index"
report source, node, "Remove redundant with_index"
when "each_with_index"
report source, node, "Use each instead of each_with_index"
- else
- # nop
end
end
diff --git a/src/ameba/rule/lint/redundant_with_object.cr b/src/ameba/rule/lint/redundant_with_object.cr
index 357e45d24..9acf0bf8d 100644
--- a/src/ameba/rule/lint/redundant_with_object.cr
+++ b/src/ameba/rule/lint/redundant_with_object.cr
@@ -27,21 +27,20 @@ module Ameba::Rule::Lint
# Lint/RedundantWithObject:
# Enabled: true
# ```
- #
- struct RedundantWithObject < Base
+ class RedundantWithObject < Base
properties do
description "Disallows redundant `with_object` calls"
end
+ MSG = "Use `each` instead of `each_with_object`"
+
def test(source, node : Crystal::Call)
return if node.name != "each_with_object" ||
node.args.size != 1 ||
node.block.nil? ||
with_index_arg?(node.block.not_nil!)
- issue_for node.name_location,
- node.name_end_location,
- "Use each instead of each_with_object"
+ issue_for node.name_location, node.name_end_location, MSG
end
private def with_index_arg?(block : Crystal::Block)
diff --git a/src/ameba/rule/lint/shadowed_argument.cr b/src/ameba/rule/lint/shadowed_argument.cr
index 0c27c25df..c21c1daed 100644
--- a/src/ameba/rule/lint/shadowed_argument.cr
+++ b/src/ameba/rule/lint/shadowed_argument.cr
@@ -35,8 +35,7 @@ module Ameba::Rule::Lint
# Lint/ShadowedArgument:
# Enabled: true
# ```
- #
- struct ShadowedArgument < Base
+ class ShadowedArgument < Base
properties do
description "Disallows shadowed arguments"
end
diff --git a/src/ameba/rule/lint/shadowed_exception.cr b/src/ameba/rule/lint/shadowed_exception.cr
index 535703407..6e10de9d3 100644
--- a/src/ameba/rule/lint/shadowed_exception.cr
+++ b/src/ameba/rule/lint/shadowed_exception.cr
@@ -33,8 +33,7 @@ module Ameba::Rule::Lint
# Lint/ShadowedException:
# Enabled: true
# ```
- #
- struct ShadowedException < Base
+ class ShadowedException < Base
properties do
description "Disallows rescued exception that get shadowed"
end
@@ -42,18 +41,17 @@ module Ameba::Rule::Lint
MSG = "Exception handler has shadowed exceptions: %s"
def test(source, node : Crystal::ExceptionHandler)
- return unless excs = node.rescues
+ return unless excs = node.rescues.try &.map(&.types)
+ return if (excs = shadowed excs).empty?
- if (excs = shadowed excs.map(&.types)).any?
- issue_for node, MSG % excs.join(", ")
- end
+ issue_for node, MSG % excs.join(", ")
end
private def shadowed(exceptions, exception_found = false)
previous_exceptions = [] of String
exceptions.reduce([] of String) do |shadowed, excs|
- excs = excs ? excs.map(&.to_s) : ["Exception"]
+ excs = excs.try(&.map(&.to_s)) || %w[Exception]
if exception_found
shadowed.concat excs
diff --git a/src/ameba/rule/lint/shadowing_local_outer_var.cr b/src/ameba/rule/lint/shadowing_local_outer_var.cr
index 846cc4840..8f70a2858 100644
--- a/src/ameba/rule/lint/shadowing_local_outer_var.cr
+++ b/src/ameba/rule/lint/shadowing_local_outer_var.cr
@@ -30,8 +30,7 @@ module Ameba::Rule::Lint
# Lint/ShadowingOuterLocalVar:
# Enabled: true
# ```
- #
- struct ShadowingOuterLocalVar < Base
+ class ShadowingOuterLocalVar < Base
properties do
description "Disallows the usage of the same name as outer local variables" \
" for block or proc arguments."
diff --git a/src/ameba/rule/lint/shared_var_in_fiber.cr b/src/ameba/rule/lint/shared_var_in_fiber.cr
index 8011fa036..2e57c7125 100644
--- a/src/ameba/rule/lint/shared_var_in_fiber.cr
+++ b/src/ameba/rule/lint/shared_var_in_fiber.cr
@@ -49,8 +49,7 @@ module Ameba::Rule::Lint
# Lint/SharedVarInFiber:
# Enabled: true
# ```
- #
- struct SharedVarInFiber < Base
+ class SharedVarInFiber < Base
properties do
description "Disallows shared variables in fibers."
end
@@ -77,7 +76,7 @@ module Ameba::Rule::Lint
declared_in = variable.assignments.first?.try &.branch
variable.assignments
- .reject { |assign| assign.scope.spawn_block? }
+ .reject(&.scope.spawn_block?)
.any? do |assign|
assign.branch.try(&.in_loop?) && assign.branch != declared_in
end
diff --git a/src/ameba/rule/lint/spec_focus.cr b/src/ameba/rule/lint/spec_focus.cr
new file mode 100644
index 000000000..d1810e318
--- /dev/null
+++ b/src/ameba/rule/lint/spec_focus.cr
@@ -0,0 +1,70 @@
+module Ameba::Rule::Lint
+ # Checks if specs are focused.
+ #
+ # In specs `focus: true` is mainly used to focus on a spec
+ # item locally during development. However, if such change
+ # is committed, it silently runs only focused spec on all
+ # other enviroment, which is undesired.
+ #
+ # This is considered bad:
+ #
+ # ```
+ # describe MyClass, focus: true do
+ # end
+ #
+ # describe ".new", focus: true do
+ # end
+ #
+ # context "my context", focus: true do
+ # end
+ #
+ # it "works", focus: true do
+ # end
+ # ```
+ #
+ # And it should be written as the following:
+ #
+ # ```
+ # describe MyClass do
+ # end
+ #
+ # describe ".new" do
+ # end
+ #
+ # context "my context" do
+ # end
+ #
+ # it "works" do
+ # end
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Lint/SpecFocus:
+ # Enabled: true
+ # ```
+ class SpecFocus < Base
+ properties do
+ description "Reports focused spec items"
+ end
+
+ MSG = "Focused spec item detected"
+ SPEC_ITEM_NAMES = %w(describe context it pending)
+
+ def test(source)
+ return unless source.spec?
+
+ AST::NodeVisitor.new self, source
+ end
+
+ def test(source, node : Crystal::Call)
+ return unless node.name.in?(SPEC_ITEM_NAMES)
+ return unless node.block
+
+ arg = node.named_args.try &.find(&.name.== "focus")
+
+ issue_for arg, MSG if arg
+ end
+ end
+end
diff --git a/src/ameba/rule/lint/syntax.cr b/src/ameba/rule/lint/syntax.cr
index 8d135401e..6fbaddda2 100644
--- a/src/ameba/rule/lint/syntax.cr
+++ b/src/ameba/rule/lint/syntax.cr
@@ -18,8 +18,7 @@ module Ameba::Rule::Lint
# rescue e : Exception
# end
# ```
- #
- struct Syntax < Base
+ class Syntax < Base
properties do
description "Reports invalid Crystal syntax"
severity Ameba::Severity::Error
diff --git a/src/ameba/rule/lint/unneeded_disable_directive.cr b/src/ameba/rule/lint/unneeded_disable_directive.cr
index 6128fbeba..4a55f4463 100644
--- a/src/ameba/rule/lint/unneeded_disable_directive.cr
+++ b/src/ameba/rule/lint/unneeded_disable_directive.cr
@@ -24,8 +24,7 @@ module Ameba::Rule::Lint
# Lint/UnneededDisableDirective
# Enabled: true
# ```
- #
- struct UnneededDisableDirective < Base
+ class UnneededDisableDirective < Base
properties do
description "Reports unneeded disable directives in comments"
end
@@ -37,7 +36,7 @@ module Ameba::Rule::Lint
next unless token.type == :COMMENT
next unless directive = source.parse_inline_directive(token.value.to_s)
next unless names = unneeded_disables(source, directive, token.location)
- next unless names.any?
+ next if names.empty?
issue_for token, MSG % names.join(", ")
end
@@ -47,11 +46,12 @@ module Ameba::Rule::Lint
return unless directive[:action] == "disable"
directive[:rules].reject do |rule_name|
+ next if rule_name == self.name
source.issues.any? do |issue|
issue.rule.name == rule_name &&
issue.disabled? &&
issue_at_location?(source, issue, location)
- end && rule_name != self.name
+ end
end
end
diff --git a/src/ameba/rule/lint/unreachable_code.cr b/src/ameba/rule/lint/unreachable_code.cr
index ded09bf57..dc7e34647 100644
--- a/src/ameba/rule/lint/unreachable_code.cr
+++ b/src/ameba/rule/lint/unreachable_code.cr
@@ -41,8 +41,7 @@ module Ameba::Rule::Lint
# Lint/UnreachableCode:
# Enabled: true
# ```
- #
- struct UnreachableCode < Base
+ class UnreachableCode < Base
include AST::Util
properties do
diff --git a/src/ameba/rule/lint/unused_argument.cr b/src/ameba/rule/lint/unused_argument.cr
index 6abadca80..16d860492 100644
--- a/src/ameba/rule/lint/unused_argument.cr
+++ b/src/ameba/rule/lint/unused_argument.cr
@@ -24,8 +24,7 @@ module Ameba::Rule::Lint
# IgnoreBlocks: false
# IgnoreProcs: false
# ```
- #
- struct UnusedArgument < Base
+ class UnusedArgument < Base
properties do
description "Disallows unused arguments"
diff --git a/src/ameba/rule/lint/useless_assign.cr b/src/ameba/rule/lint/useless_assign.cr
index 6616b6abf..e09df45ed 100644
--- a/src/ameba/rule/lint/useless_assign.cr
+++ b/src/ameba/rule/lint/useless_assign.cr
@@ -25,8 +25,7 @@ module Ameba::Rule::Lint
# Lint/UselessAssign:
# Enabled: true
# ```
- #
- struct UselessAssign < Base
+ class UselessAssign < Base
properties do
description "Disallows useless variable assignments"
end
diff --git a/src/ameba/rule/lint/useless_condition_in_when.cr b/src/ameba/rule/lint/useless_condition_in_when.cr
index 4b2fb36cf..7cf7784dd 100644
--- a/src/ameba/rule/lint/useless_condition_in_when.cr
+++ b/src/ameba/rule/lint/useless_condition_in_when.cr
@@ -30,8 +30,7 @@ module Ameba::Rule::Lint
# Lint/UselessConditionInWhen:
# Enabled: true
# ```
- #
- struct UselessConditionInWhen < Base
+ class UselessConditionInWhen < Base
properties do
description "Disallows useless conditions in when"
end
@@ -43,10 +42,7 @@ module Ameba::Rule::Lint
# simple implementation in future.
protected def check_node(source, when_node, cond)
cond_s = cond.to_s
- return if when_node
- .conds
- .map(&.to_s)
- .none? { |c| c == cond_s }
+ return if when_node.conds.none?(&.to_s.==(cond_s))
issue_for cond, MSG
end
diff --git a/src/ameba/rule/metrics/cyclomatic_complexity.cr b/src/ameba/rule/metrics/cyclomatic_complexity.cr
index fb2e2fe22..1047df670 100644
--- a/src/ameba/rule/metrics/cyclomatic_complexity.cr
+++ b/src/ameba/rule/metrics/cyclomatic_complexity.cr
@@ -8,8 +8,7 @@ module Ameba::Rule::Metrics
# Enabled: true
# MaxComplexity: 10
# ```
- #
- struct CyclomaticComplexity < Base
+ class CyclomaticComplexity < Base
properties do
description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`"
max_complexity 10
@@ -21,17 +20,17 @@ module Ameba::Rule::Metrics
complexity = AST::CountingVisitor.new(node).count
if complexity > max_complexity && (location = node.name_location)
- issue_for(
- location,
- def_name_end_location(node),
+ issue_for location, def_name_end_location(node),
MSG % {complexity, max_complexity}
- )
end
end
private def def_name_end_location(node)
return unless location = node.name_location
- line_number, column_number = location.line_number, location.column_number
+
+ line_number, column_number =
+ location.line_number, location.column_number
+
Crystal::Location.new(location.filename, line_number, column_number + node.name.size)
end
end
diff --git a/src/ameba/rule/performance/any_after_filter.cr b/src/ameba/rule/performance/any_after_filter.cr
index 5e4ae0772..bdc9c1e7f 100644
--- a/src/ameba/rule/performance/any_after_filter.cr
+++ b/src/ameba/rule/performance/any_after_filter.cr
@@ -24,23 +24,21 @@ module Ameba::Rule::Performance
# - select
# - reject
# ```
- #
- struct AnyAfterFilter < Base
- ANY_NAME = "any?"
- MSG = "Use `#{ANY_NAME} {...}` instead of `%s {...}.#{ANY_NAME}`"
-
+ class AnyAfterFilter < Base
properties do
- filter_names : Array(String) = %w(select reject)
description "Identifies usage of `any?` calls that follow filters."
+ filter_names : Array(String) = %w(select reject)
end
+ ANY_NAME = "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 obj.is_a?(Crystal::Call) && obj.block && node.block.nil?
+ return unless obj.name.in?(filter_names)
- if node.block.nil? && obj.is_a?(Crystal::Call) &&
- filter_names.includes?(obj.name) && !obj.block.nil?
- issue_for obj.name_location, node.name_end_location, MSG % obj.name
- end
+ issue_for obj.name_location, node.name_end_location, MSG % obj.name
end
end
end
diff --git a/src/ameba/rule/performance/any_instead_of_empty.cr b/src/ameba/rule/performance/any_instead_of_empty.cr
new file mode 100644
index 000000000..6dd7a2bad
--- /dev/null
+++ b/src/ameba/rule/performance/any_instead_of_empty.cr
@@ -0,0 +1,44 @@
+module Ameba::Rule::Performance
+ # This rule is used to identify usage of arg-less `Enumerable#any?` calls.
+ #
+ # Using `Enumerable#any?` instead of `Enumerable#empty?` might lead to an
+ # unexpected results (like `[nil, false].any? # => false`). In some cases
+ # it also might be less efficient, since it iterates until the block will
+ # return a _truthy_ value, instead of just checking if there's at least
+ # one value present.
+ #
+ # For example, this is considered invalid:
+ #
+ # ```
+ # [1, 2, 3].any?
+ # ```
+ #
+ # And it should be written as this:
+ #
+ # ```
+ # ![1, 2, 3].empty?
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Performance/AnyInsteadOfEmpty:
+ # Enabled: true
+ # ```
+ class AnyInsteadOfEmpty < Base
+ properties do
+ description "Identifies usage of arg-less `any?` calls."
+ end
+
+ ANY_NAME = "any?"
+ MSG = "Use `!{...}.empty?` instead of `{...}.any?`"
+
+ def test(source, node : Crystal::Call)
+ return unless node.name == ANY_NAME
+ return unless node.block.nil? && node.args.empty?
+ return unless node.obj
+
+ issue_for node.name_location, node.name_end_location, MSG
+ end
+ end
+end
diff --git a/src/ameba/rule/performance/chained_call_with_no_bang.cr b/src/ameba/rule/performance/chained_call_with_no_bang.cr
new file mode 100644
index 000000000..22a57c780
--- /dev/null
+++ b/src/ameba/rule/performance/chained_call_with_no_bang.cr
@@ -0,0 +1,75 @@
+module Ameba::Rule::Performance
+ # This rule is used to identify usage of chained calls not utilizing
+ # the bang method variants.
+ #
+ # For example, this is considered inefficient:
+ #
+ # ```
+ # names = %w[Alice Bob]
+ # chars = names
+ # .flat_map(&.chars)
+ # .uniq
+ # .sort
+ # ```
+ #
+ # And can be written as this:
+ #
+ # ```
+ # names = %w[Alice Bob]
+ # chars = names
+ # .flat_map(&.chars)
+ # .uniq!
+ # .sort!
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Performance/ChainedCallWithNoBang:
+ # Enabled: true
+ # CallNames:
+ # - uniq
+ # - sort
+ # - sort_by
+ # - shuffle
+ # - reverse
+ # ```
+ class ChainedCallWithNoBang < Base
+ properties do
+ description "Identifies usage of chained calls not utilizing the bang method variants."
+
+ # All of those have bang method variants returning `self`
+ # and are not modifying the receiver type (like `compact` does),
+ # thus are safe to switch to the bang variant.
+ call_names : Array(String) = %w(uniq sort sort_by shuffle reverse)
+ end
+
+ # All these methods are allocating a new object
+ ALLOCATING_METHOD_NAMES = %w(
+ keys values values_at map map_with_index flat_map compact_map
+ flatten compact select reject sample group_by chunks tally merge
+ combinations repeated_combinations permutations repeated_permutations
+ transpose invert chars captures named_captures clone
+ )
+
+ MSG = "Use bang method variant `%s!` after chained `%s` call"
+
+ def test(source)
+ AST::NodeVisitor.new self, source, skip: [
+ Crystal::Macro,
+ Crystal::MacroExpression,
+ Crystal::MacroIf,
+ Crystal::MacroFor,
+ ]
+ end
+
+ def test(source, node : Crystal::Call)
+ return unless (obj = node.obj).is_a?(Crystal::Call)
+ return unless node.name.in?(call_names)
+ return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES)
+
+ issue_for node.name_location, node.name_end_location,
+ MSG % {node.name, obj.name}
+ end
+ end
+end
diff --git a/src/ameba/rule/performance/compact_after_map.cr b/src/ameba/rule/performance/compact_after_map.cr
new file mode 100644
index 000000000..63184498d
--- /dev/null
+++ b/src/ameba/rule/performance/compact_after_map.cr
@@ -0,0 +1,48 @@
+module Ameba::Rule::Performance
+ # This rule is used to identify usage of `compact` calls that follow `map`.
+ #
+ # For example, this is considered inefficient:
+ #
+ # ```
+ # %w[Alice Bob].map(&.match(/^A./)).compact
+ # ```
+ #
+ # And can be written as this:
+ #
+ # ```
+ # %w[Alice Bob].compact_map(&.match(/^A./))
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Performance/CompactAfterMap:
+ # Enabled: true
+ # ```
+ class CompactAfterMap < Base
+ properties do
+ description "Identifies usage of `compact` calls that follow `map`."
+ end
+
+ COMPACT_NAME = "compact"
+ MAP_NAME = "map"
+ MSG = "Use `compact_map {...}` instead of `map {...}.compact`"
+
+ def test(source)
+ AST::NodeVisitor.new self, source, skip: [
+ Crystal::Macro,
+ Crystal::MacroExpression,
+ Crystal::MacroIf,
+ Crystal::MacroFor,
+ ]
+ end
+
+ def test(source, node : Crystal::Call)
+ return unless node.name == COMPACT_NAME && (obj = node.obj)
+ return unless obj.is_a?(Crystal::Call) && obj.block
+ return unless obj.name == MAP_NAME
+
+ issue_for obj.name_location, node.name_end_location, MSG
+ end
+ end
+end
diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr
index a35994230..f33680276 100644
--- a/src/ameba/rule/performance/first_last_after_filter.cr
+++ b/src/ameba/rule/performance/first_last_after_filter.cr
@@ -23,17 +23,16 @@ module Ameba::Rule::Performance
# FilterNames:
# - select
# ```
- #
- struct FirstLastAfterFilter < Base
- CALL_NAMES = %w(first last first? last?)
- MSG = "Use `find {...}` instead of `%s {...}.%s`"
- MSG_REVERSE = "Use `reverse_each.find {...}` instead of `%s {...}.%s`"
-
+ class FirstLastAfterFilter < Base
properties do
- filter_names : Array(String) = %w(select)
description "Identifies usage of `first/last/first?/last?` calls that follow filters."
+ filter_names : Array(String) = %w(select)
end
+ CALL_NAMES = %w(first last first? last?)
+ MSG = "Use `find {...}` instead of `%s {...}.%s`"
+ MSG_REVERSE = "Use `reverse_each.find {...}` instead of `%s {...}.%s`"
+
def test(source)
AST::NodeVisitor.new self, source, skip: [
Crystal::Macro,
@@ -44,14 +43,13 @@ module Ameba::Rule::Performance
end
def test(source, node : Crystal::Call)
- return unless CALL_NAMES.includes?(node.name) && (obj = node.obj)
- return if node.args.any?
+ return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
+ return unless obj.is_a?(Crystal::Call) && obj.block
+ return unless node.block.nil? && node.args.empty?
+ return unless obj.name.in?(filter_names)
- if node.block.nil? && obj.is_a?(Crystal::Call) &&
- filter_names.includes?(obj.name) && !obj.block.nil?
- message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE
- issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name}
- end
+ message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE
+ issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name}
end
end
end
diff --git a/src/ameba/rule/performance/flatten_after_map.cr b/src/ameba/rule/performance/flatten_after_map.cr
new file mode 100644
index 000000000..18ee2f8ab
--- /dev/null
+++ b/src/ameba/rule/performance/flatten_after_map.cr
@@ -0,0 +1,48 @@
+module Ameba::Rule::Performance
+ # This rule is used to identify usage of `flatten` calls that follow `map`.
+ #
+ # For example, this is considered inefficient:
+ #
+ # ```
+ # %w[Alice Bob].map(&.chars).flatten
+ # ```
+ #
+ # And can be written as this:
+ #
+ # ```
+ # %w[Alice Bob].flat_map(&.chars)
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Performance/FlattenAfterMap:
+ # Enabled: true
+ # ```
+ class FlattenAfterMap < Base
+ properties do
+ description "Identifies usage of `flatten` calls that follow `map`."
+ end
+
+ FLATTEN_NAME = "flatten"
+ MAP_NAME = "map"
+ MSG = "Use `flat_map {...}` instead of `map {...}.flatten`"
+
+ def test(source)
+ AST::NodeVisitor.new self, source, skip: [
+ Crystal::Macro,
+ Crystal::MacroExpression,
+ Crystal::MacroIf,
+ Crystal::MacroFor,
+ ]
+ end
+
+ def test(source, node : Crystal::Call)
+ return unless node.name == FLATTEN_NAME && (obj = node.obj)
+ return unless obj.is_a?(Crystal::Call) && obj.block
+ return unless obj.name == MAP_NAME
+
+ issue_for obj.name_location, node.name_end_location, MSG
+ end
+ end
+end
diff --git a/src/ameba/rule/performance/map_instead_of_block.cr b/src/ameba/rule/performance/map_instead_of_block.cr
new file mode 100644
index 000000000..377130fc6
--- /dev/null
+++ b/src/ameba/rule/performance/map_instead_of_block.cr
@@ -0,0 +1,52 @@
+module Ameba::Rule::Performance
+ # This rule is used to identify usage of `join/sum/product` calls
+ # that follow `map`.
+ #
+ # For example, this is considered inefficient:
+ #
+ # ```
+ # (1..3).map(&.to_s).join('.')
+ # (1..3).map(&.*(2)).sum
+ # ```
+ #
+ # And can be written as this:
+ #
+ # ```
+ # (1..3).join('.', &.to_s)
+ # (1..3).sum(&.*(2))
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Performance/MapInsteadOfBlock:
+ # Enabled: true
+ # ```
+ class MapInsteadOfBlock < Base
+ properties do
+ description "Identifies usage of `join/sum/product` calls that follow `map`."
+ end
+
+ CALL_NAMES = %w(join sum product)
+ MAP_NAME = "map"
+ MSG = "Use `%s {...}` instead of `map {...}.%s`"
+
+ def test(source)
+ AST::NodeVisitor.new self, source, skip: [
+ Crystal::Macro,
+ Crystal::MacroExpression,
+ Crystal::MacroIf,
+ Crystal::MacroFor,
+ ]
+ end
+
+ def test(source, node : Crystal::Call)
+ return unless node.name.in?(CALL_NAMES) && (obj = node.obj)
+ return unless obj.is_a?(Crystal::Call) && obj.block
+ return unless obj.name == MAP_NAME
+
+ issue_for obj.name_location, node.name_end_location,
+ MSG % {node.name, node.name}
+ end
+ end
+end
diff --git a/src/ameba/rule/performance/size_after_filter.cr b/src/ameba/rule/performance/size_after_filter.cr
index 1fe26359b..f7a2fbbb3 100644
--- a/src/ameba/rule/performance/size_after_filter.cr
+++ b/src/ameba/rule/performance/size_after_filter.cr
@@ -30,16 +30,15 @@ module Ameba::Rule::Performance
# - select
# - reject
# ```
- #
- struct SizeAfterFilter < Base
- SIZE_NAME = "size"
- MSG = "Use `count {...}` instead of `%s {...}.#{SIZE_NAME}`."
-
+ class SizeAfterFilter < Base
properties do
- filter_names : Array(String) = %w(select reject)
description "Identifies usage of `size` calls that follow filter"
+ filter_names : Array(String) = %w(select reject)
end
+ SIZE_NAME = "size"
+ MSG = "Use `count {...}` instead of `%s {...}.size`."
+
def test(source)
AST::NodeVisitor.new self, source, skip: [
Crystal::Macro,
@@ -51,11 +50,10 @@ module Ameba::Rule::Performance
def test(source, node : Crystal::Call)
return unless node.name == SIZE_NAME && (obj = node.obj)
+ return unless obj.is_a?(Crystal::Call) && obj.block
+ return unless obj.name.in?(filter_names)
- if obj.is_a?(Crystal::Call) &&
- filter_names.includes?(obj.name) && !obj.block.nil?
- issue_for obj.name_location, node.name_end_location, MSG % obj.name
- end
+ issue_for obj.name_location, node.name_end_location, MSG % obj.name
end
end
end
diff --git a/src/ameba/rule/style/constant_names.cr b/src/ameba/rule/style/constant_names.cr
index 3628006e7..e5182efb7 100644
--- a/src/ameba/rule/style/constant_names.cr
+++ b/src/ameba/rule/style/constant_names.cr
@@ -21,8 +21,7 @@ module Ameba::Rule::Style
# Style/ConstantNames:
# Enabled: true
# ```
- #
- struct ConstantNames < Base
+ class ConstantNames < Base
properties do
description "Enforces constant names to be in screaming case"
end
@@ -30,11 +29,11 @@ module Ameba::Rule::Style
MSG = "Constant name should be screaming-cased: %s, not %s"
def test(source, node : Crystal::Assign)
- if (target = node.target).is_a? Crystal::Path
+ if (target = node.target).is_a?(Crystal::Path)
name = target.names.first
expected = name.upcase
- return if expected == name || name.camelcase == name
+ return if name.in?(expected, name.camelcase)
issue_for target, MSG % {expected, name}
end
diff --git a/src/ameba/rule/style/is_a_filter.cr b/src/ameba/rule/style/is_a_filter.cr
new file mode 100644
index 000000000..fedeedeca
--- /dev/null
+++ b/src/ameba/rule/style/is_a_filter.cr
@@ -0,0 +1,74 @@
+module Ameba::Rule::Style
+ # This rule is used to identify usage of `is_a?/nil?` calls within filters.
+ #
+ # For example, this is considered invalid:
+ #
+ # ```
+ # matches = %w[Alice Bob].map(&.match(/^A./))
+ #
+ # matches.any?(&.is_a?(Regex::MatchData)) # => true
+ # matches.one?(&.nil?) # => true
+ #
+ # typeof(matches.reject(&.nil?)) # => Array(Regex::MatchData | Nil)
+ # typeof(matches.select(&.is_a?(Regex::MatchData))) # => Array(Regex::MatchData | Nil)
+ # ```
+ #
+ # And it should be written as this:
+ #
+ # ```
+ # matches = %w[Alice Bob].map(&.match(/^A./))
+ #
+ # matches.any?(Regex::MatchData) # => true
+ # matches.one?(Nil) # => true
+ #
+ # typeof(matches.reject(Nil)) # => Array(Regex::MatchData)
+ # typeof(matches.select(Regex::MatchData)) # => Array(Regex::MatchData)
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Style/IsAFilter:
+ # Enabled: true
+ # FilterNames:
+ # - select
+ # - reject
+ # - any?
+ # - all?
+ # - none?
+ # - one?
+ # ```
+ class IsAFilter < Base
+ properties do
+ description "Identifies usage of `is_a?/nil?` calls within filters."
+ filter_names : Array(String) = %w(select reject any? all? none? one?)
+ end
+
+ MSG = "Use `%s(%s)` instead of `%s {...}`"
+
+ def test(source, node : Crystal::Call)
+ return unless node.name.in?(filter_names)
+ return unless (block = node.block)
+ return unless (body = block.body).is_a?(Crystal::IsA)
+ return unless (path = body.const).is_a?(Crystal::Path)
+ return unless body.obj.is_a?(Crystal::Var)
+
+ name = path.names.join("::")
+ name = "::#{name}" if path.global? && !body.nil_check?
+
+ end_location = node.end_location
+ if !end_location || end_location.try(&.column_number.zero?)
+ if end_location = path.end_location
+ end_location = Crystal::Location.new(
+ end_location.filename,
+ end_location.line_number,
+ end_location.column_number + 1
+ )
+ end
+ end
+
+ issue_for node.name_location, end_location,
+ MSG % {node.name, name, node.name}
+ end
+ end
+end
diff --git a/src/ameba/rule/style/is_a_nil.cr b/src/ameba/rule/style/is_a_nil.cr
index 18653bbc2..72568b3a4 100644
--- a/src/ameba/rule/style/is_a_nil.cr
+++ b/src/ameba/rule/style/is_a_nil.cr
@@ -4,7 +4,7 @@ module Ameba::Rule::Style
# This is considered bad:
#
# ```
- # var.is_a? Nil
+ # var.is_a?(Nil)
# ```
#
# And needs to be written as:
@@ -19,8 +19,7 @@ module Ameba::Rule::Style
# Style/IsANil:
# Enabled: true
# ```
- #
- struct IsANil < Base
+ class IsANil < Base
properties do
description "Disallows calls to `is_a?(Nil)` in favor of `nil?`"
end
@@ -32,7 +31,10 @@ module Ameba::Rule::Style
return if node.nil_check?
const = node.const
- issue_for const, MSG if const.is_a?(Crystal::Path) && const.names == PATH_NIL_NAMES
+ return unless const.is_a?(Crystal::Path)
+ return unless const.names == PATH_NIL_NAMES
+
+ issue_for const, MSG
end
end
end
diff --git a/src/ameba/rule/style/large_numbers.cr b/src/ameba/rule/style/large_numbers.cr
index a9a2b33db..e36c4c0fa 100644
--- a/src/ameba/rule/style/large_numbers.cr
+++ b/src/ameba/rule/style/large_numbers.cr
@@ -26,12 +26,11 @@ module Ameba::Rule::Style
# Enabled: true
# IntMinDigits: 5 # i.e. integers higher than 9999
# ```
- #
- struct LargeNumbers < Base
+ class LargeNumbers < Base
properties do
+ enabled false
description "Disallows usage of large numbers without underscore"
int_min_digits 5
- enabled false
end
MSG = "Large numbers should be written with underscores: %s"
@@ -53,7 +52,7 @@ module Ameba::Rule::Style
end
private def allowed?(_sign, value, fraction, _suffix)
- return true if !fraction.nil? && fraction.size > 3
+ return true if fraction && fraction.size > 3
digits = value.chars.select &.to_s.=~ /[0-9]/
digits.size >= int_min_digits
@@ -71,7 +70,7 @@ module Ameba::Rule::Style
value.chars.reject(&.== '_').each_slice(by) do |slice|
slices << (yield slice).join
end
- end.join("_")
+ end.join('_')
end
private def parse_number(value)
@@ -83,7 +82,7 @@ module Ameba::Rule::Style
end
private def parse_sign(value)
- if "+-".includes?(value[0])
+ if value[0].in?('+', '-')
sign = value[0]
value = value[1..-1]
end
@@ -91,7 +90,7 @@ module Ameba::Rule::Style
end
private def parse_suffix(value)
- if pos = (value =~ /e/ || value =~ /_?(i|u|f)/)
+ if pos = (value =~ /(e|_?(i|u|f))/)
suffix = value[pos..-1]
value = value[0..pos - 1]
end
diff --git a/src/ameba/rule/style/method_names.cr b/src/ameba/rule/style/method_names.cr
index 905de3735..112ffbd7f 100644
--- a/src/ameba/rule/style/method_names.cr
+++ b/src/ameba/rule/style/method_names.cr
@@ -37,8 +37,7 @@ module Ameba::Rule::Style
# Style/MethodNames:
# Enabled: true
# ```
- #
- struct MethodNames < Base
+ class MethodNames < Base
properties do
description "Enforces method names to be in underscored case"
end
@@ -51,7 +50,7 @@ module Ameba::Rule::Style
line_number = node.location.try &.line_number
column_number = node.name_location.try &.column_number
- return if line_number.nil? || column_number.nil?
+ return unless line_number && column_number
issue_for(
{line_number, column_number},
diff --git a/src/ameba/rule/style/negated_conditions_in_unless.cr b/src/ameba/rule/style/negated_conditions_in_unless.cr
index 8781b36a9..653a574be 100644
--- a/src/ameba/rule/style/negated_conditions_in_unless.cr
+++ b/src/ameba/rule/style/negated_conditions_in_unless.cr
@@ -26,8 +26,7 @@ module Ameba::Rule::Style
# Style/NegatedConditionsInUnless:
# Enabled: true
# ```
- #
- struct NegatedConditionsInUnless < Base
+ class NegatedConditionsInUnless < Base
properties do
description "Disallows negated conditions in unless"
end
@@ -35,8 +34,7 @@ module Ameba::Rule::Style
MSG = "Avoid negated conditions in unless blocks"
def test(source, node : Crystal::Unless)
- return unless negated_condition? node.cond
- issue_for node, MSG
+ issue_for node, MSG if negated_condition?(node.cond)
end
private def negated_condition?(node)
@@ -44,7 +42,7 @@ module Ameba::Rule::Style
when Crystal::BinaryOp
negated_condition?(node.left) || negated_condition?(node.right)
when Crystal::Expressions
- node.expressions.any? { |e| negated_condition? e }
+ node.expressions.any? { |e| negated_condition?(e) }
when Crystal::Not
true
else
diff --git a/src/ameba/rule/style/predicate_name.cr b/src/ameba/rule/style/predicate_name.cr
index b23d938c3..24afebc1b 100644
--- a/src/ameba/rule/style/predicate_name.cr
+++ b/src/ameba/rule/style/predicate_name.cr
@@ -28,11 +28,10 @@ module Ameba::Rule::Style
# Style/PredicateName:
# Enabled: true
# ```
- #
- struct PredicateName < Base
+ class PredicateName < Base
properties do
- description "Disallows tautological predicate names"
enabled false
+ description "Disallows tautological predicate names"
end
MSG = "Favour method name '%s?' over '%s'"
diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr
index bfb028a63..3f3009f4b 100644
--- a/src/ameba/rule/style/redundant_begin.cr
+++ b/src/ameba/rule/style/redundant_begin.cr
@@ -55,9 +55,9 @@ module Ameba::Rule::Style
# Style/RedundantBegin:
# Enabled: true
# ```
- #
- struct RedundantBegin < Base
+ class RedundantBegin < Base
include AST::Util
+
properties do
description "Disallows redundant begin blocks"
end
@@ -65,9 +65,7 @@ module Ameba::Rule::Style
MSG = "Redundant `begin` block detected"
def test(source, node : Crystal::Def)
- return unless redundant_begin?(source, node)
-
- issue_for node, MSG
+ issue_for node, MSG if redundant_begin?(source, node)
end
private def redundant_begin?(source, node)
@@ -76,8 +74,6 @@ module Ameba::Rule::Style
redundant_begin_in_handler?(source, body, node)
when Crystal::Expressions
redundant_begin_in_expressions?(body)
- else
- # nop
end
end
@@ -88,7 +84,7 @@ module Ameba::Rule::Style
private def redundant_begin_in_handler?(source, handler, node)
return false if begin_exprs_in_handler?(handler) || inner_handler?(handler)
- code = node_source(node, source.lines).try &.join("\n")
+ code = node_source(node, source.lines).try &.join('\n')
def_redundant_begin? code if code
rescue
false
@@ -107,6 +103,7 @@ module Ameba::Rule::Style
private def def_redundant_begin?(code)
lexer = Crystal::Lexer.new code
in_body = in_argument_list = false
+
loop do
token = lexer.next_token
@@ -122,6 +119,7 @@ module Ameba::Rule::Style
when :NEWLINE
in_body = true unless in_argument_list
when :SPACE
+ # ignore
else
return false if in_body
end
diff --git a/src/ameba/rule/style/redundant_next.cr b/src/ameba/rule/style/redundant_next.cr
index fdf62d435..5decb7404 100644
--- a/src/ameba/rule/style/redundant_next.cr
+++ b/src/ameba/rule/style/redundant_next.cr
@@ -96,9 +96,10 @@ module Ameba::Rule::Style
# AllowMultiNext: true
# AllowEmptyNext: true
# ```
- struct RedundantNext < Base
+ class RedundantNext < Base
properties do
description "Reports redundant next expressions"
+
allow_multi_next true
allow_empty_next true
end
diff --git a/src/ameba/rule/style/redundant_return.cr b/src/ameba/rule/style/redundant_return.cr
index be226bbbd..e8ec7dc95 100644
--- a/src/ameba/rule/style/redundant_return.cr
+++ b/src/ameba/rule/style/redundant_return.cr
@@ -93,9 +93,10 @@ module Ameba::Rule::Style
# AllowMutliReturn: true
# AllowEmptyReturn: true
# ```
- struct RedundantReturn < Base
+ class RedundantReturn < Base
properties do
description "Reports redundant return expressions"
+
allow_multi_return true
allow_empty_return true
end
diff --git a/src/ameba/rule/style/type_names.cr b/src/ameba/rule/style/type_names.cr
index 2daa28f84..c05699874 100644
--- a/src/ameba/rule/style/type_names.cr
+++ b/src/ameba/rule/style/type_names.cr
@@ -51,8 +51,7 @@ module Ameba::Rule::Style
# Style/TypeNames:
# Enabled: true
# ```
- #
- struct TypeNames < Base
+ class TypeNames < Base
properties do
description "Enforces type names in camelcase manner"
end
@@ -62,7 +61,7 @@ module Ameba::Rule::Style
private def check_node(source, node)
name = node.name.to_s
expected = name.camelcase
- return if expected == name
+ return if name == expected
issue_for node, MSG % {expected, name}
end
diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr
index 55909cdc7..cdb4b966c 100644
--- a/src/ameba/rule/style/unless_else.cr
+++ b/src/ameba/rule/style/unless_else.cr
@@ -42,8 +42,7 @@ module Ameba::Rule::Style
# Style/UnlessElse:
# Enabled: true
# ```
- #
- struct UnlessElse < Base
+ class UnlessElse < Base
properties do
description "Disallows the use of an `else` block with the `unless`"
end
@@ -51,8 +50,7 @@ module Ameba::Rule::Style
MSG = "Favour if over unless with else"
def test(source, node : Crystal::Unless)
- return if node.else.nop?
- issue_for node, MSG
+ issue_for node, MSG unless node.else.nop?
end
end
end
diff --git a/src/ameba/rule/style/variable_names.cr b/src/ameba/rule/style/variable_names.cr
index 108c54d0c..e8b999c15 100644
--- a/src/ameba/rule/style/variable_names.cr
+++ b/src/ameba/rule/style/variable_names.cr
@@ -22,8 +22,7 @@ module Ameba::Rule::Style
# Style/VariableNames:
# Enabled: true
# ```
- #
- struct VariableNames < Base
+ class VariableNames < Base
properties do
description "Enforces variable names to be in underscored case"
end
diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr
new file mode 100644
index 000000000..d50490915
--- /dev/null
+++ b/src/ameba/rule/style/verbose_block.cr
@@ -0,0 +1,224 @@
+module Ameba::Rule::Style
+ # This rule is used to identify usage of single expression blocks with
+ # argument as a receiver, that can be collapsed into a short form.
+ #
+ # For example, this is considered invalid:
+ #
+ # ```
+ # (1..3).any? { |i| i.odd? }
+ # ```
+ #
+ # And it should be written as this:
+ #
+ # ```
+ # (1..3).any?(&.odd?)
+ # ```
+ #
+ # YAML configuration example:
+ #
+ # ```
+ # Style/VerboseBlock:
+ # Enabled: true
+ # ExcludeMultipleLineBlocks: true
+ # ExcludeCallsWithBlocks: false
+ # ExcludePrefixOperators: true
+ # ExcludeOperators: false
+ # ExcludeSetters: false
+ # MaxLineLength: ~
+ # MaxLength: 50 # use ~ to disable
+ # ```
+ class VerboseBlock < Base
+ properties do
+ description "Identifies usage of collapsible single expression blocks."
+
+ exclude_calls_with_block true
+ exclude_multiple_line_blocks false
+ exclude_prefix_operators true
+ exclude_operators false
+ exclude_setters false
+
+ max_line_length : Int32? = nil # 100
+ max_length : Int32? = 50
+ end
+
+ MSG = "Use short block notation instead: `%s`"
+ CALL_PATTERN = "%s(%s&.%s)"
+
+ protected def same_location_lines?(a, b)
+ return unless a_location = a.name_location
+ return unless b_location = b.location
+
+ a_location.line_number == b_location.line_number
+ end
+
+ private PREFIX_OPERATORS = {"+", "-"}
+ private OPERATOR_CHARS =
+ {'[', ']', '!', '=', '>', '<', '~', '+', '-', '*', '/', '%', '^', '|', '&'}
+
+ protected def prefix_operator?(node)
+ node.name.in?(PREFIX_OPERATORS) && node.args.empty?
+ end
+
+ protected def operator?(name)
+ name.each_char do |char|
+ return false unless char.in?(OPERATOR_CHARS)
+ end
+ !name.empty?
+ end
+
+ protected def setter?(name)
+ !name.empty? && name[0].letter? && name.ends_with?('=')
+ end
+
+ protected def valid_length?(code)
+ if max_length = self.max_length
+ return code.size <= max_length
+ end
+ true
+ end
+
+ protected def valid_line_length?(node, code)
+ if max_line_length = self.max_line_length
+ if location = node.name_location
+ final_line_length = location.column_number + code.size
+ return final_line_length <= max_line_length
+ end
+ end
+ true
+ end
+
+ protected def reference_count(node, obj : Crystal::Var)
+ i = 0
+ case node
+ when Crystal::Call
+ i += reference_count(node.obj, obj)
+ i += reference_count(node.block, obj)
+
+ node.args.each do |arg|
+ i += reference_count(arg, obj)
+ end
+ node.named_args.try &.each do |arg|
+ i += reference_count(arg.value, obj)
+ end
+ when Crystal::Block
+ i += reference_count(node.body, obj)
+ when Crystal::Var
+ i += 1 if node == obj
+ end
+ i
+ end
+
+ protected def args_to_s(io : IO, node : Crystal::Call, skip_last_arg = false)
+ node.args.dup.tap do |args|
+ args.pop? if skip_last_arg
+ args.join io, ", "
+ node.named_args.try do |named_args|
+ io << ", " unless args.empty? || named_args.empty?
+ named_args.join io, ", " do |arg, inner_io|
+ inner_io << arg.name << ": " << arg.value
+ end
+ end
+ end
+ end
+
+ protected def node_to_s(node : Crystal::Call)
+ String.build do |str|
+ case name = node.name
+ when "[]"
+ str << '['
+ args_to_s(str, node)
+ str << ']'
+ when "[]?"
+ str << '['
+ args_to_s(str, node)
+ str << "]?"
+ when "[]="
+ str << '['
+ args_to_s(str, node, skip_last_arg: true)
+ str << "]=(" << node.args.last? << ')'
+ else
+ str << name
+ if !node.args.empty? || (node.named_args && !node.named_args.try(&.empty?))
+ str << '('
+ args_to_s(str, node)
+ str << ')'
+ end
+ str << " {...}" if node.block
+ end
+ end
+ end
+
+ protected def call_code(call, body)
+ args = String.build { |io| args_to_s(io, call) }.presence
+ args += ", " if args
+
+ call_chain = %w[].tap do |arr|
+ obj = body.obj
+ while obj.is_a?(Crystal::Call)
+ arr << node_to_s(obj)
+ obj = obj.obj
+ end
+ arr.reverse!
+ arr << node_to_s(body)
+ end
+
+ name =
+ call_chain.join('.')
+
+ CALL_PATTERN % {call.name, args, name}
+ end
+
+ # ameba:disable Metrics/CyclomaticComplexity
+ protected def issue_for_valid(source, call : Crystal::Call, body : Crystal::Call)
+ return if exclude_calls_with_block && body.block
+ return if exclude_multiple_line_blocks && !same_location_lines?(call, body)
+ return if exclude_prefix_operators && prefix_operator?(body)
+ return if exclude_operators && operator?(body.name)
+ return if exclude_setters && setter?(body.name)
+
+ call_code =
+ call_code(call, body)
+
+ return unless valid_line_length?(call, call_code)
+ return unless valid_length?(call_code)
+
+ issue_for call.name_location, call.end_location,
+ MSG % call_code
+ end
+
+ def test(source, node : Crystal::Call)
+ # we are interested only in calls with block taking a single argument
+ #
+ # ```
+ # (1..3).any? { |i| i.to_i64.odd? }
+ # ^--- ^ ^------------
+ # block arg body
+ # ```
+ return unless (block = node.block) && block.args.size == 1
+
+ arg = block.args.first
+
+ # we skip auto-generated blocks - `(1..3).any?(&.odd?)`
+ return if arg.name.starts_with?("__arg")
+
+ # 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
+ # ends up being a variable - `i`
+ obj = body.obj
+ while obj.is_a?(Crystal::Call)
+ obj = obj.obj
+ end
+
+ # only calls with a first argument used as a receiver are the valid game
+ return unless obj == arg
+
+ # we bail out if the block node include the block argument
+ return if reference_count(body, arg) > 1
+
+ # add issue if the given nodes pass all of the checks
+ issue_for_valid source, node, body
+ end
+ end
+end
diff --git a/src/ameba/rule/style/while_true.cr b/src/ameba/rule/style/while_true.cr
index f57289e0a..4026c2170 100644
--- a/src/ameba/rule/style/while_true.cr
+++ b/src/ameba/rule/style/while_true.cr
@@ -25,8 +25,7 @@ module Ameba::Rule::Style
# Style/WhileTrue:
# Enabled: true
# ```
- #
- struct WhileTrue < Base
+ class WhileTrue < Base
properties do
description "Disallows while statements with a true literal as condition"
end
@@ -34,8 +33,7 @@ module Ameba::Rule::Style
MSG = "While statement using true literal as condition"
def test(source, node : Crystal::While)
- return unless node.cond.true_literal?
- issue_for node, MSG
+ issue_for node, MSG if node.cond.true_literal?
end
end
end
diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr
index ece391877..56b06e644 100644
--- a/src/ameba/runner.cr
+++ b/src/ameba/runner.cr
@@ -38,7 +38,6 @@ module Ameba
#
# Ameba::Runner.new config
# ```
- #
def initialize(config : Config)
@sources = config.sources
@formatter = config.formatter
@@ -50,7 +49,6 @@ module Ameba
.find &.name.==(Rule::Lint::UnneededDisableDirective.rule_name)
end
- # :nodoc:
protected def initialize(@rules, @sources, @formatter, @severity)
end
@@ -65,7 +63,6 @@ module Ameba
# runner = Ameba::Runner.new config
# runner.run # => returns runner again
# ```
- #
def run
@formatter.started @sources
channels = @sources.map { Channel(Exception?).new }
@@ -80,9 +77,8 @@ module Ameba
end
end
- channels.each do |c|
- e = c.receive
- raise e unless e.nil?
+ channels.each do |chan|
+ chan.receive.try { |e| raise e }
end
self
@@ -114,7 +110,6 @@ module Ameba
# runner.run
# runner.explain({file: file, line: l, column: c})
# ```
- #
def explain(location, output = STDOUT)
Formatter::ExplainFormatter.new(output, location).finished @sources
end
@@ -127,12 +122,11 @@ module Ameba
# runner.run
# runner.success? # => true or false
# ```
- #
def success?
@sources.all? do |source|
source.issues
.reject(&.disabled?)
- .none? { |issue| issue.rule.severity <= @severity }
+ .none?(&.rule.severity.<=(@severity))
end
end
diff --git a/src/ameba/severity.cr b/src/ameba/severity.cr
index 917cf4e98..6444a79db 100644
--- a/src/ameba/severity.cr
+++ b/src/ameba/severity.cr
@@ -9,8 +9,12 @@ module Ameba
# ```
# Severity::Warning.symbol # => 'W'
# ```
- def symbol
- to_s[0]
+ def symbol : Char
+ case self
+ in Error then 'E'
+ in Warning then 'W'
+ in Convention then 'C'
+ end
end
# Creates Severity by the name.
@@ -19,7 +23,6 @@ module Ameba
# Severity.parse("convention") # => Severity::Convention
# Severity.parse("foo-bar") # => Exception: Incorrect severity name
# ```
- #
def self.parse(name : String)
super name
rescue ArgumentError
diff --git a/src/ameba/source.cr b/src/ameba/source.cr
index 3fd8d751d..68dd2a1c7 100644
--- a/src/ameba/source.cr
+++ b/src/ameba/source.cr
@@ -11,10 +11,6 @@ module Ameba
# Crystal code (content of a source file).
getter code : String
- @lines : Array(String)?
- @ast : Crystal::ASTNode?
- @fullpath : String?
-
# Creates a new source by `code` and `path`.
#
# For example:
@@ -23,8 +19,7 @@ module Ameba
# path = "./src/source.cr"
# Ameba::Source.new File.read(path), path
# ```
- #
- def initialize(@code : String, @path = "")
+ def initialize(@code, @path = "")
end
# Returns lines of code splitted by new line character.
@@ -38,9 +33,7 @@ module Ameba
# source.lines # => ["a = 1", "b = 2"]
# ```
#
- def lines
- @lines ||= @code.split("\n")
- end
+ getter lines : Array(String) { code.split('\n') }
# Returns AST nodes constructed by `Crystal::Parser`.
#
@@ -49,19 +42,23 @@ module Ameba
# source.ast
# ```
#
- def ast
- @ast ||=
- Crystal::Parser.new(code)
- .tap { |parser| parser.wants_doc = true }
- .tap { |parser| parser.filename = @path }
- .parse
+ getter ast : Crystal::ASTNode do
+ Crystal::Parser.new(code)
+ .tap(&.wants_doc = true)
+ .tap(&.filename = path)
+ .parse
+ end
+
+ getter fullpath : String do
+ File.expand_path(path)
end
- def fullpath
- @fullpath ||= File.expand_path @path
+ # Returns `true` if the source is a spec file, `false` otherwise.
+ def spec?
+ path.ends_with?("_spec.cr")
end
- # Returns true if *filepath* matches the source's path, false if it does not.
+ # Returns `true` if *filepath* matches the source's path, `false` if it does not.
def matches_path?(filepath)
path == filepath || path == File.expand_path(filepath)
end
diff --git a/src/ameba/tokenizer.cr b/src/ameba/tokenizer.cr
index 070e5b552..5703a95ab 100644
--- a/src/ameba/tokenizer.cr
+++ b/src/ameba/tokenizer.cr
@@ -18,7 +18,6 @@ module Ameba
# source = Ameba::Source.new code, path
# Ameba::Tokenizer.new(source)
# ```
- #
def initialize(source)
@lexer = Crystal::Lexer.new source.code
@lexer.count_whitespace = true
@@ -33,7 +32,6 @@ module Ameba
# lexer = Crystal::Lexer.new(code)
# Ameba::Tokenizer.new(lexer)
# ```
- #
def initialize(@lexer : Crystal::Lexer)
end
@@ -44,7 +42,6 @@ module Ameba
# puts token
# end
# ```
- #
def run(&block : Crystal::Token -> _)
run_normal_state @lexer, &block
true
@@ -53,8 +50,7 @@ module Ameba
false
end
- private def run_normal_state(lexer, break_on_rcurly = false,
- &block : Crystal::Token -> _)
+ private def run_normal_state(lexer, break_on_rcurly = false, &block : Crystal::Token -> _)
loop do
token = @lexer.next_token
block.call token
@@ -68,8 +64,6 @@ module Ameba
break
when :"}"
break if break_on_rcurly
- else
- # go on
end
end
end
@@ -86,8 +80,6 @@ module Ameba
run_normal_state lexer, break_on_rcurly: true, &block
when :EOF
break
- else
- # go on
end
end
end
@@ -102,8 +94,6 @@ module Ameba
break
when :EOF
break
- else
- # go on
end
end
end