Skip to content

Further refinements #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Dec 11, 2022
Merged
43 changes: 21 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,21 @@

- [About](#about)
- [Usage](#usage)
* [Watch a tutorial](#watch-a-tutorial)
* [Autocorrection](#autocorrection)
* [Explain issues](#explain-issues)
* [Run in parallel](#run-in-parallel)
- [Watch a tutorial](#watch-a-tutorial)
- [Autocorrection](#autocorrection)
- [Explain issues](#explain-issues)
- [Run in parallel](#run-in-parallel)
- [Installation](#installation)
* [As a project dependency:](#as-a-project-dependency)
* [OS X](#os-x)
* [Docker](#docker)
* [From sources](#from-sources)
- [As a project dependency:](#as-a-project-dependency)
- [OS X](#os-x)
- [Docker](#docker)
- [From sources](#from-sources)
- [Configuration](#configuration)
* [Sources](#sources)
* [Rules](#rules)
* [Inline disabling](#inline-disabling)
- [Editors & integrations](#editors--integrations)
- [Credits & inspirations](#credits--inspirations)
- [Sources](#sources)
- [Rules](#rules)
- [Inline disabling](#inline-disabling)
- [Editors \& integrations](#editors--integrations)
- [Credits \& inspirations](#credits--inspirations)
- [Contributors](#contributors)

## About
Expand Down Expand Up @@ -201,8 +201,8 @@ In this example we define default globs and exclude `src/compiler` folder:

``` yaml
Globs:
- **/*.cr
- !lib
- "**/*.cr"
- "!lib"

Excluded:
- src/compiler
Expand Down Expand Up @@ -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) # ameba:disable Style/LargeNumbers, Lint/UselessAssign

time = Time.epoch(1483859302) # ameba:disable Style, Lint
```

## Editors & integrations

* 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)
* Sublime Text: [Sublime Linter Ameba](https://github.com/epergo/SublimeLinter-contrib-ameba)
* VSCode: [vscode-crystal-ameba](https://github.com/crystal-ameba/vscode-crystal-ameba)
* Codacy: [codacy-ameba](https://github.com/codacy/codacy-ameba)
* GitHub Actions: [github-action](https://github.com/crystal-ameba/github-action)
- 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)
- Sublime Text: [Sublime Linter Ameba](https://github.com/epergo/SublimeLinter-contrib-ameba)
- VSCode: [vscode-crystal-ameba](https://github.com/crystal-ameba/vscode-crystal-ameba)
- Codacy: [codacy-ameba](https://github.com/codacy/codacy-ameba)
- GitHub Actions: [github-action](https://github.com/crystal-ameba/github-action)

## Credits & inspirations

Expand Down
16 changes: 8 additions & 8 deletions spec/ameba/formatter/util_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module Ameba::Formatter

describe "#context" do
it "returns correct pre/post context lines" do
source = Source.new <<-EOF
source = Source.new <<-CRYSTAL
# pre:1
# pre:2
# pre:3
Expand All @@ -51,7 +51,7 @@ module Ameba::Formatter
# post:3
# post:4
# post:5
EOF
CRYSTAL

subject.context(source.lines, lineno: 6, context_lines: 3)
.should eq({<<-PRE.lines, <<-POST.lines
Expand All @@ -69,24 +69,24 @@ module Ameba::Formatter

describe "#affected_code" do
it "returns nil if there is no such a line number" do
code = <<-EOF
code = <<-CRYSTAL
a = 1
EOF
CRYSTAL
location = Crystal::Location.new("filename", 2, 1)
subject.affected_code(code, location).should be_nil
end

it "returns correct line if it is found" do
code = <<-EOF
code = <<-CRYSTAL
a = 1
EOF
CRYSTAL
location = Crystal::Location.new("filename", 1, 1)
subject.deansify(subject.affected_code(code, location))
.should eq "> a = 1\n ^\n"
end

it "returns correct line if it is found" do
code = <<-EOF
code = <<-CRYSTAL
# pre:1
# pre:2
# pre:3
Expand All @@ -98,7 +98,7 @@ module Ameba::Formatter
# post:3
# post:4
# post:5
EOF
CRYSTAL

location = Crystal::Location.new("filename", 6, 1)
subject.deansify(subject.affected_code(code, location, context_lines: 3))
Expand Down
15 changes: 1 addition & 14 deletions spec/ameba/rule/lint/literals_comparison_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,13 @@ module Ameba::Rule::Lint
CRYSTAL
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
pending "reports in macro scope" do
it "reports in macro scope" do
expect_issue subject, <<-CRYSTAL
{{ "foo" == "foo" }}
# ^^^^^^^^^^^^^^ error: Comparison always evaluates to true
CRYSTAL
end

it "passes for free variables comparisons in macro scope" do
expect_no_issues subject, <<-CRYSTAL
{{ T == Nil }}
CRYSTAL
end
end

it "reports rule, pos and message" do
Expand Down
7 changes: 7 additions & 0 deletions spec/ameba/rule/lint/not_nil_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ module Ameba::Rule::Lint
CRYSTAL
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
it "doesn't report in macro scope" do
expect_no_issues subject, <<-CRYSTAL
Expand Down
4 changes: 2 additions & 2 deletions spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ require "../../../spec_helper"

module Ameba::Rule::Metrics
subject = CyclomaticComplexity.new
complex_method = <<-CODE
complex_method = <<-CRYSTAL
def hello(a, b, c)
if a && b && c
begin
Expand All @@ -15,7 +15,7 @@ module Ameba::Rule::Metrics
end
end
end
CODE
CRYSTAL

describe CyclomaticComplexity do
it "passes for empty methods" do
Expand Down
12 changes: 10 additions & 2 deletions spec/ameba/rule/style/is_a_nil_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,25 @@ module Ameba::Rule::Style
end

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)
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
CRYSTAL

expect_correction source, <<-CRYSTAL
a = self.nil?
CRYSTAL
end

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)
# ^^^ error: Use `nil?` instead of `is_a?(Nil)`
CRYSTAL

expect_correction source, <<-CRYSTAL
a.nil?
CRYSTAL
end

it "reports rule, location and message" do
Expand Down
2 changes: 1 addition & 1 deletion src/ameba/ast/scope.cr
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ module Ameba::AST
node.is_a?(Crystal::CStructOrUnionDef)
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.
def references?(variable : Variable, check_inner_scopes = true)
variable.references.any? do |reference|
Expand Down
30 changes: 14 additions & 16 deletions src/ameba/ast/util.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Ameba::AST::Util
#
# 1. is *node* a literal?
# 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
when Crystal::NilLiteral,
Crystal::BoolLiteral,
Expand All @@ -17,44 +17,42 @@ module Ameba::AST::Util
Crystal::MacroLiteral
{true, true}
when Crystal::RangeLiteral
{true, static_literal?(node.from, include_paths) &&
static_literal?(node.to, include_paths)}
{true, static_literal?(node.from) &&
static_literal?(node.to)}
when Crystal::ArrayLiteral,
Crystal::TupleLiteral
{true, node.elements.all? do |el|
static_literal?(el, include_paths)
static_literal?(el)
end}
when Crystal::HashLiteral
{true, node.entries.all? do |entry|
static_literal?(entry.key, include_paths) &&
static_literal?(entry.value, include_paths)
static_literal?(entry.key) &&
static_literal?(entry.value)
end}
when Crystal::NamedTupleLiteral
{true, node.entries.all? do |entry|
static_literal?(entry.value, include_paths)
static_literal?(entry.value)
end}
when Crystal::Path
{include_paths, true}
else
{false, false}
end
end

# Returns `true` if current `node` is a static literal, `false` otherwise.
def static_literal?(node, include_paths = false) : Bool
is_literal, is_static = literal_kind?(node, include_paths)
def static_literal?(node) : Bool
is_literal, is_static = literal_kind?(node)
is_literal && is_static
end

# Returns `true` if current `node` is a dynamic literal, `false` otherwise.
def dynamic_literal?(node, include_paths = false) : Bool
is_literal, is_static = literal_kind?(node, include_paths)
def dynamic_literal?(node) : Bool
is_literal, is_static = literal_kind?(node)
is_literal && !is_static
end

# Returns `true` if current `node` is a literal, `false` otherwise.
def literal?(node, include_paths = false) : Bool
is_literal, _ = literal_kind?(node, include_paths)
def literal?(node) : Bool
is_literal, _ = literal_kind?(node)
is_literal
end

Expand Down Expand Up @@ -93,8 +91,8 @@ module Ameba::AST::Util
end

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')
end

Expand Down
2 changes: 1 addition & 1 deletion src/ameba/ast/variabling/variable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ module Ameba::AST
node.accept self
end

# @[AlwaysInline]
@[AlwaysInline]
private def includes_reference?(val)
val.to_s.includes?(@reference)
end
Expand Down
4 changes: 2 additions & 2 deletions src/ameba/reportable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ module Ameba

# :ditto:
def add_issue(rule,
location : Crystal::Location,
end_location : Crystal::Location,
location : Crystal::Location?,
end_location : Crystal::Location?,
message : String,
status : Issue::Status? = nil,
&block : Source::Corrector ->) : Issue
Expand Down
1 change: 0 additions & 1 deletion src/ameba/rule/lint/empty_ensure.cr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ module Ameba::Rule::Lint
#
# And it should be written as this:
#
#
# ```
# def some_method
# do_some_stuff
Expand Down
8 changes: 4 additions & 4 deletions src/ameba/rule/lint/empty_loop.cr
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ module Ameba::Rule::Lint
MSG = "Empty loop detected"

def test(source, node : Crystal::Call)
return unless loop?(node)

check_node(source, node, node.block)
check_node(source, node, node.block) if loop?(node)
end

def test(source, node : Crystal::While | Crystal::Until)
Expand All @@ -58,7 +56,9 @@ module Ameba::Rule::Lint

private def check_node(source, node, 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
2 changes: 1 addition & 1 deletion src/ameba/rule/lint/literal_assignments_in_expressions.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module Ameba::Rule::Lint

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 literal?(cond.value, include_paths: true)
return unless literal?(cond.value)

issue_for cond, MSG
end
Expand Down
7 changes: 2 additions & 5 deletions src/ameba/rule/lint/literal_in_condition.cr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Ameba::Rule::Lint
# replaced with either the body of the construct, or deleted entirely.
#
# This is considered invalid:
#
# ```
# if "something"
# :ok
Expand All @@ -29,12 +30,8 @@ module Ameba::Rule::Lint

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)
check_node source, node
issue_for node, MSG if static_literal?(node.cond)
end
end
end
Loading