mirror of
https://gitea.invidious.io/iv-org/shard-ameba.git
synced 2024-08-15 00:53:29 +00:00
Merge pull request #317 from crystal-ameba/Sija/further-refinements
This commit is contained in:
commit
4533e52aa5
40 changed files with 130 additions and 131 deletions
43
README.md
43
README.md
|
@ -19,21 +19,21 @@
|
||||||
|
|
||||||
- [About](#about)
|
- [About](#about)
|
||||||
- [Usage](#usage)
|
- [Usage](#usage)
|
||||||
* [Watch a tutorial](#watch-a-tutorial)
|
- [Watch a tutorial](#watch-a-tutorial)
|
||||||
* [Autocorrection](#autocorrection)
|
- [Autocorrection](#autocorrection)
|
||||||
* [Explain issues](#explain-issues)
|
- [Explain issues](#explain-issues)
|
||||||
* [Run in parallel](#run-in-parallel)
|
- [Run in parallel](#run-in-parallel)
|
||||||
- [Installation](#installation)
|
- [Installation](#installation)
|
||||||
* [As a project dependency:](#as-a-project-dependency)
|
- [As a project dependency:](#as-a-project-dependency)
|
||||||
* [OS X](#os-x)
|
- [OS X](#os-x)
|
||||||
* [Docker](#docker)
|
- [Docker](#docker)
|
||||||
* [From sources](#from-sources)
|
- [From sources](#from-sources)
|
||||||
- [Configuration](#configuration)
|
- [Configuration](#configuration)
|
||||||
* [Sources](#sources)
|
- [Sources](#sources)
|
||||||
* [Rules](#rules)
|
- [Rules](#rules)
|
||||||
* [Inline disabling](#inline-disabling)
|
- [Inline disabling](#inline-disabling)
|
||||||
- [Editors & integrations](#editors--integrations)
|
- [Editors \& integrations](#editors--integrations)
|
||||||
- [Credits & inspirations](#credits--inspirations)
|
- [Credits \& inspirations](#credits--inspirations)
|
||||||
- [Contributors](#contributors)
|
- [Contributors](#contributors)
|
||||||
|
|
||||||
## About
|
## About
|
||||||
|
@ -201,8 +201,8 @@ In this example we define default globs and exclude `src/compiler` folder:
|
||||||
|
|
||||||
``` yaml
|
``` yaml
|
||||||
Globs:
|
Globs:
|
||||||
- **/*.cr
|
- "**/*.cr"
|
||||||
- !lib
|
- "!lib"
|
||||||
|
|
||||||
Excluded:
|
Excluded:
|
||||||
- src/compiler
|
- src/compiler
|
||||||
|
@ -245,18 +245,17 @@ One or more rules or one or more group of rules can be disabled using inline dir
|
||||||
time = Time.epoch(1483859302)
|
time = Time.epoch(1483859302)
|
||||||
|
|
||||||
time = Time.epoch(1483859302) # ameba:disable Style/LargeNumbers, Lint/UselessAssign
|
time = Time.epoch(1483859302) # ameba:disable Style/LargeNumbers, Lint/UselessAssign
|
||||||
|
|
||||||
time = Time.epoch(1483859302) # ameba:disable Style, Lint
|
time = Time.epoch(1483859302) # ameba:disable Style, Lint
|
||||||
```
|
```
|
||||||
|
|
||||||
## Editors & integrations
|
## Editors & integrations
|
||||||
|
|
||||||
* Vim: [vim-crystal](https://github.com/rhysd/vim-crystal), [Ale](https://github.com/w0rp/ale)
|
- Vim: [vim-crystal](https://github.com/rhysd/vim-crystal), [Ale](https://github.com/w0rp/ale)
|
||||||
* Emacs: [ameba.el](https://github.com/crystal-ameba/ameba.el)
|
- Emacs: [ameba.el](https://github.com/crystal-ameba/ameba.el)
|
||||||
* Sublime Text: [Sublime Linter Ameba](https://github.com/epergo/SublimeLinter-contrib-ameba)
|
- Sublime Text: [Sublime Linter Ameba](https://github.com/epergo/SublimeLinter-contrib-ameba)
|
||||||
* VSCode: [vscode-crystal-ameba](https://github.com/crystal-ameba/vscode-crystal-ameba)
|
- VSCode: [vscode-crystal-ameba](https://github.com/crystal-ameba/vscode-crystal-ameba)
|
||||||
* Codacy: [codacy-ameba](https://github.com/codacy/codacy-ameba)
|
- Codacy: [codacy-ameba](https://github.com/codacy/codacy-ameba)
|
||||||
* GitHub Actions: [github-action](https://github.com/crystal-ameba/github-action)
|
- GitHub Actions: [github-action](https://github.com/crystal-ameba/github-action)
|
||||||
|
|
||||||
## Credits & inspirations
|
## Credits & inspirations
|
||||||
|
|
||||||
|
|
|
@ -39,7 +39,7 @@ module Ameba::Formatter
|
||||||
|
|
||||||
describe "#context" do
|
describe "#context" do
|
||||||
it "returns correct pre/post context lines" do
|
it "returns correct pre/post context lines" do
|
||||||
source = Source.new <<-EOF
|
source = Source.new <<-CRYSTAL
|
||||||
# pre:1
|
# pre:1
|
||||||
# pre:2
|
# pre:2
|
||||||
# pre:3
|
# pre:3
|
||||||
|
@ -51,7 +51,7 @@ module Ameba::Formatter
|
||||||
# post:3
|
# post:3
|
||||||
# post:4
|
# post:4
|
||||||
# post:5
|
# post:5
|
||||||
EOF
|
CRYSTAL
|
||||||
|
|
||||||
subject.context(source.lines, lineno: 6, context_lines: 3)
|
subject.context(source.lines, lineno: 6, context_lines: 3)
|
||||||
.should eq({<<-PRE.lines, <<-POST.lines
|
.should eq({<<-PRE.lines, <<-POST.lines
|
||||||
|
@ -69,24 +69,24 @@ module Ameba::Formatter
|
||||||
|
|
||||||
describe "#affected_code" do
|
describe "#affected_code" do
|
||||||
it "returns nil if there is no such a line number" do
|
it "returns nil if there is no such a line number" do
|
||||||
code = <<-EOF
|
code = <<-CRYSTAL
|
||||||
a = 1
|
a = 1
|
||||||
EOF
|
CRYSTAL
|
||||||
location = Crystal::Location.new("filename", 2, 1)
|
location = Crystal::Location.new("filename", 2, 1)
|
||||||
subject.affected_code(code, location).should be_nil
|
subject.affected_code(code, location).should be_nil
|
||||||
end
|
end
|
||||||
|
|
||||||
it "returns correct line if it is found" do
|
it "returns correct line if it is found" do
|
||||||
code = <<-EOF
|
code = <<-CRYSTAL
|
||||||
a = 1
|
a = 1
|
||||||
EOF
|
CRYSTAL
|
||||||
location = Crystal::Location.new("filename", 1, 1)
|
location = Crystal::Location.new("filename", 1, 1)
|
||||||
subject.deansify(subject.affected_code(code, location))
|
subject.deansify(subject.affected_code(code, location))
|
||||||
.should eq "> a = 1\n ^\n"
|
.should eq "> a = 1\n ^\n"
|
||||||
end
|
end
|
||||||
|
|
||||||
it "returns correct line if it is found" do
|
it "returns correct line if it is found" do
|
||||||
code = <<-EOF
|
code = <<-CRYSTAL
|
||||||
# pre:1
|
# pre:1
|
||||||
# pre:2
|
# pre:2
|
||||||
# pre:3
|
# pre:3
|
||||||
|
@ -98,7 +98,7 @@ module Ameba::Formatter
|
||||||
# post:3
|
# post:3
|
||||||
# post:4
|
# post:4
|
||||||
# post:5
|
# post:5
|
||||||
EOF
|
CRYSTAL
|
||||||
|
|
||||||
location = Crystal::Location.new("filename", 6, 1)
|
location = Crystal::Location.new("filename", 6, 1)
|
||||||
subject.deansify(subject.affected_code(code, location, context_lines: 3))
|
subject.deansify(subject.affected_code(code, location, context_lines: 3))
|
||||||
|
|
|
@ -41,24 +41,20 @@ module Ameba::Rule::Lint
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
||||||
it "reports if there is a static path comparison evaluating to false" do
|
|
||||||
expect_issue subject, <<-CRYSTAL
|
|
||||||
String == Nil
|
|
||||||
# ^^^^^^^^^^^ error: Comparison always evaluates to false
|
|
||||||
CRYSTAL
|
|
||||||
end
|
|
||||||
|
|
||||||
context "macro" do
|
context "macro" do
|
||||||
pending "reports in macro scope" do
|
it "reports in macro scope" do
|
||||||
expect_issue subject, <<-CRYSTAL
|
expect_issue subject, <<-CRYSTAL
|
||||||
{{ "foo" == "foo" }}
|
{{ "foo" == "foo" }}
|
||||||
# ^^^^^^^^^^^^^^ error: Comparison always evaluates to true
|
# ^^^^^^^^^^^^^^ error: Comparison always evaluates to true
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
||||||
it "passes for free variables comparisons in macro scope" do
|
it "passes for valid cases" do
|
||||||
expect_no_issues subject, <<-CRYSTAL
|
expect_no_issues subject, <<-CRYSTAL
|
||||||
{{ T == Nil }}
|
{{ "foo" == foo }}
|
||||||
|
{{ "foo" != foo }}
|
||||||
|
{% foo == "foo" %}
|
||||||
|
{% foo != "foo" %}
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -18,6 +18,13 @@ module Ameba::Rule::Lint
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "reports if there is a `not_nil!` call in the middle of the call-chain" do
|
||||||
|
expect_issue subject, <<-CRYSTAL
|
||||||
|
(1..3).first?.not_nil!.to_s
|
||||||
|
# ^^^^^^^^ error: Avoid using `not_nil!`
|
||||||
|
CRYSTAL
|
||||||
|
end
|
||||||
|
|
||||||
context "macro" do
|
context "macro" do
|
||||||
it "doesn't report in macro scope" do
|
it "doesn't report in macro scope" do
|
||||||
expect_no_issues subject, <<-CRYSTAL
|
expect_no_issues subject, <<-CRYSTAL
|
||||||
|
|
|
@ -2,7 +2,7 @@ require "../../../spec_helper"
|
||||||
|
|
||||||
module Ameba::Rule::Metrics
|
module Ameba::Rule::Metrics
|
||||||
subject = CyclomaticComplexity.new
|
subject = CyclomaticComplexity.new
|
||||||
complex_method = <<-CODE
|
complex_method = <<-CRYSTAL
|
||||||
def hello(a, b, c)
|
def hello(a, b, c)
|
||||||
if a && b && c
|
if a && b && c
|
||||||
begin
|
begin
|
||||||
|
@ -15,7 +15,7 @@ module Ameba::Rule::Metrics
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
CODE
|
CRYSTAL
|
||||||
|
|
||||||
describe CyclomaticComplexity do
|
describe CyclomaticComplexity do
|
||||||
it "passes for empty methods" do
|
it "passes for empty methods" do
|
||||||
|
|
|
@ -14,17 +14,25 @@ module Ameba::Rule::Style
|
||||||
end
|
end
|
||||||
|
|
||||||
it "reports if there is a call to is_a?(Nil) without receiver" do
|
it "reports if there is a call to is_a?(Nil) without receiver" do
|
||||||
expect_issue subject, <<-CRYSTAL
|
source = expect_issue subject, <<-CRYSTAL
|
||||||
a = is_a?(Nil)
|
a = is_a?(Nil)
|
||||||
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
|
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
|
|
||||||
|
expect_correction source, <<-CRYSTAL
|
||||||
|
a = self.nil?
|
||||||
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
||||||
it "reports if there is a call to is_a?(Nil) with receiver" do
|
it "reports if there is a call to is_a?(Nil) with receiver" do
|
||||||
expect_issue subject, <<-CRYSTAL
|
source = expect_issue subject, <<-CRYSTAL
|
||||||
a.is_a?(Nil)
|
a.is_a?(Nil)
|
||||||
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
|
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
|
||||||
CRYSTAL
|
CRYSTAL
|
||||||
|
|
||||||
|
expect_correction source, <<-CRYSTAL
|
||||||
|
a.nil?
|
||||||
|
CRYSTAL
|
||||||
end
|
end
|
||||||
|
|
||||||
it "reports rule, location and message" do
|
it "reports rule, location and message" do
|
||||||
|
|
|
@ -32,7 +32,7 @@ module Ameba::AST
|
||||||
def initialize(@node, @parent)
|
def initialize(@node, @parent)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if current branch is in a loop, false - otherwise.
|
# Returns `true` if current branch is in a loop, `false` - otherwise.
|
||||||
# For example, this branch is in a loop:
|
# For example, this branch is in a loop:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -35,7 +35,8 @@ module Ameba::AST
|
||||||
def initialize(@node, @parent = nil)
|
def initialize(@node, @parent = nil)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if this node or one of the parent branchables is a loop, false otherwise.
|
# Returns `true` if this node or one of the parent branchables is a loop,
|
||||||
|
# `false` otherwise.
|
||||||
def loop?
|
def loop?
|
||||||
loop?(node) || !!parent.try(&.loop?)
|
loop?(node) || !!parent.try(&.loop?)
|
||||||
end
|
end
|
||||||
|
|
|
@ -134,7 +134,7 @@ module Ameba::AST
|
||||||
node.is_a?(Crystal::CStructOrUnionDef)
|
node.is_a?(Crystal::CStructOrUnionDef)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if current scope (or any of inner scopes) references variable,
|
# Returns `true` if current scope (or any of inner scopes) references variable,
|
||||||
# `false` otherwise.
|
# `false` otherwise.
|
||||||
def references?(variable : Variable, check_inner_scopes = true)
|
def references?(variable : Variable, check_inner_scopes = true)
|
||||||
variable.references.any? do |reference|
|
variable.references.any? do |reference|
|
||||||
|
@ -153,7 +153,7 @@ module Ameba::AST
|
||||||
outer_scope.nil?
|
outer_scope.nil?
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if var is an argument in current scope, `false` otherwise.
|
# Returns `true` if var is an argument in current scope, `false` otherwise.
|
||||||
def arg?(var)
|
def arg?(var)
|
||||||
case current_node = node
|
case current_node = node
|
||||||
when Crystal::Def
|
when Crystal::Def
|
||||||
|
|
|
@ -4,7 +4,7 @@ module Ameba::AST::Util
|
||||||
#
|
#
|
||||||
# 1. is *node* a literal?
|
# 1. is *node* a literal?
|
||||||
# 2. can *node* be proven static?
|
# 2. can *node* be proven static?
|
||||||
protected def literal_kind?(node, include_paths = false) : {Bool, Bool}
|
protected def literal_kind?(node) : {Bool, Bool}
|
||||||
case node
|
case node
|
||||||
when Crystal::NilLiteral,
|
when Crystal::NilLiteral,
|
||||||
Crystal::BoolLiteral,
|
Crystal::BoolLiteral,
|
||||||
|
@ -17,44 +17,42 @@ module Ameba::AST::Util
|
||||||
Crystal::MacroLiteral
|
Crystal::MacroLiteral
|
||||||
{true, true}
|
{true, true}
|
||||||
when Crystal::RangeLiteral
|
when Crystal::RangeLiteral
|
||||||
{true, static_literal?(node.from, include_paths) &&
|
{true, static_literal?(node.from) &&
|
||||||
static_literal?(node.to, include_paths)}
|
static_literal?(node.to)}
|
||||||
when Crystal::ArrayLiteral,
|
when Crystal::ArrayLiteral,
|
||||||
Crystal::TupleLiteral
|
Crystal::TupleLiteral
|
||||||
{true, node.elements.all? do |el|
|
{true, node.elements.all? do |el|
|
||||||
static_literal?(el, include_paths)
|
static_literal?(el)
|
||||||
end}
|
end}
|
||||||
when Crystal::HashLiteral
|
when Crystal::HashLiteral
|
||||||
{true, node.entries.all? do |entry|
|
{true, node.entries.all? do |entry|
|
||||||
static_literal?(entry.key, include_paths) &&
|
static_literal?(entry.key) &&
|
||||||
static_literal?(entry.value, include_paths)
|
static_literal?(entry.value)
|
||||||
end}
|
end}
|
||||||
when Crystal::NamedTupleLiteral
|
when Crystal::NamedTupleLiteral
|
||||||
{true, node.entries.all? do |entry|
|
{true, node.entries.all? do |entry|
|
||||||
static_literal?(entry.value, include_paths)
|
static_literal?(entry.value)
|
||||||
end}
|
end}
|
||||||
when Crystal::Path
|
|
||||||
{include_paths, true}
|
|
||||||
else
|
else
|
||||||
{false, false}
|
{false, false}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns `true` if current `node` is a static literal, `false` otherwise.
|
# Returns `true` if current `node` is a static literal, `false` otherwise.
|
||||||
def static_literal?(node, include_paths = false) : Bool
|
def static_literal?(node) : Bool
|
||||||
is_literal, is_static = literal_kind?(node, include_paths)
|
is_literal, is_static = literal_kind?(node)
|
||||||
is_literal && is_static
|
is_literal && is_static
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns `true` if current `node` is a dynamic literal, `false` otherwise.
|
# Returns `true` if current `node` is a dynamic literal, `false` otherwise.
|
||||||
def dynamic_literal?(node, include_paths = false) : Bool
|
def dynamic_literal?(node) : Bool
|
||||||
is_literal, is_static = literal_kind?(node, include_paths)
|
is_literal, is_static = literal_kind?(node)
|
||||||
is_literal && !is_static
|
is_literal && !is_static
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns `true` if current `node` is a literal, `false` otherwise.
|
# Returns `true` if current `node` is a literal, `false` otherwise.
|
||||||
def literal?(node, include_paths = false) : Bool
|
def literal?(node) : Bool
|
||||||
is_literal, _ = literal_kind?(node, include_paths)
|
is_literal, _ = literal_kind?(node)
|
||||||
is_literal
|
is_literal
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -93,8 +91,8 @@ module Ameba::AST::Util
|
||||||
end
|
end
|
||||||
|
|
||||||
return if last_line.size < end_column + 1
|
return if last_line.size < end_column + 1
|
||||||
node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "")
|
|
||||||
|
|
||||||
|
node_lines[-1] = last_line.sub(end_column + 1...last_line.size, "")
|
||||||
node_lines.join('\n')
|
node_lines.join('\n')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -31,7 +31,7 @@ module Ameba::AST
|
||||||
def initialize(@node, @variable)
|
def initialize(@node, @variable)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if the name starts with '_', false if not.
|
# Returns `true` if the name starts with '_', `false` if not.
|
||||||
def ignored?
|
def ignored?
|
||||||
name.starts_with? '_'
|
name.starts_with? '_'
|
||||||
end
|
end
|
||||||
|
|
|
@ -41,7 +41,7 @@ module Ameba::AST
|
||||||
@variable.referenced? && !!@branch.try(&.in_loop?)
|
@variable.referenced? && !!@branch.try(&.in_loop?)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if this assignment is an op assign, false if not.
|
# Returns `true` if this assignment is an op assign, `false` if not.
|
||||||
# For example, this is an op assign:
|
# For example, this is an op assign:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
@ -51,7 +51,7 @@ module Ameba::AST
|
||||||
node.is_a?(Crystal::OpAssign)
|
node.is_a?(Crystal::OpAssign)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if this assignment is in a branch, false if not.
|
# Returns `true` if this assignment is in a branch, `false` if not.
|
||||||
# For example, this assignment is in a branch:
|
# For example, this assignment is in a branch:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -174,7 +174,7 @@ module Ameba::AST
|
||||||
node.accept self
|
node.accept self
|
||||||
end
|
end
|
||||||
|
|
||||||
# @[AlwaysInline]
|
@[AlwaysInline]
|
||||||
private def includes_reference?(val)
|
private def includes_reference?(val)
|
||||||
val.to_s.includes?(@reference)
|
val.to_s.includes?(@reference)
|
||||||
end
|
end
|
||||||
|
|
|
@ -20,7 +20,7 @@ module Ameba::AST
|
||||||
end
|
end
|
||||||
|
|
||||||
# A main visit method that accepts `Crystal::ASTNode`.
|
# A main visit method that accepts `Crystal::ASTNode`.
|
||||||
# Returns true meaning all child nodes will be traversed.
|
# Returns `true`, meaning all child nodes will be traversed.
|
||||||
def visit(node : Crystal::ASTNode)
|
def visit(node : Crystal::ASTNode)
|
||||||
true
|
true
|
||||||
end
|
end
|
||||||
|
|
|
@ -54,7 +54,7 @@ class Ameba::Config
|
||||||
# ```
|
# ```
|
||||||
property excluded : Array(String)
|
property excluded : Array(String)
|
||||||
|
|
||||||
# Returns true if correctable issues should be autocorrected.
|
# Returns `true` if correctable issues should be autocorrected.
|
||||||
property? autocorrect = false
|
property? autocorrect = false
|
||||||
|
|
||||||
@rule_groups : Hash(String, Array(Rule::Base))
|
@rule_groups : Hash(String, Array(Rule::Base))
|
||||||
|
|
|
@ -10,8 +10,8 @@ module Ameba
|
||||||
Enable
|
Enable
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if current location is disabled for a particular rule,
|
# Returns `true` if current location is disabled for a particular rule,
|
||||||
# false otherwise.
|
# `false` otherwise.
|
||||||
#
|
#
|
||||||
# Location is disabled in two cases:
|
# Location is disabled in two cases:
|
||||||
# 1. The line of the location ends with a comment directive.
|
# 1. The line of the location ends with a comment directive.
|
||||||
|
@ -74,7 +74,7 @@ module Ameba
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if the line at the given `line_number` is a comment.
|
# Returns `true` if the line at the given `line_number` is a comment.
|
||||||
def comment?(line_number : Int32)
|
def comment?(line_number : Int32)
|
||||||
return unless line = lines[line_number]?
|
return unless line = lines[line_number]?
|
||||||
comment?(line)
|
comment?(line)
|
||||||
|
|
|
@ -21,8 +21,8 @@ module Ameba
|
||||||
|
|
||||||
# :ditto:
|
# :ditto:
|
||||||
def add_issue(rule,
|
def add_issue(rule,
|
||||||
location : Crystal::Location,
|
location : Crystal::Location?,
|
||||||
end_location : Crystal::Location,
|
end_location : Crystal::Location?,
|
||||||
message : String,
|
message : String,
|
||||||
status : Issue::Status? = nil,
|
status : Issue::Status? = nil,
|
||||||
&block : Source::Corrector ->) : Issue
|
&block : Source::Corrector ->) : Issue
|
||||||
|
|
|
@ -94,7 +94,7 @@ module Ameba::Rule
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if this rule is special and behaves differently than
|
# Returns `true` if this rule is special and behaves differently than
|
||||||
# usual rules.
|
# usual rules.
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -17,7 +17,6 @@ module Ameba::Rule::Lint
|
||||||
#
|
#
|
||||||
# And it should be written as this:
|
# And it should be written as this:
|
||||||
#
|
#
|
||||||
#
|
|
||||||
# ```
|
# ```
|
||||||
# def some_method
|
# def some_method
|
||||||
# do_some_stuff
|
# do_some_stuff
|
||||||
|
|
|
@ -47,9 +47,7 @@ module Ameba::Rule::Lint
|
||||||
MSG = "Empty loop detected"
|
MSG = "Empty loop detected"
|
||||||
|
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless loop?(node)
|
check_node(source, node, node.block) if loop?(node)
|
||||||
|
|
||||||
check_node(source, node, node.block)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def test(source, node : Crystal::While | Crystal::Until)
|
def test(source, node : Crystal::While | Crystal::Until)
|
||||||
|
@ -58,7 +56,9 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
private def check_node(source, node, loop_body)
|
private def check_node(source, node, loop_body)
|
||||||
body = loop_body.is_a?(Crystal::Block) ? loop_body.body : loop_body
|
body = loop_body.is_a?(Crystal::Block) ? loop_body.body : loop_body
|
||||||
issue_for node, MSG if body.nil? || body.nop?
|
return unless body.nil? || body.nop?
|
||||||
|
|
||||||
|
issue_for node, MSG
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -35,7 +35,7 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until)
|
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until)
|
||||||
return unless (cond = node.cond).is_a?(Crystal::Assign)
|
return unless (cond = node.cond).is_a?(Crystal::Assign)
|
||||||
return unless literal?(cond.value, include_paths: true)
|
return unless literal?(cond.value)
|
||||||
|
|
||||||
issue_for cond, MSG
|
issue_for cond, MSG
|
||||||
end
|
end
|
||||||
|
|
|
@ -7,6 +7,7 @@ module Ameba::Rule::Lint
|
||||||
# replaced with either the body of the construct, or deleted entirely.
|
# replaced with either the body of the construct, or deleted entirely.
|
||||||
#
|
#
|
||||||
# This is considered invalid:
|
# This is considered invalid:
|
||||||
|
#
|
||||||
# ```
|
# ```
|
||||||
# if "something"
|
# if "something"
|
||||||
# :ok
|
# :ok
|
||||||
|
@ -29,12 +30,8 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
MSG = "Literal value found in conditional"
|
MSG = "Literal value found in conditional"
|
||||||
|
|
||||||
def check_node(source, node)
|
|
||||||
issue_for node, MSG if static_literal?(node.cond)
|
|
||||||
end
|
|
||||||
|
|
||||||
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case)
|
def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case)
|
||||||
check_node source, node
|
issue_for node, MSG if static_literal?(node.cond)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -4,7 +4,6 @@ module Ameba::Rule::Lint
|
||||||
# They usually have the same result - except for non-primitive
|
# They usually have the same result - except for non-primitive
|
||||||
# types like containers, range or regex.
|
# types like containers, range or regex.
|
||||||
#
|
#
|
||||||
#
|
|
||||||
# For example, this will be always false:
|
# For example, this will be always false:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
@ -29,28 +28,12 @@ module Ameba::Rule::Lint
|
||||||
MSG = "Comparison always evaluates to %s"
|
MSG = "Comparison always evaluates to %s"
|
||||||
MSG_LIKELY = "Comparison most likely evaluates to %s"
|
MSG_LIKELY = "Comparison most likely evaluates to %s"
|
||||||
|
|
||||||
# Edge-case: `{{ T == Nil }}`
|
|
||||||
#
|
|
||||||
# Current implementation just skips all macro contexts,
|
|
||||||
# regardless of the free variable being present.
|
|
||||||
#
|
|
||||||
# Ideally we should only check whether either of the sides
|
|
||||||
# is a free var
|
|
||||||
def test(source)
|
|
||||||
AST::NodeVisitor.new self, source, skip: [
|
|
||||||
Crystal::Macro,
|
|
||||||
Crystal::MacroExpression,
|
|
||||||
Crystal::MacroIf,
|
|
||||||
Crystal::MacroFor,
|
|
||||||
]
|
|
||||||
end
|
|
||||||
|
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless node.name.in?(OP_NAMES)
|
return unless node.name.in?(OP_NAMES)
|
||||||
return unless (obj = node.obj) && (arg = node.args.first?)
|
return unless (obj = node.obj) && (arg = node.args.first?)
|
||||||
|
|
||||||
obj_is_literal, obj_is_static = literal_kind?(obj, include_paths: true)
|
obj_is_literal, obj_is_static = literal_kind?(obj)
|
||||||
arg_is_literal, arg_is_static = literal_kind?(arg, include_paths: true)
|
arg_is_literal, arg_is_static = literal_kind?(arg)
|
||||||
|
|
||||||
return unless obj_is_literal && arg_is_literal
|
return unless obj_is_literal && arg_is_literal
|
||||||
|
|
||||||
|
|
|
@ -32,10 +32,8 @@ module Ameba::Rule::Lint
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless node.name == "rand" &&
|
return unless node.name == "rand" &&
|
||||||
node.args.size == 1 &&
|
node.args.size == 1 &&
|
||||||
(arg = node.args.first) &&
|
(arg = node.args.first).is_a?(Crystal::NumberLiteral) &&
|
||||||
arg.is_a?(Crystal::NumberLiteral) &&
|
arg.value.in?("0", "1")
|
||||||
(value = arg.value) &&
|
|
||||||
value.in?("0", "1")
|
|
||||||
|
|
||||||
issue_for node, MSG % node
|
issue_for node, MSG % node
|
||||||
end
|
end
|
||||||
|
|
|
@ -17,7 +17,7 @@ module Ameba::Rule::Lint
|
||||||
# YAML configuration example:
|
# YAML configuration example:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
# Lint/RedundantStringCoersion
|
# Lint/RedundantStringCoercion
|
||||||
# Enabled: true
|
# Enabled: true
|
||||||
# ```
|
# ```
|
||||||
class RedundantStringCoercion < Base
|
class RedundantStringCoercion < Base
|
||||||
|
|
|
@ -42,7 +42,6 @@ module Ameba::Rule::Lint
|
||||||
|
|
||||||
def test(source, node : Crystal::ExceptionHandler)
|
def test(source, node : Crystal::ExceptionHandler)
|
||||||
rescues = node.rescues
|
rescues = node.rescues
|
||||||
|
|
||||||
return if rescues.nil?
|
return if rescues.nil?
|
||||||
|
|
||||||
shadowed(rescues).each do |path|
|
shadowed(rescues).each do |path|
|
||||||
|
|
|
@ -63,8 +63,9 @@ module Ameba::Rule::Lint
|
||||||
return unless node.block
|
return unless node.block
|
||||||
|
|
||||||
arg = node.named_args.try &.find(&.name.== "focus")
|
arg = node.named_args.try &.find(&.name.== "focus")
|
||||||
|
return unless arg
|
||||||
|
|
||||||
issue_for arg, MSG if arg
|
issue_for arg, MSG
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -7,6 +7,7 @@ module Ameba::Rule::Lint
|
||||||
# a + b
|
# a + b
|
||||||
# end
|
# end
|
||||||
# ```
|
# ```
|
||||||
|
#
|
||||||
# and should be written as:
|
# and should be written as:
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
|
|
|
@ -41,6 +41,7 @@ module Ameba::Rule::Performance
|
||||||
return unless node.name == ANY_NAME
|
return unless node.name == ANY_NAME
|
||||||
return unless node.block.nil? && node.args.empty?
|
return unless node.block.nil? && node.args.empty?
|
||||||
return unless node.obj
|
return unless node.obj
|
||||||
|
|
||||||
return unless location = node.location
|
return unless location = node.location
|
||||||
return unless name_location = node.name_location
|
return unless name_location = node.name_location
|
||||||
return unless end_location = name_end_location(node)
|
return unless end_location = name_end_location(node)
|
||||||
|
|
|
@ -68,12 +68,13 @@ module Ameba::Rule::Performance
|
||||||
end
|
end
|
||||||
|
|
||||||
def test(source, node : Crystal::Call)
|
def test(source, node : Crystal::Call)
|
||||||
return unless location = node.name_location
|
|
||||||
return unless end_location = name_end_location(node)
|
|
||||||
return unless (obj = node.obj).is_a?(Crystal::Call)
|
return unless (obj = node.obj).is_a?(Crystal::Call)
|
||||||
return unless node.name.in?(call_names)
|
return unless node.name.in?(call_names)
|
||||||
return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES)
|
return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES)
|
||||||
|
|
||||||
|
return unless location = node.name_location
|
||||||
|
return unless end_location = name_end_location(node)
|
||||||
|
|
||||||
issue_for location, end_location, MSG % {node.name, obj.name} do |corrector|
|
issue_for location, end_location, MSG % {node.name, obj.name} do |corrector|
|
||||||
corrector.insert_after(end_location, '!')
|
corrector.insert_after(end_location, '!')
|
||||||
end
|
end
|
||||||
|
|
|
@ -51,7 +51,9 @@ module Ameba::Rule::Performance
|
||||||
return unless obj.name.in?(filter_names)
|
return unless obj.name.in?(filter_names)
|
||||||
|
|
||||||
message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE
|
message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE
|
||||||
issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name}
|
|
||||||
|
issue_for obj.name_location, node.name_end_location,
|
||||||
|
message % {obj.name, node.name}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -34,7 +34,9 @@ module Ameba::Rule::Style
|
||||||
const = node.const
|
const = node.const
|
||||||
return unless path_named?(const, "Nil")
|
return unless path_named?(const, "Nil")
|
||||||
|
|
||||||
issue_for const, MSG
|
issue_for const, MSG do |corrector|
|
||||||
|
corrector.replace(node, "#{node.obj}.nil?")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -59,20 +59,20 @@ module Ameba::Rule::Style
|
||||||
private def allowed?(_sign, value, fraction, _suffix)
|
private def allowed?(_sign, value, fraction, _suffix)
|
||||||
return true if fraction && fraction.size > 3
|
return true if fraction && fraction.size > 3
|
||||||
|
|
||||||
digits = value.chars.select(&.number?)
|
digits = value.chars.select!(&.number?)
|
||||||
digits.size >= int_min_digits
|
digits.size >= int_min_digits
|
||||||
end
|
end
|
||||||
|
|
||||||
private def underscored(sign, value, fraction, suffix)
|
private def underscored(sign, value, fraction, suffix)
|
||||||
value = slice_digits(value.reverse).reverse
|
value = slice_digits(value.reverse).reverse
|
||||||
fraction = "." + slice_digits(fraction) if fraction
|
fraction = ".#{slice_digits(fraction)}" if fraction
|
||||||
|
|
||||||
"#{sign}#{value}#{fraction}#{suffix}"
|
"#{sign}#{value}#{fraction}#{suffix}"
|
||||||
end
|
end
|
||||||
|
|
||||||
private def slice_digits(value, by = 3)
|
private def slice_digits(value, by = 3)
|
||||||
%w[].tap do |slices|
|
%w[].tap do |slices|
|
||||||
value.chars.reject(&.== '_').each_slice(by) do |slice|
|
value.chars.reject!(&.== '_').each_slice(by) do |slice|
|
||||||
slices << slice.join
|
slices << slice.join
|
||||||
end
|
end
|
||||||
end.join('_')
|
end.join('_')
|
||||||
|
|
|
@ -48,6 +48,7 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
def test(source, node : Crystal::Def)
|
def test(source, node : Crystal::Def)
|
||||||
return if (expected = node.name.underscore) == node.name
|
return if (expected = node.name.underscore) == node.name
|
||||||
|
|
||||||
return unless location = name_location(node)
|
return unless location = name_location(node)
|
||||||
return unless end_location = name_end_location(node)
|
return unless end_location = name_end_location(node)
|
||||||
|
|
||||||
|
|
|
@ -188,9 +188,6 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
# ameba:disable Metrics/CyclomaticComplexity
|
# ameba:disable Metrics/CyclomaticComplexity
|
||||||
protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
|
protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
|
||||||
return unless location = call.name_location
|
|
||||||
return unless end_location = block.end_location
|
|
||||||
|
|
||||||
return if exclude_calls_with_block? && body.block
|
return if exclude_calls_with_block? && body.block
|
||||||
return if exclude_multiple_line_blocks? && !same_location_lines?(call, body)
|
return if exclude_multiple_line_blocks? && !same_location_lines?(call, body)
|
||||||
return if exclude_prefix_operators? && prefix_operator?(body)
|
return if exclude_prefix_operators? && prefix_operator?(body)
|
||||||
|
@ -203,6 +200,9 @@ module Ameba::Rule::Style
|
||||||
return unless valid_line_length?(call, call_code)
|
return unless valid_line_length?(call, call_code)
|
||||||
return unless valid_length?(call_code)
|
return unless valid_length?(call_code)
|
||||||
|
|
||||||
|
return unless location = call.name_location
|
||||||
|
return unless end_location = block.end_location
|
||||||
|
|
||||||
if call_code.includes?("{...}")
|
if call_code.includes?("{...}")
|
||||||
issue_for location, end_location, MSG % call_code
|
issue_for location, end_location, MSG % call_code
|
||||||
else
|
else
|
||||||
|
|
|
@ -34,6 +34,7 @@ module Ameba::Rule::Style
|
||||||
|
|
||||||
def test(source, node : Crystal::While)
|
def test(source, node : Crystal::While)
|
||||||
return unless node.cond.true_literal?
|
return unless node.cond.true_literal?
|
||||||
|
|
||||||
return unless location = node.location
|
return unless location = node.location
|
||||||
return unless end_location = node.cond.end_location
|
return unless end_location = node.cond.end_location
|
||||||
|
|
||||||
|
|
|
@ -46,7 +46,7 @@ module Ameba
|
||||||
# Checks for unneeded disable directives. Always inspects a source last
|
# Checks for unneeded disable directives. Always inspects a source last
|
||||||
@unneeded_disable_directive_rule : Rule::Base?
|
@unneeded_disable_directive_rule : Rule::Base?
|
||||||
|
|
||||||
# Returns true if correctable issues should be autocorrected.
|
# Returns `true` if correctable issues should be autocorrected.
|
||||||
private getter? autocorrect : Bool
|
private getter? autocorrect : Bool
|
||||||
|
|
||||||
# Instantiates a runner using a `config`.
|
# Instantiates a runner using a `config`.
|
||||||
|
@ -162,7 +162,7 @@ module Ameba
|
||||||
end
|
end
|
||||||
|
|
||||||
# Indicates whether the last inspection successful or not.
|
# Indicates whether the last inspection successful or not.
|
||||||
# It returns true if no issues matching severity in sources found, false otherwise.
|
# It returns `true` if no issues matching severity in sources found, `false` otherwise.
|
||||||
#
|
#
|
||||||
# ```
|
# ```
|
||||||
# runner = Ameba::Runner.new config
|
# runner = Ameba::Runner.new config
|
||||||
|
|
|
@ -65,7 +65,7 @@ class Ameba::Source
|
||||||
@action_root = Rewriter::Action.new(0, code.size)
|
@action_root = Rewriter::Action.new(0, code.size)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Returns true if no (non trivial) update has been recorded
|
# Returns `true` if no (non trivial) update has been recorded
|
||||||
def empty?
|
def empty?
|
||||||
@action_root.empty?
|
@action_root.empty?
|
||||||
end
|
end
|
||||||
|
|
|
@ -70,7 +70,7 @@ class Ameba::Source::Rewriter
|
||||||
end
|
end
|
||||||
|
|
||||||
protected def place_in_hierarchy(action)
|
protected def place_in_hierarchy(action)
|
||||||
family = analyse_hierarchy(action)
|
family = analyze_hierarchy(action)
|
||||||
sibling_left, sibling_right = family[:sibling_left], family[:sibling_right]
|
sibling_left, sibling_right = family[:sibling_left], family[:sibling_right]
|
||||||
|
|
||||||
if fusible = family[:fusible]
|
if fusible = family[:fusible]
|
||||||
|
@ -126,7 +126,7 @@ class Ameba::Source::Rewriter
|
||||||
# In case a child has equal range to *action*, it is returned as `:parent`
|
# In case a child has equal range to *action*, it is returned as `:parent`
|
||||||
#
|
#
|
||||||
# Reminder: an empty range 1...1 is considered disjoint from 1...10
|
# Reminder: an empty range 1...1 is considered disjoint from 1...10
|
||||||
protected def analyse_hierarchy(action) # ameba:disable Metrics/CyclomaticComplexity
|
protected def analyze_hierarchy(action) # ameba:disable Metrics/CyclomaticComplexity
|
||||||
# left_index is the index of the first child that isn't completely to the left of action
|
# left_index is the index of the first child that isn't completely to the left of action
|
||||||
left_index = bsearch_child_index { |child| child.end_pos > action.begin_pos }
|
left_index = bsearch_child_index { |child| child.end_pos > action.begin_pos }
|
||||||
# right_index is the index of the first child that is completely on the right of action
|
# right_index is the index of the first child that is completely on the right of action
|
||||||
|
@ -147,6 +147,7 @@ class Ameba::Source::Rewriter
|
||||||
else
|
else
|
||||||
overlap_left = @children[left_index].begin_pos <=> action.begin_pos
|
overlap_left = @children[left_index].begin_pos <=> action.begin_pos
|
||||||
overlap_right = @children[right_index - 1].end_pos <=> action.end_pos
|
overlap_right = @children[right_index - 1].end_pos <=> action.end_pos
|
||||||
|
|
||||||
raise "Unable to compare begin pos" if overlap_left.nil?
|
raise "Unable to compare begin pos" if overlap_left.nil?
|
||||||
raise "Unable to compare end pos" if overlap_right.nil?
|
raise "Unable to compare end pos" if overlap_right.nil?
|
||||||
|
|
||||||
|
|
|
@ -160,8 +160,10 @@ module Ameba::Spec::ExpectIssue
|
||||||
file = __FILE__,
|
file = __FILE__,
|
||||||
line = __LINE__)
|
line = __LINE__)
|
||||||
lines = code.split('\n') # must preserve trailing newline
|
lines = code.split('\n') # must preserve trailing newline
|
||||||
|
|
||||||
_, actual_annotations = actual_annotations(rules, code, path, lines)
|
_, actual_annotations = actual_annotations(rules, code, path, lines)
|
||||||
return if actual_annotations.to_s == code
|
return if actual_annotations.to_s == code
|
||||||
|
|
||||||
fail <<-MSG, file, line
|
fail <<-MSG, file, line
|
||||||
Expected no issues, but got:
|
Expected no issues, but got:
|
||||||
|
|
||||||
|
@ -182,9 +184,10 @@ module Ameba::Spec::ExpectIssue
|
||||||
private def format_issue(code, **replacements)
|
private def format_issue(code, **replacements)
|
||||||
replacements.each do |keyword, value|
|
replacements.each do |keyword, value|
|
||||||
value = value.to_s
|
value = value.to_s
|
||||||
code = code.gsub("%{#{keyword}}", value)
|
code = code
|
||||||
code = code.gsub("^{#{keyword}}", "^" * value.size)
|
.gsub("%{#{keyword}}", value)
|
||||||
code = code.gsub("_{#{keyword}}", " " * value.size)
|
.gsub("^{#{keyword}}", "^" * value.size)
|
||||||
|
.gsub("_{#{keyword}}", " " * value.size)
|
||||||
end
|
end
|
||||||
code
|
code
|
||||||
end
|
end
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue