Autocorrect various rules (#253)

This commit is contained in:
fn ⌃ ⌥ 2021-11-16 13:30:33 -08:00 committed by GitHub
parent 255d10f921
commit 63a6c73dc0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
24 changed files with 676 additions and 175 deletions

View file

@ -345,5 +345,89 @@ module Ameba::AST
exp_code.should eq "{1, 2}" exp_code.should eq "{1, 2}"
end end
end end
describe "#name_end_location" do
it "works on method call" do
node = as_node("name(foo)").as Crystal::Call
subject.name_end_location(node).to_s.should eq ":1:4"
end
it "works on method definition" do
node = as_node("def name; end").as Crystal::Def
subject.name_end_location(node).to_s.should eq ":1:8"
end
it "works on macro definition" do
node = as_node("macro name; end").as Crystal::Macro
subject.name_end_location(node).to_s.should eq ":1:10"
end
it "works on class definition" do
node = as_node("class Name; end").as Crystal::ClassDef
subject.name_end_location(node).to_s.should eq ":1:10"
end
it "works on module definition" do
node = as_node("module Name; end").as Crystal::ModuleDef
subject.name_end_location(node).to_s.should eq ":1:11"
end
it "works on annotation definition" do
node = as_node("annotation Name; end").as Crystal::AnnotationDef
subject.name_end_location(node).to_s.should eq ":1:15"
end
it "works on enum definition" do
node = as_node("enum Name; end").as Crystal::EnumDef
subject.name_end_location(node).to_s.should eq ":1:9"
end
it "works on alias definition" do
node = as_node("alias Name = Foo").as Crystal::Alias
subject.name_end_location(node).to_s.should eq ":1:10"
end
it "works on generic" do
node = as_node("Name(Foo)").as Crystal::Generic
subject.name_end_location(node).to_s.should eq ":1:4"
end
it "works on include" do
node = as_node("include Name").as Crystal::Include
subject.name_end_location(node).to_s.should eq ":1:12"
end
it "works on extend" do
node = as_node("extend Name").as Crystal::Extend
subject.name_end_location(node).to_s.should eq ":1:11"
end
it "works on variable type declaration" do
node = as_node("name : Foo").as Crystal::TypeDeclaration
subject.name_end_location(node).to_s.should eq ":1:4"
end
it "works on uninitialized variable" do
node = as_node("name = uninitialized Foo").as Crystal::UninitializedVar
subject.name_end_location(node).to_s.should eq ":1:4"
end
it "works on lib definition" do
node = as_node("lib Name; end").as Crystal::LibDef
subject.name_end_location(node).to_s.should eq ":1:8"
end
it "works on lib type definition" do
node = as_node("lib Foo; type Name = Bar; end").as(Crystal::LibDef).body
node.class.should eq Crystal::TypeDef
subject.name_end_location(node).to_s.should eq ":1:18"
end
it "works on metaclass" do
node = as_node("foo : Name.class").as(Crystal::TypeDeclaration).declared_type
node.class.should eq Crystal::Metaclass
subject.name_end_location(node).to_s.should eq ":1:10"
end
end
end end
end end

View file

@ -0,0 +1,54 @@
require "../../spec_helper"
describe Crystal::Location do
subject = Crystal::Location.new(nil, 2, 3)
describe "#with" do
it "changes line number" do
subject.with(line_number: 1).to_s.should eq ":1:3"
end
it "changes column number" do
subject.with(column_number: 1).to_s.should eq ":2:1"
end
it "changes line and column numbers" do
subject.with(line_number: 1, column_number: 2).to_s.should eq ":1:2"
end
end
describe "#adjust" do
it "adjusts line number" do
subject.adjust(line_number: 1).to_s.should eq ":3:3"
end
it "adjusts column number" do
subject.adjust(column_number: 1).to_s.should eq ":2:4"
end
it "adjusts line and column numbers" do
subject.adjust(line_number: 1, column_number: 2).to_s.should eq ":3:5"
end
end
describe "#seek" do
it "adjusts column number if line offset is 1" do
subject.seek(Crystal::Location.new(nil, 1, 2)).to_s.should eq ":2:4"
end
it "adjusts line number and changes column number if line offset is greater than 1" do
subject.seek(Crystal::Location.new(nil, 2, 1)).to_s.should eq ":3:1"
end
it "adjusts line number and changes column number if line offset is less than 1" do
subject.seek(Crystal::Location.new(nil, 0, 1)).to_s.should eq ":1:1"
end
it "raises exception if filenames don't match" do
expect_raises(ArgumentError, "Mismatching filenames:\n source.cr\n source2.cr") do
location = Crystal::Location.new("source.cr", 1, 1)
location.seek(Crystal::Location.new("source2.cr", 1, 1))
end
end
end
end

View file

@ -5,13 +5,15 @@ module Ameba::Rule::Layout
describe TrailingWhitespace do describe TrailingWhitespace do
it "passes if all lines do not have trailing whitespace" do it "passes if all lines do not have trailing whitespace" do
source = Source.new "no-whispace" expect_no_issues subject, "no-whispace"
subject.catch(source).should be_valid
end end
it "fails if there is a line with trailing whitespace" do it "fails if there is a line with trailing whitespace" do
source = Source.new "whitespace at the end " source = expect_issue subject,
subject.catch(source).should_not be_valid "whitespace at the end \n" \
" # ^^ error: Trailing whitespace detected"
expect_correction source, "whitespace at the end"
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do
@ -21,7 +23,7 @@ module Ameba::Rule::Layout
issue = source.issues.first issue = source.issues.first
issue.rule.should_not be_nil issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:2:7" issue.location.to_s.should eq "source.cr:2:7"
issue.end_location.should be_nil issue.end_location.to_s.should eq "source.cr:2:7"
issue.message.should eq "Trailing whitespace detected" issue.message.should eq "Trailing whitespace detected"
end end
end end

View file

@ -5,7 +5,7 @@ module Ameba::Rule::Lint
describe ComparisonToBoolean do describe ComparisonToBoolean do
it "passes if there is no comparison to boolean" do it "passes if there is no comparison to boolean" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = true a = true
if a if a
@ -32,34 +32,67 @@ module Ameba::Rule::Lint
when false when false
:not_ok :not_ok
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
context "boolean on the right" do context "boolean on the right" do
it "fails if there is == comparison to boolean" do it "fails if there is == comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
if s.empty? == true if s.empty? == true
# ^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok :ok
end end
)
subject.catch(source).should_not be_valid if s.empty? == false
# ^^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok
end
CRYSTAL
expect_correction source, <<-CRYSTAL
if s.empty?
:ok
end
if !s.empty?
:ok
end
CRYSTAL
end end
it "fails if there is != comparison to boolean" do it "fails if there is != comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
if a != false if a != false
# ^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok :ok
end end
)
subject.catch(source).should_not be_valid if a != true
# ^^^^^^^^^ error: Comparison to a boolean is pointless
:ok
end
CRYSTAL
expect_correction source, <<-CRYSTAL
if a
:ok
end
if !a
:ok
end
CRYSTAL
end end
it "fails if there is case comparison to boolean" do it "fails if there is case comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
a === true a === true
) # ^^^^^^^^ error: Comparison to a boolean is pointless
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
a
CRYSTAL
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do
@ -75,28 +108,62 @@ module Ameba::Rule::Lint
context "boolean on the left" do context "boolean on the left" do
it "fails if there is == comparison to boolean" do it "fails if there is == comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
if true == s.empty? if true == s.empty?
# ^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok :ok
end end
)
subject.catch(source).should_not be_valid if false == s.empty?
# ^^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok
end
CRYSTAL
expect_correction source, <<-CRYSTAL
if s.empty?
:ok
end
if !s.empty?
:ok
end
CRYSTAL
end end
it "fails if there is != comparison to boolean" do it "fails if there is != comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
if false != a if false != a
# ^^^^^^^^^^ error: Comparison to a boolean is pointless
:ok :ok
end end
)
subject.catch(source).should_not be_valid if true != a
# ^^^^^^^^^ error: Comparison to a boolean is pointless
:ok
end
CRYSTAL
expect_correction source, <<-CRYSTAL
if a
:ok
end
if !a
:ok
end
CRYSTAL
end end
it "fails if there is case comparison to boolean" do it "fails if there is case comparison to boolean" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
true === a true === a
) # ^^^^^^^^ error: Comparison to a boolean is pointless
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
a
CRYSTAL
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

