From 1a166c4a6a7e104cdb9bf7008e4d5b943d4083d5 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Mon, 2 Dec 2024 21:26:29 -0500 Subject: [PATCH 1/6] Add `Style/MultiLineCurlyBlock` --- .../rule/style/multi_line_curly_block_spec.cr | 30 ++++++++++++ .../rule/style/multi_line_curly_block.cr | 46 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 spec/ameba/rule/style/multi_line_curly_block_spec.cr create mode 100644 src/ameba/rule/style/multi_line_curly_block.cr diff --git a/spec/ameba/rule/style/multi_line_curly_block_spec.cr b/spec/ameba/rule/style/multi_line_curly_block_spec.cr new file mode 100644 index 000000000..a21fafcc9 --- /dev/null +++ b/spec/ameba/rule/style/multi_line_curly_block_spec.cr @@ -0,0 +1,30 @@ +require "../../../spec_helper" + +module Ameba::Rule::Style + describe MultiLineCurlyBlock do + subject = MultiLineCurlyBlock.new + + it "doesn't report if a curly block is on a single line" do + expect_no_issues subject, <<-CRYSTAL + incr_stack { node.accept(self) } + CRYSTAL + end + + it "doesn't report for `do`...`end` blocks" do + expect_no_issues subject, <<-CRYSTAL + incr_stack do + node.accept(self) + end + CRYSTAL + end + + it "reports if there is a multi-line curly block" do + expect_issue subject, <<-CRYSTAL + incr_stack { + # ^ error: Use `do`...`end` instead of curly brackets for multi-line blocks + node.accept(self) + } + CRYSTAL + end + end +end diff --git a/src/ameba/rule/style/multi_line_curly_block.cr b/src/ameba/rule/style/multi_line_curly_block.cr new file mode 100644 index 000000000..591859c2c --- /dev/null +++ b/src/ameba/rule/style/multi_line_curly_block.cr @@ -0,0 +1,46 @@ +module Ameba::Rule::Style + # A rule that disallows multi-line blocks that use curly brackets instead of + # `do`...`end`. + # + # For example, this is considered invalid: + # + # ``` + # incr_stack { + # node.body.accept(self) + # } + # ``` + # + # And should be rewritten to the following: + # + # ``` + # incr_stack do + # node.body.accept(self) + # end + # ``` + # + # YAML configuration example: + # + # ``` + # Style/MultiLineCurlyBlock: + # Enabled: true + # ``` + class MultiLineCurlyBlock < Base + include AST::Util + + properties do + since_version "1.7.0" + description "Disallows multi-line blocks using curly block syntax" + end + + MSG = "Use `do`...`end` instead of curly brackets for multi-line blocks" + + def test(source, node : Crystal::Block) + return unless start_location = node.location + return unless end_location = node.end_location + return if start_location.line_number == end_location.line_number + return unless source.code[source.pos(start_location)]? == '{' + + issue_for node, MSG + end + end +end From bb50c50c13424cbdece8df22d4d740f9bbd39113 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Mon, 2 Dec 2024 21:35:12 -0500 Subject: [PATCH 2/6] Fix ameba codebase due to new rule --- src/ameba/rule/documentation/documentation_admonition.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ameba/rule/documentation/documentation_admonition.cr b/src/ameba/rule/documentation/documentation_admonition.cr index 5b7993838..a9fd7e8e9 100644 --- a/src/ameba/rule/documentation/documentation_admonition.cr +++ b/src/ameba/rule/documentation/documentation_admonition.cr @@ -45,9 +45,9 @@ module Ameba::Rule::Documentation MSG_ERR = "%s admonition error: %s" @[YAML::Field(ignore: true)] - private getter location : Time::Location { + private getter location : Time::Location do Time::Location.load(self.timezone) - } + end def test(source) Tokenizer.new(source).run do |token| From c61224cf950e26077ba42660b0969eee7d1a0351 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Tue, 3 Dec 2024 09:28:19 -0500 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Sijawusz Pur Rahnama --- spec/ameba/rule/style/multi_line_curly_block_spec.cr | 12 ++++++------ src/ameba/rule/style/multi_line_curly_block.cr | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/spec/ameba/rule/style/multi_line_curly_block_spec.cr b/spec/ameba/rule/style/multi_line_curly_block_spec.cr index a21fafcc9..c68abba9e 100644 --- a/spec/ameba/rule/style/multi_line_curly_block_spec.cr +++ b/spec/ameba/rule/style/multi_line_curly_block_spec.cr @@ -6,23 +6,23 @@ module Ameba::Rule::Style it "doesn't report if a curly block is on a single line" do expect_no_issues subject, <<-CRYSTAL - incr_stack { node.accept(self) } + foo { :bar } CRYSTAL end it "doesn't report for `do`...`end` blocks" do expect_no_issues subject, <<-CRYSTAL - incr_stack do - node.accept(self) + foo do + :bar end CRYSTAL end it "reports if there is a multi-line curly block" do expect_issue subject, <<-CRYSTAL - incr_stack { - # ^ error: Use `do`...`end` instead of curly brackets for multi-line blocks - node.accept(self) + foo { + # ^ error: Use `do`...`end` instead of curly brackets for multi-line blocks + :bar } CRYSTAL end diff --git a/src/ameba/rule/style/multi_line_curly_block.cr b/src/ameba/rule/style/multi_line_curly_block.cr index 591859c2c..5ae77af6e 100644 --- a/src/ameba/rule/style/multi_line_curly_block.cr +++ b/src/ameba/rule/style/multi_line_curly_block.cr @@ -1,20 +1,20 @@ module Ameba::Rule::Style - # A rule that disallows multi-line blocks that use curly brackets instead of - # `do`...`end`. + # A rule that disallows multi-line blocks that use curly brackets + # instead of `do`...`end`. # # For example, this is considered invalid: # # ``` - # incr_stack { - # node.body.accept(self) + # (0..10).map { |i| + # i * 2 # } # ``` # # And should be rewritten to the following: # # ``` - # incr_stack do - # node.body.accept(self) + # (0..10).map do |i| + # i * 2 # end # ``` # From 30e301e630cf132034872f50f57dc29c5f4641e0 Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Tue, 3 Dec 2024 13:18:42 -0500 Subject: [PATCH 4/6] Update multi_line_curly_block_spec.cr Co-authored-by: Sijawusz Pur Rahnama --- spec/ameba/rule/style/multi_line_curly_block_spec.cr | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/ameba/rule/style/multi_line_curly_block_spec.cr b/spec/ameba/rule/style/multi_line_curly_block_spec.cr index c68abba9e..4d3ed8799 100644 --- a/spec/ameba/rule/style/multi_line_curly_block_spec.cr +++ b/spec/ameba/rule/style/multi_line_curly_block_spec.cr @@ -18,6 +18,11 @@ module Ameba::Rule::Style CRYSTAL end + it "doesn't report for `do`...`end` blocks on a single line" do + expect_no_issues subject, <<-CRYSTAL + foo do :bar end + CRYSTAL + end it "reports if there is a multi-line curly block" do expect_issue subject, <<-CRYSTAL foo { From 255b87b3185cff3946899ed58b1c2ac0a45a0b7a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 4 Dec 2024 06:14:46 +0100 Subject: [PATCH 5/6] Update spec/ameba/rule/style/multi_line_curly_block_spec.cr --- spec/ameba/rule/style/multi_line_curly_block_spec.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/ameba/rule/style/multi_line_curly_block_spec.cr b/spec/ameba/rule/style/multi_line_curly_block_spec.cr index 4d3ed8799..94b6c86ef 100644 --- a/spec/ameba/rule/style/multi_line_curly_block_spec.cr +++ b/spec/ameba/rule/style/multi_line_curly_block_spec.cr @@ -23,6 +23,7 @@ module Ameba::Rule::Style foo do :bar end CRYSTAL end + it "reports if there is a multi-line curly block" do expect_issue subject, <<-CRYSTAL foo { From d7f1840d0d15118d831a0b0e4d59247cc95bf45a Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Wed, 4 Dec 2024 00:20:31 -0500 Subject: [PATCH 6/6] MultiLineCurlyBlock -> MultilineCurlyBlock --- ...line_curly_block_spec.cr => multiline_curly_block_spec.cr} | 4 ++-- .../{multi_line_curly_block.cr => multiline_curly_block.cr} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename spec/ameba/rule/style/{multi_line_curly_block_spec.cr => multiline_curly_block_spec.cr} (91%) rename src/ameba/rule/style/{multi_line_curly_block.cr => multiline_curly_block.cr} (93%) diff --git a/spec/ameba/rule/style/multi_line_curly_block_spec.cr b/spec/ameba/rule/style/multiline_curly_block_spec.cr similarity index 91% rename from spec/ameba/rule/style/multi_line_curly_block_spec.cr rename to spec/ameba/rule/style/multiline_curly_block_spec.cr index 94b6c86ef..fc902b82a 100644 --- a/spec/ameba/rule/style/multi_line_curly_block_spec.cr +++ b/spec/ameba/rule/style/multiline_curly_block_spec.cr @@ -1,8 +1,8 @@ require "../../../spec_helper" module Ameba::Rule::Style - describe MultiLineCurlyBlock do - subject = MultiLineCurlyBlock.new + describe MultilineCurlyBlock do + subject = MultilineCurlyBlock.new it "doesn't report if a curly block is on a single line" do expect_no_issues subject, <<-CRYSTAL diff --git a/src/ameba/rule/style/multi_line_curly_block.cr b/src/ameba/rule/style/multiline_curly_block.cr similarity index 93% rename from src/ameba/rule/style/multi_line_curly_block.cr rename to src/ameba/rule/style/multiline_curly_block.cr index 5ae77af6e..45c2130ce 100644 --- a/src/ameba/rule/style/multi_line_curly_block.cr +++ b/src/ameba/rule/style/multiline_curly_block.cr @@ -21,10 +21,10 @@ module Ameba::Rule::Style # YAML configuration example: # # ``` - # Style/MultiLineCurlyBlock: + # Style/MultilineCurlyBlock: # Enabled: true # ``` - class MultiLineCurlyBlock < Base + class MultilineCurlyBlock < Base include AST::Util properties do