From 63a6c73dc0549d5091016dcc1aa74d4d747269f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?= <70830482+FnControlOption@users.noreply.github.com> Date: Tue, 16 Nov 2021 13:30:33 -0800 Subject: [PATCH] Autocorrect various rules (#253) --- spec/ameba/ast/util_spec.cr | 84 ++++++++++++++ spec/ameba/ext/location_spec.cr | 54 +++++++++ .../rule/layout/trailing_whitespace_spec.cr | 12 +- .../rule/lint/comparison_to_boolean_spec.cr | 109 ++++++++++++++---- .../metrics/cyclomatic_complexity_spec.cr | 2 +- .../performance/any_instead_of_empty_spec.cr | 34 +++--- .../chained_call_with_no_bang_spec.cr | 44 +++---- spec/ameba/rule/style/redundant_begin_spec.cr | 105 ++++++++++++++--- spec/ameba/rule/style/verbose_block_spec.cr | 87 +++++++++++--- spec/spec_helper.cr | 28 ++--- src/ameba.cr | 1 + src/ameba/ast/util.cr | 40 +++++++ src/ameba/ext/location.cr | 35 ++++++ src/ameba/rule/layout/trailing_whitespace.cr | 9 +- src/ameba/rule/lint/comparison_to_boolean.cr | 30 ++++- .../rule/metrics/cyclomatic_complexity.cr | 13 +-- .../rule/performance/any_instead_of_empty.cr | 10 +- .../performance/chained_call_with_no_bang.cr | 9 +- src/ameba/rule/style/is_a_filter.cr | 6 +- src/ameba/rule/style/large_numbers.cr | 6 +- src/ameba/rule/style/method_names.cr | 15 +-- src/ameba/rule/style/redundant_begin.cr | 62 +++++++--- src/ameba/rule/style/verbose_block.cr | 52 +++++++-- src/ameba/spec/support.cr | 4 + 24 files changed, 676 insertions(+), 175 deletions(-) create mode 100644 spec/ameba/ext/location_spec.cr create mode 100644 src/ameba/ext/location.cr diff --git a/spec/ameba/ast/util_spec.cr b/spec/ameba/ast/util_spec.cr index be47dd70..9132eeeb 100644 --- a/spec/ameba/ast/util_spec.cr +++ b/spec/ameba/ast/util_spec.cr @@ -345,5 +345,89 @@ module Ameba::AST exp_code.should eq "{1, 2}" 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 diff --git a/spec/ameba/ext/location_spec.cr b/spec/ameba/ext/location_spec.cr new file mode 100644 index 00000000..3de5fab1 --- /dev/null +++ b/spec/ameba/ext/location_spec.cr @@ -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 diff --git a/spec/ameba/rule/layout/trailing_whitespace_spec.cr b/spec/ameba/rule/layout/trailing_whitespace_spec.cr index 59c95e25..b6d9902d 100644 --- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr @@ -5,13 +5,15 @@ module Ameba::Rule::Layout describe TrailingWhitespace do it "passes if all lines do not have trailing whitespace" do - source = Source.new "no-whispace" - subject.catch(source).should be_valid + expect_no_issues subject, "no-whispace" end it "fails if there is a line with trailing whitespace" do - source = Source.new "whitespace at the end " - subject.catch(source).should_not be_valid + source = expect_issue subject, + "whitespace at the end \n" \ + " # ^^ error: Trailing whitespace detected" + + expect_correction source, "whitespace at the end" end it "reports rule, pos and message" do @@ -21,7 +23,7 @@ module Ameba::Rule::Layout issue = source.issues.first issue.rule.should_not be_nil 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" end end diff --git a/spec/ameba/rule/lint/comparison_to_boolean_spec.cr b/spec/ameba/rule/lint/comparison_to_boolean_spec.cr index da737643..e72278d8 100644 --- a/spec/ameba/rule/lint/comparison_to_boolean_spec.cr +++ b/spec/ameba/rule/lint/comparison_to_boolean_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Lint describe ComparisonToBoolean do it "passes if there is no comparison to boolean" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = true if a @@ -32,34 +32,67 @@ module Ameba::Rule::Lint when false :not_ok end - ) - subject.catch(source).should be_valid + CRYSTAL end context "boolean on the right" do it "fails if there is == comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL if s.empty? == true + # ^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless :ok 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 it "fails if there is != comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL if a != false + # ^^^^^^^^^^ error: Comparison to a boolean is pointless :ok 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 it "fails if there is case comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL a === true - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^ error: Comparison to a boolean is pointless + CRYSTAL + + expect_correction source, <<-CRYSTAL + a + CRYSTAL end it "reports rule, pos and message" do @@ -75,28 +108,62 @@ module Ameba::Rule::Lint context "boolean on the left" do it "fails if there is == comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL if true == s.empty? + # ^^^^^^^^^^^^^^^^ error: Comparison to a boolean is pointless :ok 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 it "fails if there is != comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL if false != a + # ^^^^^^^^^^ error: Comparison to a boolean is pointless :ok 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 it "fails if there is case comparison to boolean" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL true === a - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^ error: Comparison to a boolean is pointless + CRYSTAL + + expect_correction source, <<-CRYSTAL + a + CRYSTAL end it "reports rule, pos and message" do diff --git a/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr b/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr index bc81fc7d..eb4f5519 100644 --- a/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr +++ b/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr @@ -34,7 +34,7 @@ module Ameba::Rule::Metrics issue = source.issues.first issue.rule.should eq subject 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]" end diff --git a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr index 8606e309..e598a327 100644 --- a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr +++ b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr @@ -5,35 +5,41 @@ module Ameba::Rule::Performance describe AnyInsteadOfEmpty 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?(String) [1, 2, 3].any?(1..3) [1, 2, 3].any? { |e| e > 1 } - ) - subject.catch(source).should be_valid + CRYSTAL end 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? - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^ error: Use `!{...}.empty?` instead of `{...}.any?` + CRYSTAL + + expect_correction source, <<-CRYSTAL + ![1, 2, 3].empty? + CRYSTAL end 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? - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end context "macro" do it "reports in macro scope" do - source = Source.new %( + source = expect_issue subject, <<-CRYSTAL {{ [1, 2, 3].any? }} - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^ error: Use `!{...}.empty?` instead of `{...}.any?` + CRYSTAL + + expect_correction source, <<-CRYSTAL + {{ ![1, 2, 3].empty? }} + CRYSTAL end end @@ -45,8 +51,8 @@ module Ameba::Rule::Performance issue = source.issues.first issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:15" + issue.location.to_s.should eq "source.cr:1:1" + issue.end_location.to_s.should eq "source.cr:1:14" issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`" end end diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr index e5a31162..ad3cda81 100644 --- a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr +++ b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr @@ -5,46 +5,51 @@ module Ameba::Rule::Performance describe ChainedCallWithNoBang 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_by!(&.itself) (1..3).select { |e| e > 1 }.uniq! (1..3).select { |e| e > 1 }.shuffle! (1..3).select { |e| e > 1 }.reverse! (1..3).select { |e| e > 1 }.rotate! - ) - subject.catch(source).should be_valid + CRYSTAL end 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 - ) - subject.catch(source).should_not be_valid + # ^^^^^^^ error: Use bang method variant `reverse!` after chained `select` call + CRYSTAL + + expect_correction source, <<-CRYSTAL + [1, 2, 3].select { |e| e > 1 }.reverse! + CRYSTAL end 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 - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end 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 - ) - subject.catch(source).should_not be_valid + # ^^^^^^^ error: Use bang method variant `reverse!` after chained `select` call + CRYSTAL + + expect_correction source, <<-CRYSTAL + [1, 2, 3].select { |e| e > 2 }.reverse!.size + CRYSTAL end context "properties" do it "allows to configure `call_names`" do - source = Source.new %( - [1, 2, 3].select { |e| e > 2 }.reverse - ) rule = ChainedCallWithNoBang.new 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 @@ -59,17 +64,16 @@ module Ameba::Rule::Performance issue = source.issues.first issue.rule.should_not be_nil 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" end context "macro" 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 }} - ) - subject.catch(source).should be_valid + CRYSTAL end end end diff --git a/spec/ameba/rule/style/redundant_begin_spec.cr b/spec/ameba/rule/style/redundant_begin_spec.cr index 7fe188b8..80aae590 100644 --- a/spec/ameba/rule/style/redundant_begin_spec.cr +++ b/spec/ameba/rule/style/redundant_begin_spec.cr @@ -83,10 +83,10 @@ module Ameba::Rule::Style end it "fails if there is a redundant begin block" do - expect_issue subject, <<-CRYSTAL + source = expect_issue subject, <<-CRYSTAL def method(a : String) : String - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected begin + # ^^^^^ error: Redundant `begin` block detected open_file do_some_stuff ensure @@ -94,61 +94,115 @@ module Ameba::Rule::Style end end 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 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 - # ^^^^^^^^ error: Redundant `begin` block detected begin + # ^^^^^ error: Redundant `begin` block detected open_file ensure close_file end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method + #{trailing_whitespace} + open_file + ensure + close_file + #{trailing_whitespace} + end + CRYSTAL end 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 - # ^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected begin + # ^^^^^ error: Redundant `begin` block detected open_file ensure close_file end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method : String + #{trailing_whitespace} + open_file + ensure + close_file + #{trailing_whitespace} + end + CRYSTAL end 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, - # ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected b : String) begin + # ^^^^^ error: Redundant `begin` block detected open_file ensure close_file end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a : String, + b : String) + #{trailing_whitespace} + open_file + ensure + close_file + #{trailing_whitespace} + end + CRYSTAL end 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, - # ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected b : String ) begin + # ^^^^^ error: Redundant `begin` block detected open_file ensure close_file end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a : String, + b : String + ) + #{trailing_whitespace} + open_file + ensure + close_file + #{trailing_whitespace} + end + CRYSTAL end it "doesn't report if there is an inner redundant block" do @@ -165,28 +219,47 @@ module Ameba::Rule::Style end it "fails if there is a redundant block with yield" do - expect_issue subject, <<-CRYSTAL + source = expect_issue subject, <<-CRYSTAL def method - # ^^^^^^^^ error: Redundant `begin` block detected begin + # ^^^^^ error: Redundant `begin` block detected yield ensure close_file end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method + #{trailing_whitespace} + yield + ensure + close_file + #{trailing_whitespace} + end + CRYSTAL end it "fails if there is top level redundant block in a method" do - expect_issue subject, <<-CRYSTAL + source = expect_issue subject, <<-CRYSTAL def method - # ^^^^^^^^ error: Redundant `begin` block detected begin + # ^^^^^ error: Redundant `begin` block detected a = 1 b = 2 end end CRYSTAL + + expect_correction source, <<-CRYSTAL + def method + #{trailing_whitespace} + a = 1 + b = 2 + #{trailing_whitespace} + end + CRYSTAL end 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.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:7:3" + issue.location.to_s.should eq "source.cr:2:3" + issue.end_location.to_s.should eq "source.cr:2:7" issue.message.should eq "Redundant `begin` block detected" end end diff --git a/spec/ameba/rule/style/verbose_block_spec.cr b/spec/ameba/rule/style/verbose_block_spec.cr index c2280d80..6d86e4b0 100644 --- a/spec/ameba/rule/style/verbose_block_spec.cr +++ b/spec/ameba/rule/style/verbose_block_spec.cr @@ -29,24 +29,36 @@ module Ameba::Rule::Style end 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? } # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).any?(&.odd?) + CRYSTAL end 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 } # ^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `join('.', &.to_s)` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).join('.', &.to_s) + CRYSTAL end 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 } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `map(&.to_s.split.reverse.join.strip)` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).map(&.to_s.split.reverse.join.strip) + CRYSTAL end context "properties" do @@ -57,9 +69,13 @@ module Ameba::Rule::Style (1..3).in_groups_of(1) { |i| i.map(&.to_s) } CRYSTAL 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) } - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 end @@ -72,12 +88,16 @@ module Ameba::Rule::Style end CRYSTAL rule.exclude_multiple_line_blocks = false - expect_issue rule, <<-CRYSTAL + source = expect_issue rule, <<-CRYSTAL (1..3).any? do |i| # ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` i.odd? end CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).any?(&.odd?) + CRYSTAL end it "#exclude_prefix_operators" do @@ -90,7 +110,7 @@ module Ameba::Rule::Style CRYSTAL rule.exclude_prefix_operators = false rule.exclude_operators = false - expect_issue rule, <<-CRYSTAL + source = expect_issue rule, <<-CRYSTAL (1..3).sum { |i| +i } # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)` (1..3).sum { |i| -i } @@ -98,6 +118,12 @@ module Ameba::Rule::Style (1..3).sum { |i| ~i } # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).sum(&.+) + (1..3).sum(&.-) + (1..3).sum(&.~) + CRYSTAL end it "#exclude_operators" do @@ -107,10 +133,14 @@ module Ameba::Rule::Style (1..3).sum { |i| i * 2 } CRYSTAL rule.exclude_operators = false - expect_issue rule, <<-CRYSTAL + source = expect_issue rule, <<-CRYSTAL (1..3).sum { |i| i * 2 } # ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).sum(&.*(2)) + CRYSTAL end it "#exclude_setters" do @@ -120,10 +150,14 @@ module Ameba::Rule::Style Char::Reader.new("abc").tap { |reader| reader.pos = 0 } CRYSTAL rule.exclude_setters = false - expect_issue rule, <<-CRYSTAL + source = expect_issue rule, <<-CRYSTAL Char::Reader.new("abc").tap { |reader| reader.pos = 0 } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))` CRYSTAL + + expect_correction source, <<-CRYSTAL + Char::Reader.new("abc").tap(&.pos=(0)) + CRYSTAL end it "#max_line_length" do @@ -136,12 +170,16 @@ module Ameba::Rule::Style end CRYSTAL 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| # ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)` i.to_s.reverse.strip.blank? end CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap(&.to_s.reverse.strip.blank?) + CRYSTAL end it "#max_length" do @@ -151,26 +189,34 @@ module Ameba::Rule::Style (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } CRYSTAL 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? } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `tap(&.to_s.split.reverse.join.strip.blank?)` CRYSTAL + + expect_correction source, <<-CRYSTAL + (1..3).tap(&.to_s.split.reverse.join.strip.blank?) + CRYSTAL end end context "macro" do it "reports in macro scope" do - expect_issue subject, <<-CRYSTAL + source = expect_issue subject, <<-CRYSTAL {{ (1..3).any? { |i| i.odd? } }} # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` CRYSTAL + + expect_correction source, <<-CRYSTAL + {{ (1..3).any?(&.odd?) }} + CRYSTAL end end it "reports call args and named_args" do rule = VerboseBlock.new 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]? } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 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 } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `join(separator: '.', &.to_s)` 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 it "reports rule, pos and message" do diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index aa489fca..9020b9b3 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -103,7 +103,7 @@ module Ameba end 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") issue_for(node.name, message: "A to AA") do |corrector| @@ -120,7 +120,7 @@ module Ameba end 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") issue_for(node.name, message: "A to B") do |corrector| @@ -137,7 +137,7 @@ module Ameba end 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") issue_for(node.name, message: "B to A") do |corrector| @@ -154,7 +154,7 @@ module Ameba end 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") issue_for(node.name, message: "B to C") do |corrector| @@ -171,7 +171,7 @@ module Ameba end 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") issue_for(node.name, message: "C to A") do |corrector| @@ -188,13 +188,10 @@ module Ameba end 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| corrector.replace(location, end_location, "module") end @@ -209,13 +206,10 @@ module Ameba end 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| corrector.replace(location, end_location, "class") end diff --git a/src/ameba.cr b/src/ameba.cr index 2a34d0ef..d2eeffc1 100644 --- a/src/ameba.cr +++ b/src/ameba.cr @@ -1,5 +1,6 @@ require "./ameba/*" require "./ameba/ast/**" +require "./ameba/ext/**" require "./ameba/rule/**" require "./ameba/formatter/*" require "./ameba/source/**" diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 2965247f..dd577f30 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -170,4 +170,44 @@ module Ameba::AST::Util "{#{source_between(exp_start, exp_end, code_lines)}}" 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 diff --git a/src/ameba/ext/location.cr b/src/ameba/ext/location.cr new file mode 100644 index 00000000..88404b8d --- /dev/null +++ b/src/ameba/ext/location.cr @@ -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 diff --git a/src/ameba/rule/layout/trailing_whitespace.cr b/src/ameba/rule/layout/trailing_whitespace.cr index a80a56ec..b579e096 100644 --- a/src/ameba/rule/layout/trailing_whitespace.cr +++ b/src/ameba/rule/layout/trailing_whitespace.cr @@ -16,7 +16,14 @@ module Ameba::Rule::Layout def test(source) 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 diff --git a/src/ameba/rule/lint/comparison_to_boolean.cr b/src/ameba/rule/lint/comparison_to_boolean.cr index d95d68d7..cc9eda9e 100644 --- a/src/ameba/rule/lint/comparison_to_boolean.cr +++ b/src/ameba/rule/lint/comparison_to_boolean.cr @@ -20,6 +20,8 @@ module Ameba::Rule::Lint # Enabled: true # ``` class ComparisonToBoolean < Base + include AST::Util + properties do enabled false description "Disallows comparison to booleans" @@ -29,11 +31,31 @@ module Ameba::Rule::Lint OP_NAMES = %w(== != ===) def test(source, node : Crystal::Call) - comparison = node.name.in?(OP_NAMES) - to_boolean = node.args.first?.try(&.is_a?(Crystal::BoolLiteral)) || - node.obj.is_a?(Crystal::BoolLiteral) + return unless node.name.in?(OP_NAMES) + return unless node.args.size == 1 - 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 diff --git a/src/ameba/rule/metrics/cyclomatic_complexity.cr b/src/ameba/rule/metrics/cyclomatic_complexity.cr index 1047df67..1b0e6e90 100644 --- a/src/ameba/rule/metrics/cyclomatic_complexity.cr +++ b/src/ameba/rule/metrics/cyclomatic_complexity.cr @@ -9,6 +9,8 @@ module Ameba::Rule::Metrics # MaxComplexity: 10 # ``` class CyclomaticComplexity < Base + include AST::Util + properties do description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`" max_complexity 10 @@ -20,18 +22,9 @@ module Ameba::Rule::Metrics complexity = AST::CountingVisitor.new(node).count 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} 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 diff --git a/src/ameba/rule/performance/any_instead_of_empty.cr b/src/ameba/rule/performance/any_instead_of_empty.cr index 4ccd8633..ca0f5547 100644 --- a/src/ameba/rule/performance/any_instead_of_empty.cr +++ b/src/ameba/rule/performance/any_instead_of_empty.cr @@ -28,6 +28,8 @@ module Ameba::Rule::Performance # Enabled: true # ``` class AnyInsteadOfEmpty < Base + include AST::Util + properties do description "Identifies usage of arg-less `any?` calls." end @@ -39,8 +41,14 @@ module Ameba::Rule::Performance return unless node.name == ANY_NAME return unless node.block.nil? && node.args.empty? 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 diff --git a/src/ameba/rule/performance/chained_call_with_no_bang.cr b/src/ameba/rule/performance/chained_call_with_no_bang.cr index 954200c4..9ca3bf29 100644 --- a/src/ameba/rule/performance/chained_call_with_no_bang.cr +++ b/src/ameba/rule/performance/chained_call_with_no_bang.cr @@ -37,6 +37,8 @@ module Ameba::Rule::Performance # - reverse # ``` class ChainedCallWithNoBang < Base + include AST::Util + properties do description "Identifies usage of chained calls not utilizing the bang method variants." @@ -66,12 +68,15 @@ module Ameba::Rule::Performance end 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 node.name.in?(call_names) return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES) - issue_for node.name_location, node.name_end_location, - MSG % {node.name, obj.name} + issue_for location, end_location, MSG % {node.name, obj.name} do |corrector| + corrector.insert_after(end_location, '!') + end end end end diff --git a/src/ameba/rule/style/is_a_filter.cr b/src/ameba/rule/style/is_a_filter.cr index 31867d4e..4ed5e77c 100644 --- a/src/ameba/rule/style/is_a_filter.cr +++ b/src/ameba/rule/style/is_a_filter.cr @@ -72,11 +72,7 @@ module Ameba::Rule::Style end_location = node.end_location if !end_location || end_location.try(&.column_number.zero?) if end_location = path.end_location - end_location = Crystal::Location.new( - end_location.filename, - end_location.line_number, - end_location.column_number + 1 - ) + end_location = end_location.adjust(column_number: 1) end end diff --git a/src/ameba/rule/style/large_numbers.cr b/src/ameba/rule/style/large_numbers.cr index 4c2de7ff..1c04e458 100644 --- a/src/ameba/rule/style/large_numbers.cr +++ b/src/ameba/rule/style/large_numbers.cr @@ -43,11 +43,7 @@ module Ameba::Rule::Style if allowed?(*parsed) && (expected = underscored *parsed) != token.raw location = token.location - end_location = Crystal::Location.new( - location.filename, - location.line_number, - location.column_number + token.raw.size - 1 - ) + end_location = location.adjust(column_number: token.raw.size - 1) issue_for location, end_location, MSG % expected do |corrector| corrector.replace(location, end_location, expected) end diff --git a/src/ameba/rule/style/method_names.cr b/src/ameba/rule/style/method_names.cr index 112ffbd7..dd96682c 100644 --- a/src/ameba/rule/style/method_names.cr +++ b/src/ameba/rule/style/method_names.cr @@ -38,6 +38,8 @@ module Ameba::Rule::Style # Enabled: true # ``` class MethodNames < Base + include AST::Util + properties do description "Enforces method names to be in underscored case" end @@ -46,17 +48,10 @@ module Ameba::Rule::Style def test(source, node : Crystal::Def) 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 - 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} - ) + issue_for location, end_location, MSG % {expected, node.name} end end end diff --git a/src/ameba/rule/style/redundant_begin.cr b/src/ameba/rule/style/redundant_begin.cr index bfbf3813..6cdfb057 100644 --- a/src/ameba/rule/style/redundant_begin.cr +++ b/src/ameba/rule/style/redundant_begin.cr @@ -65,15 +65,27 @@ module Ameba::Rule::Style MSG = "Redundant `begin` block detected" def test(source, node : Crystal::Def) - issue_for node, MSG if redundant_begin?(source, node) - end + return unless def_loc = node.location - private def redundant_begin?(source, node) case body = node.body when Crystal::ExceptionHandler - redundant_begin_in_handler?(source, body, node) + return if begin_exprs_in_handler?(body) || inner_handler?(body) 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 @@ -81,27 +93,27 @@ module Ameba::Rule::Style node.keyword == :begin 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) handler.body.is_a?(Crystal::ExceptionHandler) end private def begin_exprs_in_handler?(handler) if (body = handler.body).is_a?(Crystal::Expressions) - body.expressions.first.is_a?(Crystal::ExceptionHandler) + body.expressions.first?.is_a?(Crystal::ExceptionHandler) 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 + 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 loop do @@ -111,7 +123,9 @@ module Ameba::Rule::Style when :EOF, :"->" break when :IDENT - return token.value == :begin if in_body + next unless in_body + return unless token.value == :begin + return token.location when :"(" in_argument_list = true when :")" @@ -121,9 +135,21 @@ module Ameba::Rule::Style when :SPACE # ignore else - return false if in_body + return if in_body 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 diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr index 233bbf44..0cb6a1ef 100644 --- a/src/ameba/rule/style/verbose_block.cr +++ b/src/ameba/rule/style/verbose_block.cr @@ -28,6 +28,8 @@ module Ameba::Rule::Style # MaxLength: 50 # use ~ to disable # ``` class VerboseBlock < Base + include AST::Util + properties do description "Identifies usage of collapsible single expression blocks." @@ -108,20 +110,27 @@ module Ameba::Rule::Style i 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| args.pop? if skip_last_arg 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? named_args.join io, ", " do |arg, inner_io| inner_io << arg.name << ": " << arg.value end end + + if short_block + io << ", " unless args.empty? && (named_args.nil? || named_args.empty?) + io << short_block + end end end - protected def node_to_s(node : Crystal::Call) + protected def node_to_s(source, node : Crystal::Call) String.build do |str| case name = node.name when "[]" @@ -137,29 +146,41 @@ module Ameba::Rule::Style args_to_s(str, node, skip_last_arg: true) str << "]=(" << node.args.last? << ')' else + short_block = short_block_code(source, node) 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 << '(' - args_to_s(str, node) + args_to_s(str, node, short_block) str << ')' end - str << " {...}" if node.block + str << " {...}" if node.block && short_block.nil? 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 += ", " if args call_chain = %w[].tap do |arr| obj = body.obj while obj.is_a?(Crystal::Call) - arr << node_to_s(obj) + arr << node_to_s(source, obj) obj = obj.obj end arr.reverse! - arr << node_to_s(body) + arr << node_to_s(source, body) end name = @@ -170,6 +191,8 @@ module Ameba::Rule::Style # ameba:disable Metrics/CyclomaticComplexity 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_multiple_line_blocks && !same_location_lines?(call, body) return if exclude_prefix_operators && prefix_operator?(body) @@ -177,13 +200,18 @@ module Ameba::Rule::Style return if exclude_setters && setter?(body.name) call_code = - call_code(call, body) + call_code(source, call, body) return unless valid_line_length?(call, call_code) return unless valid_length?(call_code) - issue_for call.name_location, block.end_location, - MSG % call_code + if call_code.includes?("{...}") + 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 def test(source, node : Crystal::Call) diff --git a/src/ameba/spec/support.cr b/src/ameba/spec/support.cr index 851401f6..2813ce15 100644 --- a/src/ameba/spec/support.cr +++ b/src/ameba/spec/support.cr @@ -14,5 +14,9 @@ module Ameba end end +def trailing_whitespace + ' ' +end + include Ameba::Spec::BeValid include Ameba::Spec::ExpectIssue