View file

@ -34,7 +34,7 @@ module Ameba::Rule::Metrics
issue = source.issues.first issue = source.issues.first
issue.rule.should eq subject issue.rule.should eq subject
issue.location.to_s.should eq "source.cr:1:5" issue.location.to_s.should eq "source.cr:1:5"
issue.end_location.to_s.should eq "source.cr:1:10" issue.end_location.to_s.should eq "source.cr:1:9"
issue.message.should eq "Cyclomatic complexity too high [8/5]" issue.message.should eq "Cyclomatic complexity too high [8/5]"
end end

View file

@ -5,35 +5,41 @@ module Ameba::Rule::Performance
describe AnyInsteadOfEmpty do describe AnyInsteadOfEmpty 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, 2, 3].any?(&.zero?) [1, 2, 3].any?(&.zero?)
[1, 2, 3].any?(String) [1, 2, 3].any?(String)
[1, 2, 3].any?(1..3) [1, 2, 3].any?(1..3)
[1, 2, 3].any? { |e| e > 1 } [1, 2, 3].any? { |e| e > 1 }
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is any? call without a block nor argument" do it "reports if there is any? call without a block nor argument" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
[1, 2, 3].any? [1, 2, 3].any?
) # ^^^^^^^^^^^^ error: Use `!{...}.empty?` instead of `{...}.any?`
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
![1, 2, 3].empty?
CRYSTAL
end end
it "does not report if source is a spec" do it "does not report if source is a spec" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL, "source_spec.cr"
[1, 2, 3].any? [1, 2, 3].any?
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
context "macro" do context "macro" do
it "reports in macro scope" do it "reports in macro scope" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
{{ [1, 2, 3].any? }} {{ [1, 2, 3].any? }}
) # ^^^^^^^^^^^^^^ error: Use `!{...}.empty?` instead of `{...}.any?`
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
{{ ![1, 2, 3].empty? }}
CRYSTAL
end end
end end
@ -45,8 +51,8 @@ module Ameba::Rule::Performance
issue = source.issues.first issue = source.issues.first
issue.rule.should_not be_nil issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:1:11" issue.location.to_s.should eq "source.cr:1:1"
issue.end_location.to_s.should eq "source.cr:1:15" issue.end_location.to_s.should eq "source.cr:1:14"
issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`" issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`"
end end
end end

