mirror of
https://gitea.invidious.io/iv-org/shard-ameba.git
synced 2024-08-15 00:53:29 +00:00
Merge pull request #324 from crystal-ameba/Sija/extend-several-rule-with-corrections
Extend `Lint/UnusedArgument` and `Lint/UnusedBlockArgument` rules with corrections
This commit is contained in:
commit
4f8b79ec6b
4 changed files with 190 additions and 149 deletions
|
@ -6,7 +6,7 @@ module Ameba::Rule::Lint
|
|||
|
||||
describe UnusedArgument do
|
||||
it "doesn't report if arguments are used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(a, b, c)
|
||||
a + b + c
|
||||
end
|
||||
|
@ -16,155 +16,158 @@ module Ameba::Rule::Lint
|
|||
end
|
||||
|
||||
->(i : Int32) { i + 1 }
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports if method argument is unused" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
def method(a, b, c)
|
||||
# ^ error: Unused argument `c`. If it's necessary, use `_c` as an argument name to indicate that it won't be used.
|
||||
a + b
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues.first.message.should eq "Unused argument `c`. If it's necessary, use `_c` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
def method(a, b, _c)
|
||||
a + b
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports if block argument is unused" do
|
||||
s = Source.new %(
|
||||
[1,2].each_with_index do |a, i|
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
[1, 2].each_with_index do |a, i|
|
||||
# ^ error: Unused argument `i`. [...]
|
||||
a
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues.first.message.should eq "Unused argument `i`. If it's necessary, use `_` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
[1, 2].each_with_index do |a, _|
|
||||
a
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports if proc argument is unused" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
-> (a : Int32, b : String) do
|
||||
# ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used.
|
||||
a = a + 1
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
-> (a : Int32, _b : String) do
|
||||
a = a + 1
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports multiple unused args" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
def method(a, b, c)
|
||||
# ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used.
|
||||
# ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used.
|
||||
# ^ error: Unused argument `c`. If it's necessary, use `_c` as an argument name to indicate that it won't be used.
|
||||
nil
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues[0].message.should eq "Unused argument `a`. If it's necessary, use `_a` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
s.issues[1].message.should eq "Unused argument `b`. If it's necessary, use `_b` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
s.issues[2].message.should eq "Unused argument `c`. If it's necessary, use `_c` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
def method(_a, _b, _c)
|
||||
nil
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if it is an instance var argument" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
class A
|
||||
def method(@name)
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if a typed argument is used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(x : Int32)
|
||||
3.times do
|
||||
puts x
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if an argument with default value is used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(x = 1)
|
||||
puts x
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if argument starts with a _" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(_x)
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if it is a block and used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(&block)
|
||||
block.call
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if block arg is not used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(&block)
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if unused and there is yield" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(&block)
|
||||
yield 1
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if it's an anonymous block" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(&)
|
||||
yield 1
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if variable is referenced implicitly" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, b)
|
||||
super
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if arg if referenced in case" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def foo(a)
|
||||
case a
|
||||
when /foo/
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if enum in a record" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
class Class
|
||||
record Record do
|
||||
enum Enum
|
||||
|
@ -172,59 +175,49 @@ module Ameba::Rule::Lint
|
|||
end
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
context "super" do
|
||||
it "reports if variable is not referenced implicitly by super" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, b)
|
||||
# ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used.
|
||||
super a
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
end
|
||||
CRYSTAL
|
||||
|
||||
it "reports rule, location and message" do
|
||||
s = Source.new %(
|
||||
def method(a)
|
||||
expect_correction source, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, _b)
|
||||
super a
|
||||
end
|
||||
end
|
||||
), "source.cr"
|
||||
subject.catch(s).should_not be_valid
|
||||
issue = s.issues.first
|
||||
issue.rule.should_not be_nil
|
||||
issue.message.should eq "Unused argument `a`. If it's necessary, use `_a` " \
|
||||
"as an argument name to indicate that it won't be used."
|
||||
issue.location.to_s.should eq "source.cr:1:12"
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
|
||||
context "macro" do
|
||||
it "doesn't report if it is a used macro argument" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
macro my_macro(arg)
|
||||
{% arg %}
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if it is a used macro block argument" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
macro my_macro(&block)
|
||||
{% block %}
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report used macro args with equal names in record" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
record X do
|
||||
macro foo(a, b)
|
||||
{{ a }} + {{ b }}
|
||||
|
@ -234,12 +227,11 @@ module Ameba::Rule::Lint
|
|||
{{ a }} + {{ b }} + {{ c }}
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report used args in macro literals" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def print(f : Array(U)) forall U
|
||||
f.size.times do |i|
|
||||
{% if U == Float64 %}
|
||||
|
@ -249,65 +241,73 @@ module Ameba::Rule::Lint
|
|||
{% end %}
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
|
||||
context "properties" do
|
||||
describe "#ignore_defs" do
|
||||
it "lets the rule to ignore def scopes if true" do
|
||||
subject.ignore_defs = true
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_defs = true
|
||||
|
||||
expect_no_issues rule, <<-CRYSTAL
|
||||
def method(a)
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "lets the rule not to ignore def scopes if false" do
|
||||
subject.ignore_defs = false
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_defs = false
|
||||
|
||||
expect_issue rule, <<-CRYSTAL
|
||||
def method(a)
|
||||
# ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used.
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
|
||||
context "#ignore_blocks" do
|
||||
it "lets the rule to ignore block scopes if true" do
|
||||
subject.ignore_blocks = true
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_blocks = true
|
||||
|
||||
expect_no_issues rule, <<-CRYSTAL
|
||||
3.times { |i| puts "yo!" }
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "lets the rule not to ignore block scopes if false" do
|
||||
subject.ignore_blocks = false
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_blocks = false
|
||||
|
||||
expect_issue rule, <<-CRYSTAL
|
||||
3.times { |i| puts "yo!" }
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
# ^ error: Unused argument `i`. If it's necessary, use `_` as an argument name to indicate that it won't be used.
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
|
||||
context "#ignore_procs" do
|
||||
it "lets the rule to ignore proc scopes if true" do
|
||||
subject.ignore_procs = true
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_procs = true
|
||||
|
||||
expect_no_issues rule, <<-CRYSTAL
|
||||
->(a : Int32) {}
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "lets the rule not to ignore proc scopes if false" do
|
||||
subject.ignore_procs = false
|
||||
s = Source.new %(
|
||||
rule = UnusedArgument.new
|
||||
rule.ignore_procs = false
|
||||
|
||||
expect_issue rule, <<-CRYSTAL
|
||||
->(a : Int32) {}
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
# ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used.
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5,105 +5,118 @@ module Ameba::Rule::Lint
|
|||
|
||||
describe UnusedBlockArgument do
|
||||
it "doesn't report if it is an instance var argument" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
class A
|
||||
def initialize(&@callback)
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if anonymous" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(a, b, c, &)
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if argument name starts with a `_`" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(a, b, c, &_block)
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if it is a block and used" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(a, b, c, &block)
|
||||
block.call
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports if block arg is not used" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
def method(a, b, c, &block)
|
||||
# ^ error: Unused block argument `block`. [...]
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
def method(a, b, c, &_block)
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "reports if unused and there is yield" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
def method(a, b, c, &block)
|
||||
# ^ error: Use `&` as an argument name to indicate that it won't be referenced.
|
||||
3.times do |i|
|
||||
i.try do
|
||||
yield i
|
||||
end
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
def method(a, b, c, &)
|
||||
3.times do |i|
|
||||
i.try do
|
||||
yield i
|
||||
end
|
||||
end
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if anonymous and there is yield" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
def method(a, b, c, &)
|
||||
yield 1
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
it "doesn't report if variable is referenced implicitly" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, b, c, &block)
|
||||
super
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
|
||||
context "super" do
|
||||
it "reports if variable is not referenced implicitly by super" do
|
||||
s = Source.new %(
|
||||
source = expect_issue subject, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, b, c, &block)
|
||||
# ^ error: Unused block argument `block`. [...]
|
||||
super a, b, c
|
||||
end
|
||||
end
|
||||
)
|
||||
subject.catch(s).should_not be_valid
|
||||
s.issues.first.message.should eq "Unused block argument `block`. If it's necessary, " \
|
||||
"use `_block` as an argument name to indicate " \
|
||||
"that it won't be used."
|
||||
CRYSTAL
|
||||
|
||||
expect_correction source, <<-CRYSTAL
|
||||
class Bar < Foo
|
||||
def method(a, b, c, &_block)
|
||||
super a, b, c
|
||||
end
|
||||
end
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
|
||||
context "macro" do
|
||||
it "doesn't report if it is a used macro block argument" do
|
||||
s = Source.new %(
|
||||
expect_no_issues subject, <<-CRYSTAL
|
||||
macro my_macro(&block)
|
||||
{% block %}
|
||||
end
|
||||
)
|
||||
subject.catch(s).should be_valid
|
||||
CRYSTAL
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -63,7 +63,18 @@ module Ameba::Rule::Lint
|
|||
next if scope.references?(argument.variable)
|
||||
|
||||
name_suggestion = scope.node.is_a?(Crystal::Block) ? '_' : "_#{argument.name}"
|
||||
issue_for argument.node, MSG % {argument.name, name_suggestion}
|
||||
message = MSG % {argument.name, name_suggestion}
|
||||
|
||||
location = argument.node.location
|
||||
end_location = location.try &.adjust(column_number: argument.name.size - 1)
|
||||
|
||||
if location && end_location
|
||||
issue_for argument.node, message do |corrector|
|
||||
corrector.replace(location, end_location, name_suggestion)
|
||||
end
|
||||
else
|
||||
issue_for argument.node, message
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -31,6 +31,8 @@ module Ameba::Rule::Lint
|
|||
# Enabled: true
|
||||
# ```
|
||||
class UnusedBlockArgument < Base
|
||||
include AST::Util
|
||||
|
||||
properties do
|
||||
description "Disallows unused block arguments"
|
||||
end
|
||||
|
@ -51,11 +53,26 @@ module Ameba::Rule::Lint
|
|||
return if block_arg.anonymous?
|
||||
return if scope.references?(block_arg.variable)
|
||||
|
||||
location = block_arg.node.location
|
||||
end_location = location.try &.adjust(column_number: block_arg.name.size - 1)
|
||||
|
||||
if scope.yields?
|
||||
issue_for block_arg.node, MSG_YIELDED
|
||||
if location && end_location
|
||||
issue_for block_arg.node, MSG_YIELDED do |corrector|
|
||||
corrector.remove(location, end_location)
|
||||
end
|
||||
else
|
||||
issue_for block_arg.node, MSG_YIELDED
|
||||
end
|
||||
else
|
||||
return if block_arg.ignored?
|
||||
issue_for block_arg.node, MSG_UNUSED % block_arg.name
|
||||
if location && end_location
|
||||
issue_for block_arg.node, MSG_UNUSED % block_arg.name do |corrector|
|
||||
corrector.insert_before(location, '_')
|
||||
end
|
||||
else
|
||||
issue_for block_arg.node, MSG_UNUSED % block_arg.name
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue