Convert Style/VerboseBlock spec

This commit is contained in:
fn ⌃ ⌥ 2021-10-27 15:51:00 -07:00
parent e93dfe9cdc
commit 571969265f
2 changed files with 119 additions and 123 deletions

View file

@ -5,7 +5,7 @@ module Ameba::Rule::Style
describe VerboseBlock do describe VerboseBlock do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).any?(&.odd?) (1..3).any?(&.odd?)
(1..3).join('.', &.to_s) (1..3).join('.', &.to_s)
(1..3).each_with_index { |i, idx| i * idx } (1..3).each_with_index { |i, idx| i * idx }
@ -14,192 +14,188 @@ module Ameba::Rule::Style
(1..3).map { |i| :foo } (1..3).map { |i| :foo }
(1..3).map { |i| :foo.to_s.split.join('.') } (1..3).map { |i| :foo.to_s.split.join('.') }
(1..3).map { :foo } (1..3).map { :foo }
) CRYSTAL
subject.catch(source).should be_valid
end end
it "passes if the block argument is used within the body" do it "passes if the block argument is used within the body" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).map { |i| i * i } (1..3).map { |i| i * i }
(1..3).map { |j| j * j.to_i64 } (1..3).map { |j| j * j.to_i64 }
(1..3).map { |k| k.to_i64 * k } (1..3).map { |k| k.to_i64 * k }
(1..3).map { |l| l.to_i64 * l.to_i64 } (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 { |m| m.to_s[start: m.to_i64, count: 3]? }
(1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join } (1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join }
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a call with a collapsible block" do it "reports if there is a call with a collapsible block" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).any? { |i| i.odd? } (1..3).any? { |i| i.odd? }
) # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is a call with an argument + collapsible block" do it "reports if there is a call with an argument + collapsible block" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).join('.') { |i| i.to_s } (1..3).join('.') { |i| i.to_s }
) # ^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `join('.', &.to_s)`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is a call with a collapsible block (with chained call)" do it "reports if there is a call with a collapsible block (with chained call)" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).map { |i| i.to_s.split.reverse.join.strip } (1..3).map { |i| i.to_s.split.reverse.join.strip }
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `map(&.to_s.split.reverse.join.strip)`
subject.catch(source).should_not be_valid CRYSTAL
end end
context "properties" do context "properties" do
it "#exclude_calls_with_block" 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 = VerboseBlock.new
rule rule.exclude_calls_with_block = true
.tap(&.exclude_calls_with_block = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
rule CRYSTAL
.tap(&.exclude_calls_with_block = false) rule.exclude_calls_with_block = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).in_groups_of(1) { |i| i.map(&.to_s) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `in_groups_of(1, &.map {...})`
CRYSTAL
end end
it "#exclude_multiple_line_blocks" do it "#exclude_multiple_line_blocks" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_multiple_line_blocks = true
expect_no_issues rule, <<-CRYSTAL
(1..3).any? do |i| (1..3).any? do |i|
i.odd? i.odd?
end end
) CRYSTAL
rule = VerboseBlock.new rule.exclude_multiple_line_blocks = false
rule expect_issue rule, <<-CRYSTAL
.tap(&.exclude_multiple_line_blocks = true) (1..3).any? do |i|
.catch(source).should be_valid # ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
rule i.odd?
.tap(&.exclude_multiple_line_blocks = false) end
.catch(source).should_not be_valid CRYSTAL
end end
it "#exclude_prefix_operators" do it "#exclude_prefix_operators" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_prefix_operators = true
expect_no_issues rule, <<-CRYSTAL
(1..3).sum { |i| +i } (1..3).sum { |i| +i }
(1..3).sum { |i| -i } (1..3).sum { |i| -i }
(1..3).sum { |i| ~i } (1..3).sum { |i| ~i }
) CRYSTAL
rule = VerboseBlock.new rule.exclude_prefix_operators = false
rule rule.exclude_operators = false
.tap(&.exclude_prefix_operators = true) expect_issue rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).sum { |i| +i }
rule # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)`
.tap(&.exclude_prefix_operators = false) (1..3).sum { |i| -i }
.tap(&.exclude_operators = false) # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.-)`
.catch(source).should_not be_valid (1..3).sum { |i| ~i }
# ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)`
CRYSTAL
end end
it "#exclude_operators" do it "#exclude_operators" do
source = Source.new %(
(1..3).sum { |i| i * 2 }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_operators = true
.tap(&.exclude_operators = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).sum { |i| i * 2 }
rule CRYSTAL
.tap(&.exclude_operators = false) rule.exclude_operators = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).sum { |i| i * 2 }
# ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))`
CRYSTAL
end end
it "#exclude_setters" do it "#exclude_setters" do
source = Source.new %(
Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_setters = true
.tap(&.exclude_setters = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
rule CRYSTAL
.tap(&.exclude_setters = false) rule.exclude_setters = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))`
CRYSTAL
end end
it "#max_line_length" do it "#max_line_length" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_multiple_line_blocks = false
rule.max_line_length = 60
expect_no_issues rule, <<-CRYSTAL
(1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
i.to_s.reverse.strip.blank? i.to_s.reverse.strip.blank?
end end
) CRYSTAL
rule = VerboseBlock.new rule.max_line_length = nil
.tap(&.exclude_multiple_line_blocks = false) expect_issue rule, <<-CRYSTAL
rule (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
.tap(&.max_line_length = 60) # ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)`
.catch(source).should be_valid i.to_s.reverse.strip.blank?
rule end
.tap(&.max_line_length = nil) CRYSTAL
.catch(source).should_not be_valid
end end
it "#max_length" do it "#max_length" do
source = Source.new %(
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.max_length = 30
.tap(&.max_length = 30) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
rule CRYSTAL
.tap(&.max_length = nil) rule.max_length = nil
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `tap(&.to_s.split.reverse.join.strip.blank?)`
CRYSTAL
end end
end end
context "macro" do context "macro" do
it "reports in macro scope" do it "reports in macro scope" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
{{ (1..3).any? { |i| i.odd? } }} {{ (1..3).any? { |i| i.odd? } }}
) # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
subject.catch(source).should_not be_valid CRYSTAL
end end
end end
it "reports call args and named_args" do 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 }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_operators = false
.tap(&.exclude_operators = false) expect_issue rule, <<-CRYSTAL
.catch(source).should_not be_valid (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? }
source.issues.size.should eq(short_block_variants.size) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[start: 0.to_i64, count: 3]?)`
(1..3).map { |i| i.to_s[0.to_i64, count: 3]? }
source.issues.each_with_index do |issue, i| # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[0.to_i64, count: 3]?)`
issue.message.should eq(VerboseBlock::MSG % short_block_variants[i]) (1..3).map { |i| i.to_s[0.to_i64, 3]? }
end # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[0.to_i64, 3]?)`
(1..3).map { |i| i.to_s[start: 0.to_i64, count: 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))`
(1..3).map { |i| i.to_s[0.to_i64, count: 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[0.to_i64, count: 3]=("foo"))`
(1..3).map { |i| i.to_s[0.to_i64, 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.[0.to_i64, 3]=("foo"))`
(1..3).map { |i| i.to_s.camelcase(lower: true) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.camelcase(lower: true))`
(1..3).map { |i| i.to_s.camelcase }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.camelcase)`
(1..3).map { |i| i.to_s.gsub('_', '-') }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.to_s.gsub('_', '-'))`
(1..3).map { |i| i.in?(*{1, 2, 3}, **{foo: :bar}) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.in?(*{1, 2, 3}, **{foo: :bar}))`
(1..3).map { |i| i.in?(1, *foo, 3, **bar) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `map(&.in?(1, *foo, 3, **bar))`
(1..3).join(separator: '.') { |i| i.to_s }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short [...] `join(separator: '.', &.to_s)`
CRYSTAL
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

View file

@ -169,7 +169,7 @@ module Ameba::Rule::Style
end end
# ameba:disable Metrics/CyclomaticComplexity # ameba:disable Metrics/CyclomaticComplexity
protected def issue_for_valid(source, call : Crystal::Call, body : Crystal::Call) protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
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)
@ -182,7 +182,7 @@ 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)
issue_for call.name_location, call.end_location, issue_for call.name_location, block.end_location,
MSG % call_code MSG % call_code
end end
@ -218,7 +218,7 @@ module Ameba::Rule::Style
return if reference_count(body, arg) > 1 return if reference_count(body, arg) > 1
# add issue if the given nodes pass all of the checks # add issue if the given nodes pass all of the checks
issue_for_valid source, node, body issue_for_valid source, node, block, body
end end
end end
end end