View file

@ -5,46 +5,51 @@ module Ameba::Rule::Performance
describe ChainedCallWithNoBang do describe ChainedCallWithNoBang 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).select { |e| e > 1 }.sort! (1..3).select { |e| e > 1 }.sort!
(1..3).select { |e| e > 1 }.sort_by!(&.itself) (1..3).select { |e| e > 1 }.sort_by!(&.itself)
(1..3).select { |e| e > 1 }.uniq! (1..3).select { |e| e > 1 }.uniq!
(1..3).select { |e| e > 1 }.shuffle! (1..3).select { |e| e > 1 }.shuffle!
(1..3).select { |e| e > 1 }.reverse! (1..3).select { |e| e > 1 }.reverse!
(1..3).select { |e| e > 1 }.rotate! (1..3).select { |e| e > 1 }.rotate!
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is select followed by reverse" do it "reports if there is select followed by reverse" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
[1, 2, 3].select { |e| e > 1 }.reverse [1, 2, 3].select { |e| e > 1 }.reverse
) # ^^^^^^^ error: Use bang method variant `reverse!` after chained `select` call
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
[1, 2, 3].select { |e| e > 1 }.reverse!
CRYSTAL
end end
it "does not report if source is a spec" do it "does not report if source is a spec" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL, "source_spec.cr"
[1, 2, 3].select { |e| e > 1 }.reverse [1, 2, 3].select { |e| e > 1 }.reverse
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is select followed by reverse followed by other call" do it "reports if there is select followed by reverse followed by other call" do
source = Source.new %( source = expect_issue subject, <<-CRYSTAL
[1, 2, 3].select { |e| e > 2 }.reverse.size [1, 2, 3].select { |e| e > 2 }.reverse.size
) # ^^^^^^^ error: Use bang method variant `reverse!` after chained `select` call
subject.catch(source).should_not be_valid CRYSTAL
expect_correction source, <<-CRYSTAL
[1, 2, 3].select { |e| e > 2 }.reverse!.size
CRYSTAL
end end
context "properties" do context "properties" do
it "allows to configure `call_names`" do it "allows to configure `call_names`" do
source = Source.new %(
[1, 2, 3].select { |e| e > 2 }.reverse
)
rule = ChainedCallWithNoBang.new rule = ChainedCallWithNoBang.new
rule.call_names = %w(uniq) rule.call_names = %w(uniq)
rule.catch(source).should be_valid expect_no_issues rule, <<-CRYSTAL
[1, 2, 3].select { |e| e > 2 }.reverse
CRYSTAL
end end
end end
@ -59,17 +64,16 @@ module Ameba::Rule::Performance
issue = source.issues.first issue = source.issues.first
issue.rule.should_not be_nil issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:1:32" issue.location.to_s.should eq "source.cr:1:32"
issue.end_location.to_s.should eq "source.cr:1:39" issue.end_location.to_s.should eq "source.cr:1:38"
issue.message.should eq "Use bang method variant `reverse!` after chained `select` call" issue.message.should eq "Use bang method variant `reverse!` after chained `select` call"
end end
context "macro" do context "macro" do
it "doesn't report in macro scope" do it "doesn't report in macro scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
{{ [1, 2, 3].select { |e| e > 2 }.reverse }} {{ [1, 2, 3].select { |e| e > 2 }.reverse }}
) CRYSTAL
subject.catch(source).should be_valid
end end
end end
end end

View file

@ -83,10 +83,10 @@ module Ameba::Rule::Style
end end
it "fails if there is a redundant begin block" do it "fails if there is a redundant begin block" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method(a : String) : String def method(a : String) : String
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
begin begin
# ^^^^^ error: Redundant `begin` block detected
open_file open_file
do_some_stuff do_some_stuff
ensure ensure
@ -94,61 +94,115 @@ module Ameba::Rule::Style
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method(a : String) : String
#{trailing_whitespace}
open_file
do_some_stuff
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "fails if there is a redundant begin block in a method without args" do it "fails if there is a redundant begin block in a method without args" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
# ^^^^^ error: Redundant `begin` block detected
open_file open_file
ensure ensure
close_file close_file
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method
#{trailing_whitespace}
open_file
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "fails if there is a redundant block in a method with return type" do it "fails if there is a redundant block in a method with return type" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method : String def method : String
# ^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
begin begin
# ^^^^^ error: Redundant `begin` block detected
open_file open_file
ensure ensure
close_file close_file
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method : String
#{trailing_whitespace}
open_file
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "fails if there is a redundant block in a method with multiple args" do it "fails if there is a redundant block in a method with multiple args" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method(a : String, def method(a : String,
# ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
b : String) b : String)
begin begin
# ^^^^^ error: Redundant `begin` block detected
open_file open_file
ensure ensure
close_file close_file
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method(a : String,
b : String)
#{trailing_whitespace}
open_file
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "fails if there is a redundant block in a method with multiple args" do it "fails if there is a redundant block in a method with multiple args" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method(a : String, def method(a : String,
# ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
b : String b : String
) )
begin begin
# ^^^^^ error: Redundant `begin` block detected
open_file open_file
ensure ensure
close_file close_file
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method(a : String,
b : String
)
#{trailing_whitespace}
open_file
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "doesn't report if there is an inner redundant block" do it "doesn't report if there is an inner redundant block" do
@ -165,28 +219,47 @@ module Ameba::Rule::Style
end end
it "fails if there is a redundant block with yield" do it "fails if there is a redundant block with yield" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
# ^^^^^ error: Redundant `begin` block detected
yield yield
ensure ensure
close_file close_file
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method
#{trailing_whitespace}
yield
ensure
close_file
#{trailing_whitespace}
end
CRYSTAL
end end
it "fails if there is top level redundant block in a method" do it "fails if there is top level redundant block in a method" do
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
# ^^^^^ error: Redundant `begin` block detected
a = 1 a = 1
b = 2 b = 2
end end
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
def method
#{trailing_whitespace}
a = 1
b = 2
#{trailing_whitespace}
end
CRYSTAL
end end
it "doesn't report if begin-end block in a proc literal" do it "doesn't report if begin-end block in a proc literal" do
@ -215,8 +288,8 @@ module Ameba::Rule::Style
issue = s.issues.first issue = s.issues.first
issue.rule.should_not be_nil issue.rule.should_not be_nil
issue.location.to_s.should eq "source.cr:1:1" issue.location.to_s.should eq "source.cr:2:3"
issue.end_location.to_s.should eq "source.cr:7:3" issue.end_location.to_s.should eq "source.cr:2:7"
issue.message.should eq "Redundant `begin` block detected" issue.message.should eq "Redundant `begin` block detected"
end end
end end

View file

@ -29,24 +29,36 @@ module Ameba::Rule::Style
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
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
(1..3).any? { |i| i.odd? } (1..3).any? { |i| i.odd? }
# ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).any?(&.odd?)
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
expect_issue subject, <<-CRYSTAL source = 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)` # ^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `join('.', &.to_s)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).join('.', &.to_s)
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
expect_issue subject, <<-CRYSTAL source = 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)` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `map(&.to_s.split.reverse.join.strip)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).map(&.to_s.split.reverse.join.strip)
CRYSTAL
end end
context "properties" do context "properties" do
@ -57,9 +69,13 @@ module Ameba::Rule::Style
(1..3).in_groups_of(1) { |i| i.map(&.to_s) } (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
CRYSTAL CRYSTAL
rule.exclude_calls_with_block = false rule.exclude_calls_with_block = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).in_groups_of(1) { |i| i.map(&.to_s) } (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `in_groups_of(1, &.map {...})` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `in_groups_of(1, &.map(&.to_s))`
CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).in_groups_of(1, &.map(&.to_s))
CRYSTAL CRYSTAL
end end
@ -72,12 +88,16 @@ module Ameba::Rule::Style
end end
CRYSTAL CRYSTAL
rule.exclude_multiple_line_blocks = false rule.exclude_multiple_line_blocks = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).any? do |i| (1..3).any? do |i|
# ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` # ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
i.odd? i.odd?
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).any?(&.odd?)
CRYSTAL
end end
it "#exclude_prefix_operators" do it "#exclude_prefix_operators" do
@ -90,7 +110,7 @@ module Ameba::Rule::Style
CRYSTAL CRYSTAL
rule.exclude_prefix_operators = false rule.exclude_prefix_operators = false
rule.exclude_operators = false rule.exclude_operators = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).sum { |i| +i } (1..3).sum { |i| +i }
# ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)` # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)`
(1..3).sum { |i| -i } (1..3).sum { |i| -i }
@ -98,6 +118,12 @@ module Ameba::Rule::Style
(1..3).sum { |i| ~i } (1..3).sum { |i| ~i }
# ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)` # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).sum(&.+)
(1..3).sum(&.-)
(1..3).sum(&.~)
CRYSTAL
end end
it "#exclude_operators" do it "#exclude_operators" do
@ -107,10 +133,14 @@ module Ameba::Rule::Style
(1..3).sum { |i| i * 2 } (1..3).sum { |i| i * 2 }
CRYSTAL CRYSTAL
rule.exclude_operators = false rule.exclude_operators = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).sum { |i| i * 2 } (1..3).sum { |i| i * 2 }
# ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))` # ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).sum(&.*(2))
CRYSTAL
end end
it "#exclude_setters" do it "#exclude_setters" do
@ -120,10 +150,14 @@ module Ameba::Rule::Style
Char::Reader.new("abc").tap { |reader| reader.pos = 0 } Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
CRYSTAL CRYSTAL
rule.exclude_setters = false rule.exclude_setters = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
Char::Reader.new("abc").tap { |reader| reader.pos = 0 } Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
Char::Reader.new("abc").tap(&.pos=(0))
CRYSTAL
end end
it "#max_line_length" do it "#max_line_length" do
@ -136,12 +170,16 @@ module Ameba::Rule::Style
end end
CRYSTAL CRYSTAL
rule.max_line_length = nil rule.max_line_length = nil
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
# ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)` # ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)`
i.to_s.reverse.strip.blank? i.to_s.reverse.strip.blank?
end end
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap(&.to_s.reverse.strip.blank?)
CRYSTAL
end end
it "#max_length" do it "#max_length" do
@ -151,26 +189,34 @@ module Ameba::Rule::Style
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
CRYSTAL CRYSTAL
rule.max_length = nil rule.max_length = nil
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `tap(&.to_s.split.reverse.join.strip.blank?)` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `tap(&.to_s.split.reverse.join.strip.blank?)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).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
expect_issue subject, <<-CRYSTAL source = expect_issue subject, <<-CRYSTAL
{{ (1..3).any? { |i| i.odd? } }} {{ (1..3).any? { |i| i.odd? } }}
# ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
{{ (1..3).any?(&.odd?) }}
CRYSTAL
end end
end end
it "reports call args and named_args" do it "reports call args and named_args" do
rule = VerboseBlock.new rule = VerboseBlock.new
rule.exclude_operators = false rule.exclude_operators = false
expect_issue rule, <<-CRYSTAL source = expect_issue rule, <<-CRYSTAL
(1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? } (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 0.to_i64, count: 3]?)` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.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, count: 3]? }
@ -196,6 +242,21 @@ module Ameba::Rule::Style
(1..3).join(separator: '.') { |i| i.to_s } (1..3).join(separator: '.') { |i| i.to_s }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `join(separator: '.', &.to_s)` # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `join(separator: '.', &.to_s)`
CRYSTAL CRYSTAL
expect_correction source, <<-CRYSTAL
(1..3).map(&.to_s.[start: 0.to_i64, count: 3]?)
(1..3).map(&.to_s.[0.to_i64, count: 3]?)
(1..3).map(&.to_s.[0.to_i64, 3]?)
(1..3).map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))
(1..3).map(&.to_s.[0.to_i64, count: 3]=("foo"))
(1..3).map(&.to_s.[0.to_i64, 3]=("foo"))
(1..3).map(&.to_s.camelcase(lower: true))
(1..3).map(&.to_s.camelcase)
(1..3).map(&.to_s.gsub('_', '-'))
(1..3).map(&.in?(*{1, 2, 3}, **{foo: :bar}))
(1..3).map(&.in?(1, *foo, 3, **bar))
(1..3).join(separator: '.', &.to_s)
CRYSTAL
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

View file

@ -103,7 +103,7 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
return unless (name = node_source(node.name, source.lines)) return unless name = node_source(node.name, source.lines)
return unless name.includes?("A") return unless name.includes?("A")
issue_for(node.name, message: "A to AA") do |corrector| issue_for(node.name, message: "A to AA") do |corrector|
@ -120,7 +120,7 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
return unless (name = node_source(node.name, source.lines)) return unless name = node_source(node.name, source.lines)
return unless name.includes?("A") return unless name.includes?("A")
issue_for(node.name, message: "A to B") do |corrector| issue_for(node.name, message: "A to B") do |corrector|
@ -137,7 +137,7 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
return unless (name = node_source(node.name, source.lines)) return unless name = node_source(node.name, source.lines)
return unless name.includes?("B") return unless name.includes?("B")
issue_for(node.name, message: "B to A") do |corrector| issue_for(node.name, message: "B to A") do |corrector|
@ -154,7 +154,7 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
return unless (name = node_source(node.name, source.lines)) return unless name = node_source(node.name, source.lines)
return unless name.includes?("B") return unless name.includes?("B")
issue_for(node.name, message: "B to C") do |corrector| issue_for(node.name, message: "B to C") do |corrector|
@ -171,7 +171,7 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef | Crystal::ModuleDef) def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
return unless (name = node_source(node.name, source.lines)) return unless name = node_source(node.name, source.lines)
return unless name.includes?("C") return unless name.includes?("C")
issue_for(node.name, message: "C to A") do |corrector| issue_for(node.name, message: "C to A") do |corrector|
@ -188,13 +188,10 @@ module Ameba
end end
def test(source, node : Crystal::ClassDef) def test(source, node : Crystal::ClassDef)
return unless (location = node.location) return unless location = node.location
end_location = location.adjust(column_number: {{"class".size - 1}})
end_location = Crystal::Location.new(
location.filename,
location.line_number,
location.column_number + "class".size - 1
)
issue_for(location, end_location, message: "class to module") do |corrector| issue_for(location, end_location, message: "class to module") do |corrector|
corrector.replace(location, end_location, "module") corrector.replace(location, end_location, "module")
end end
@ -209,13 +206,10 @@ module Ameba
end end
def test(source, node : Crystal::ModuleDef) def test(source, node : Crystal::ModuleDef)
return unless (location = node.location) return unless location = node.location
end_location = location.adjust(column_number: {{"module".size - 1}})
end_location = Crystal::Location.new(
location.filename,
location.line_number,
location.column_number + "module".size - 1
)
issue_for(location, end_location, message: "module to class") do |corrector| issue_for(location, end_location, message: "module to class") do |corrector|
corrector.replace(location, end_location, "class") corrector.replace(location, end_location, "class")
end end

View file

@ -1,5 +1,6 @@
require "./ameba/*" require "./ameba/*"
require "./ameba/ast/**" require "./ameba/ast/**"
require "./ameba/ext/**"
require "./ameba/rule/**" require "./ameba/rule/**"
require "./ameba/formatter/*" require "./ameba/formatter/*"
require "./ameba/source/**" require "./ameba/source/**"

View file

@ -170,4 +170,44 @@ module Ameba::AST::Util
"{#{source_between(exp_start, exp_end, code_lines)}}" "{#{source_between(exp_start, exp_end, code_lines)}}"
end end
# Returns `nil` if *node* does not contain a name.
def name_location(node)
if loc = node.name_location
return loc
end
return node.var.location if node.is_a?(Crystal::TypeDeclaration) ||
node.is_a?(Crystal::UninitializedVar)
return unless node.responds_to?(:name) && (name = node.name)
return unless name.is_a?(Crystal::ASTNode)
name.location
end
# Returns zero if *node* does not contain a name.
def name_size(node)
unless (size = node.name_size).zero?
return size
end
return 0 unless node.responds_to?(:name) && (name = node.name)
case name
when Crystal::ASTNode then name.name_size
when Symbol then name.to_s.size # Crystal::MagicConstant
else name.size
end
end
# Returns `nil` if *node* does not contain a name.
#
# NOTE: Use this instead of `Crystal::Call#name_end_location` to avoid an
# off-by-one error.
def name_end_location(node)
return unless loc = name_location(node)
return if (size = name_size(node)).zero?
loc.adjust(column_number: size - 1)
end
end end

35
src/ameba/ext/location.cr Normal file
View file

@ -0,0 +1,35 @@
# Extensions to Crystal::Location
module Ameba::Ext::Location
# Returns the same location as this location but with the line and/or column number(s) changed
# to the given value(s).
def with(line_number = @line_number, column_number = @column_number) : self
self.class.new(@filename, line_number, column_number)
end
# Returns the same location as this location but with the line and/or column number(s) adjusted
# by the given amount(s).
def adjust(line_number = 0, column_number = 0) : self
self.class.new(@filename, @line_number + line_number, @column_number + column_number)
end
# Seeks to a given *offset* relative to `self`.
def seek(offset : self) : self
if offset.filename.as?(String).presence && @filename != offset.filename
raise ArgumentError.new <<-MSG
Mismatching filenames:
#{@filename}
#{offset.filename}
MSG
end
if offset.line_number == 1
self.class.new(@filename, @line_number, @column_number + offset.column_number - 1)
else
self.class.new(@filename, @line_number + offset.line_number - 1, offset.column_number)
end
end
end
class Crystal::Location
include Ameba::Ext::Location
end

View file

@ -16,7 +16,14 @@ module Ameba::Rule::Layout
def test(source) def test(source)
source.lines.each_with_index do |line, index| source.lines.each_with_index do |line, index|
issue_for({index + 1, line.size}, MSG) if line =~ /\s$/ next unless ws_index = line =~ /\s+$/
location = {index + 1, ws_index + 1}
end_location = {index + 1, line.size}
issue_for location, end_location, MSG do |corrector|
corrector.remove(location, end_location)
end
end end
end end
end end

View file

@ -20,6 +20,8 @@ module Ameba::Rule::Lint
# Enabled: true # Enabled: true
# ``` # ```
class ComparisonToBoolean < Base class ComparisonToBoolean < Base
include AST::Util
properties do properties do
enabled false enabled false
description "Disallows comparison to booleans" description "Disallows comparison to booleans"
@ -29,11 +31,31 @@ module Ameba::Rule::Lint
OP_NAMES = %w(== != ===) OP_NAMES = %w(== != ===)
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)
comparison = node.name.in?(OP_NAMES) return unless node.name.in?(OP_NAMES)
to_boolean = node.args.first?.try(&.is_a?(Crystal::BoolLiteral)) || return unless node.args.size == 1
node.obj.is_a?(Crystal::BoolLiteral)
issue_for node, MSG if comparison && to_boolean arg, obj = node.args.first, node.obj
case
when arg.is_a?(Crystal::BoolLiteral)
bool, exp = arg, obj
when obj.is_a?(Crystal::BoolLiteral)
bool, exp = obj, arg
end
return unless bool && exp
return unless exp_code = node_source(exp, source.lines)
not =
case node.name
when "==", "===" then !bool.value # foo == false
when "!=" then bool.value # foo != true
end
exp_code = "!#{exp_code}" if not
issue_for node, MSG do |corrector|
corrector.replace(node, exp_code)
end
end end
end end
end end

View file

@ -9,6 +9,8 @@ module Ameba::Rule::Metrics
# MaxComplexity: 10 # MaxComplexity: 10
# ``` # ```
class CyclomaticComplexity < Base class CyclomaticComplexity < Base
include AST::Util
properties do properties do
description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`" description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`"
max_complexity 10 max_complexity 10
@ -20,18 +22,9 @@ module Ameba::Rule::Metrics
complexity = AST::CountingVisitor.new(node).count complexity = AST::CountingVisitor.new(node).count
if complexity > max_complexity && (location = node.name_location) if complexity > max_complexity && (location = node.name_location)
issue_for location, def_name_end_location(node), issue_for location, name_end_location(node),
MSG % {complexity, max_complexity} MSG % {complexity, max_complexity}
end end
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
Crystal::Location.new(location.filename, line_number, column_number + node.name.size)
end
end end
end end

View file

@ -28,6 +28,8 @@ module Ameba::Rule::Performance
# Enabled: true # Enabled: true
# ``` # ```
class AnyInsteadOfEmpty < Base class AnyInsteadOfEmpty < Base
include AST::Util
properties do properties do
description "Identifies usage of arg-less `any?` calls." description "Identifies usage of arg-less `any?` calls."
end end
@ -39,8 +41,14 @@ 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 name_location = node.name_location
return unless end_location = name_end_location(node)
issue_for node.name_location, node.name_end_location, MSG issue_for location, end_location, MSG do |corrector|
corrector.insert_before(location, '!')
corrector.replace(name_location, end_location, "empty?")
end
end end
end end
end end

View file

@ -37,6 +37,8 @@ module Ameba::Rule::Performance
# - reverse # - reverse
# ``` # ```
class ChainedCallWithNoBang < Base class ChainedCallWithNoBang < Base
include AST::Util
properties do properties do
description "Identifies usage of chained calls not utilizing the bang method variants." description "Identifies usage of chained calls not utilizing the bang method variants."
@ -66,12 +68,15 @@ 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)
issue_for node.name_location, node.name_end_location, issue_for location, end_location, MSG % {node.name, obj.name} do |corrector|
MSG % {node.name, obj.name} corrector.insert_after(end_location, '!')
end
end end
end end
end end

View file

@ -72,11 +72,7 @@ module Ameba::Rule::Style
end_location = node.end_location end_location = node.end_location
if !end_location || end_location.try(&.column_number.zero?) if !end_location || end_location.try(&.column_number.zero?)
if end_location = path.end_location if end_location = path.end_location
end_location = Crystal::Location.new( end_location = end_location.adjust(column_number: 1)
end_location.filename,
end_location.line_number,
end_location.column_number + 1
)
end end
end end

View file

@ -43,11 +43,7 @@ module Ameba::Rule::Style
if allowed?(*parsed) && (expected = underscored *parsed) != token.raw if allowed?(*parsed) && (expected = underscored *parsed) != token.raw
location = token.location location = token.location
end_location = Crystal::Location.new( end_location = location.adjust(column_number: token.raw.size - 1)
location.filename,
location.line_number,
location.column_number + token.raw.size - 1
)
issue_for location, end_location, MSG % expected do |corrector| issue_for location, end_location, MSG % expected do |corrector|
corrector.replace(location, end_location, expected) corrector.replace(location, end_location, expected)
end end

View file

@ -38,6 +38,8 @@ module Ameba::Rule::Style
# Enabled: true # Enabled: true
# ``` # ```
class MethodNames < Base class MethodNames < Base
include AST::Util
properties do properties do
description "Enforces method names to be in underscored case" description "Enforces method names to be in underscored case"
end end
@ -46,17 +48,10 @@ 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 end_location = name_end_location(node)
line_number = node.location.try &.line_number issue_for location, end_location, MSG % {expected, node.name}
column_number = node.name_location.try &.column_number
return unless line_number && column_number
issue_for(
{line_number, column_number},
{line_number, column_number + node.name.size - 1},
MSG % {expected, node.name}
)
end end
end end
end end

View file

@ -65,15 +65,27 @@ module Ameba::Rule::Style
MSG = "Redundant `begin` block detected" MSG = "Redundant `begin` block detected"
def test(source, node : Crystal::Def) def test(source, node : Crystal::Def)
issue_for node, MSG if redundant_begin?(source, node) return unless def_loc = node.location
end
private def redundant_begin?(source, node)
case body = node.body case body = node.body
when Crystal::ExceptionHandler when Crystal::ExceptionHandler
redundant_begin_in_handler?(source, body, node) return if begin_exprs_in_handler?(body) || inner_handler?(body)
when Crystal::Expressions when Crystal::Expressions
redundant_begin_in_expressions?(body) return unless redundant_begin_in_expressions?(body)
else
return
end
return unless begin_range = def_redundant_begin_range(source, node)
begin_loc, end_loc = begin_range
begin_loc, end_loc = def_loc.seek(begin_loc), def_loc.seek(end_loc)
begin_end_loc = begin_loc.adjust(column_number: {{"begin".size - 1}})
end_end_loc = end_loc.adjust(column_number: {{"end".size - 1}})
issue_for begin_loc, begin_end_loc, MSG do |corrector|
corrector.remove(begin_loc, begin_end_loc)
corrector.remove(end_loc, end_end_loc)
end end
end end
@ -81,27 +93,27 @@ module Ameba::Rule::Style
node.keyword == :begin node.keyword == :begin
end end
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)
def_redundant_begin? code if code
rescue
false
end
private def inner_handler?(handler) private def inner_handler?(handler)
handler.body.is_a?(Crystal::ExceptionHandler) handler.body.is_a?(Crystal::ExceptionHandler)
end end
private def begin_exprs_in_handler?(handler) private def begin_exprs_in_handler?(handler)
if (body = handler.body).is_a?(Crystal::Expressions) if (body = handler.body).is_a?(Crystal::Expressions)
body.expressions.first.is_a?(Crystal::ExceptionHandler) body.expressions.first?.is_a?(Crystal::ExceptionHandler)
end end
end end
private def def_redundant_begin?(code) private def def_redundant_begin_range(source, node)
return unless code = node_source(node, source.lines)
lexer = Crystal::Lexer.new code lexer = Crystal::Lexer.new code
return unless begin_loc = def_redundant_begin_loc(lexer)
return unless end_loc = def_redundant_end_loc(lexer)
{begin_loc, end_loc}
end
private def def_redundant_begin_loc(lexer)
in_body = in_argument_list = false in_body = in_argument_list = false
loop do loop do
@ -111,7 +123,9 @@ module Ameba::Rule::Style
when :EOF, :"->" when :EOF, :"->"
break break
when :IDENT when :IDENT
return token.value == :begin if in_body next unless in_body
return unless token.value == :begin
return token.location
when :"(" when :"("
in_argument_list = true in_argument_list = true
when :")" when :")"
@ -121,9 +135,21 @@ module Ameba::Rule::Style
when :SPACE when :SPACE
# ignore # ignore
else else
return false if in_body return if in_body
end end
end end
end end
private def def_redundant_end_loc(lexer)
end_loc = def_end_loc = nil
while (token = lexer.next_token).type != :EOF
next unless token.value == :end
end_loc, def_end_loc = def_end_loc, token.location
end
end_loc
end
end end
end end

View file

@ -28,6 +28,8 @@ module Ameba::Rule::Style
# MaxLength: 50 # use ~ to disable # MaxLength: 50 # use ~ to disable
# ``` # ```
class VerboseBlock < Base class VerboseBlock < Base
include AST::Util
properties do properties do
description "Identifies usage of collapsible single expression blocks." description "Identifies usage of collapsible single expression blocks."
@ -108,20 +110,27 @@ module Ameba::Rule::Style
i i
end end
protected def args_to_s(io : IO, node : Crystal::Call, skip_last_arg = false) protected def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false)
node.args.dup.tap do |args| node.args.dup.tap do |args|
args.pop? if skip_last_arg args.pop? if skip_last_arg
args.join io, ", " args.join io, ", "
node.named_args.try do |named_args|
named_args = node.named_args
if named_args
io << ", " unless args.empty? || named_args.empty? io << ", " unless args.empty? || named_args.empty?
named_args.join io, ", " do |arg, inner_io| named_args.join io, ", " do |arg, inner_io|
inner_io << arg.name << ": " << arg.value inner_io << arg.name << ": " << arg.value
end end
end end
if short_block
io << ", " unless args.empty? && (named_args.nil? || named_args.empty?)
io << short_block
end
end end
end end
protected def node_to_s(node : Crystal::Call) protected def node_to_s(source, node : Crystal::Call)
String.build do |str| String.build do |str|
case name = node.name case name = node.name
when "[]" when "[]"
@ -137,29 +146,41 @@ module Ameba::Rule::Style
args_to_s(str, node, skip_last_arg: true) args_to_s(str, node, skip_last_arg: true)
str << "]=(" << node.args.last? << ')' str << "]=(" << node.args.last? << ')'
else else
short_block = short_block_code(source, node)
str << name str << name
if !node.args.empty? || (node.named_args && !node.named_args.try(&.empty?)) if !node.args.empty? || (node.named_args && !node.named_args.try(&.empty?)) || short_block
str << '(' str << '('
args_to_s(str, node) args_to_s(str, node, short_block)
str << ')' str << ')'
end end
str << " {...}" if node.block str << " {...}" if node.block && short_block.nil?
end end
end end
end end
protected def call_code(call, body) protected def short_block_code(source, node : Crystal::Call)
return unless block = node.block
return unless block_location = block.location
return unless block_end_location = block.body.end_location
block_code = source_between(block_location, block_end_location, source.lines)
return unless block_code.try(&.starts_with?("&."))
block_code
end
protected def call_code(source, call, body)
args = String.build { |io| args_to_s(io, call) }.presence args = String.build { |io| args_to_s(io, call) }.presence
args += ", " if args args += ", " if args
call_chain = %w[].tap do |arr| call_chain = %w[].tap do |arr|
obj = body.obj obj = body.obj
while obj.is_a?(Crystal::Call) while obj.is_a?(Crystal::Call)
arr << node_to_s(obj) arr << node_to_s(source, obj)
obj = obj.obj obj = obj.obj
end end
arr.reverse! arr.reverse!
arr << node_to_s(body) arr << node_to_s(source, body)
end end
name = name =
@ -170,6 +191,8 @@ 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)
@ -177,13 +200,18 @@ module Ameba::Rule::Style
return if exclude_setters && setter?(body.name) return if exclude_setters && setter?(body.name)
call_code = call_code =
call_code(call, body) call_code(source, call, body)
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, block.end_location, if call_code.includes?("{...}")
MSG % call_code issue_for location, end_location, MSG % call_code
else
issue_for location, end_location, MSG % call_code do |corrector|
corrector.replace(location, end_location, call_code)
end
end
end end
def test(source, node : Crystal::Call) def test(source, node : Crystal::Call)

View file

@ -14,5 +14,9 @@ module Ameba
end end
end end
def trailing_whitespace
' '
end
include Ameba::Spec::BeValid include Ameba::Spec::BeValid
include Ameba::Spec::ExpectIssue include Ameba::Spec::ExpectIssue