From aa1dbf16c9f93b1d8ad8dd4dd24301aa3ef182cf Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 12 Feb 2025 17:43:30 +0100 Subject: [PATCH 01/42] =?UTF-8?q?Tweak=20rules=E2=80=99=20docs=20a=20bit?= =?UTF-8?q?=20moar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ameba/rule/lint/duplicated_require.cr | 4 ++-- src/ameba/rule/lint/empty_ensure.cr | 4 ++-- src/ameba/rule/lint/rand_zero.cr | 2 +- src/ameba/rule/style/negated_conditions_in_unless.cr | 2 +- src/ameba/rule/style/while_true.cr | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ameba/rule/lint/duplicated_require.cr b/src/ameba/rule/lint/duplicated_require.cr index fcbc64efc..50c8f5c0c 100644 --- a/src/ameba/rule/lint/duplicated_require.cr +++ b/src/ameba/rule/lint/duplicated_require.cr @@ -1,5 +1,5 @@ module Ameba::Rule::Lint - # A rule that reports duplicated require statements. + # A rule that reports duplicated `require` statements. # # ``` # require "./thing" @@ -16,7 +16,7 @@ module Ameba::Rule::Lint class DuplicatedRequire < Base properties do since_version "0.14.0" - description "Reports duplicated require statements" + description "Reports duplicated `require` statements" end MSG = "Duplicated require of `%s`" diff --git a/src/ameba/rule/lint/empty_ensure.cr b/src/ameba/rule/lint/empty_ensure.cr index fc08f3fc2..122b8fd57 100644 --- a/src/ameba/rule/lint/empty_ensure.cr +++ b/src/ameba/rule/lint/empty_ensure.cr @@ -1,5 +1,5 @@ module Ameba::Rule::Lint - # A rule that disallows empty ensure statement. + # A rule that disallows empty `ensure` statement. # # For example, this is considered invalid: # @@ -40,7 +40,7 @@ module Ameba::Rule::Lint class EmptyEnsure < Base properties do since_version "0.3.0" - description "Disallows empty ensure statement" + description "Disallows empty `ensure` statement" end MSG = "Empty `ensure` block detected" diff --git a/src/ameba/rule/lint/rand_zero.cr b/src/ameba/rule/lint/rand_zero.cr index 32f92bb01..349d9ad8b 100644 --- a/src/ameba/rule/lint/rand_zero.cr +++ b/src/ameba/rule/lint/rand_zero.cr @@ -25,7 +25,7 @@ module Ameba::Rule::Lint class RandZero < Base properties do since_version "0.5.1" - description "Disallows rand zero calls" + description "Disallows `rand` zero calls" end MSG = "`%s` always returns `0`" diff --git a/src/ameba/rule/style/negated_conditions_in_unless.cr b/src/ameba/rule/style/negated_conditions_in_unless.cr index bd4156fdc..7fa7d4614 100644 --- a/src/ameba/rule/style/negated_conditions_in_unless.cr +++ b/src/ameba/rule/style/negated_conditions_in_unless.cr @@ -29,7 +29,7 @@ module Ameba::Rule::Style class NegatedConditionsInUnless < Base properties do since_version "0.2.0" - description "Disallows negated conditions in unless" + description "Disallows negated conditions in `unless`" end MSG = "Avoid negated conditions in unless blocks" diff --git a/src/ameba/rule/style/while_true.cr b/src/ameba/rule/style/while_true.cr index c6a2b173b..0a95fd7f7 100644 --- a/src/ameba/rule/style/while_true.cr +++ b/src/ameba/rule/style/while_true.cr @@ -28,7 +28,7 @@ module Ameba::Rule::Style class WhileTrue < Base properties do since_version "0.3.0" - description "Disallows while statements with a true literal as condition" + description "Disallows `while` statements with a `true` literal as condition" end MSG = "While statement using `true` literal as condition" From f9107035cf5f72b734933bbe09c159fc5baf9b7f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 12 Feb 2025 17:44:20 +0100 Subject: [PATCH 02/42] Use the unescaped literal value in specs --- spec/ameba/rule/lint/unused_literal_spec.cr | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr index af17b796f..61d616a87 100644 --- a/spec/ameba/rule/lint/unused_literal_spec.cr +++ b/spec/ameba/rule/lint/unused_literal_spec.cr @@ -11,7 +11,7 @@ module Ameba::Rule::Lint end it "passes if a char literal is used to assign" do - expect_no_issues subject, <<-CRYSTAL + expect_no_issues subject, <<-'CRYSTAL' c = '\t' CRYSTAL end @@ -146,9 +146,9 @@ module Ameba::Rule::Lint end it "fails if a char literal is top-level" do - expect_issue subject, <<-CRYSTAL + expect_issue subject, <<-'CRYSTAL' '\t' - # ^ error: Literal value is not used + # ^^ error: Literal value is not used CRYSTAL end @@ -227,10 +227,10 @@ module Ameba::Rule::Lint end it "fails if a char literal is in void of method body" do - expect_issue subject, <<-CRYSTAL + expect_issue subject, <<-'CRYSTAL' def foo '\t' - # ^^^ error: Literal value is not used + # ^^^^ error: Literal value is not used return end CRYSTAL @@ -323,10 +323,10 @@ module Ameba::Rule::Lint end it "fails if a char literal is in void of if statement body" do - expect_issue subject, <<-CRYSTAL + expect_issue subject, <<-'CRYSTAL' if true '\t' - # ^^^ error: Literal value is not used + # ^^^^ error: Literal value is not used nil end CRYSTAL From 28f9df56051ff21e81e42b76036135c8fea229bd Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 12 Feb 2025 17:51:17 +0100 Subject: [PATCH 03/42] Minor spec cleanups --- spec/ameba/rule/lint/unused_literal_spec.cr | 28 ++++++++++----------- spec/spec_helper.cr | 2 +- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr index 61d616a87..9dbd186ee 100644 --- a/spec/ameba/rule/lint/unused_literal_spec.cr +++ b/spec/ameba/rule/lint/unused_literal_spec.cr @@ -71,9 +71,7 @@ module Ameba::Rule::Lint expect_no_issues subject, <<-CRYSTAL def foo : Nil return - :bar - nil end CRYSTAL @@ -126,8 +124,8 @@ module Ameba::Rule::Lint expect_issue subject, <<-'CRYSTAL' "hello world" # ^^^^^^^^^^^ error: Literal value is not used - "interp #{string}" - # ^^^^^^^^^^^^^^^^ error: Literal value is not used + "foo #{bar}" + # ^^^^^^^^^^ error: Literal value is not used CRYSTAL end @@ -199,8 +197,8 @@ module Ameba::Rule::Lint def foo "hello world" # ^^^^^^^^^^^^^ error: Literal value is not used - "interp #{string}" - # ^^^^^^^^^^^^^^^^^^ error: Literal value is not used + "foo #{bar}" + # ^^^^^^^^^^^^ error: Literal value is not used return end CRYSTAL @@ -295,8 +293,8 @@ module Ameba::Rule::Lint if true "hello world" # ^^^^^^^^^^^^^ error: Literal value is not used - "interp #{string}" - # ^^^^^^^^^^^^^^^^^^ error: Literal value is not used + "foo #{bar}" + # ^^^^^^^^^^^^ error: Literal value is not used nil end CRYSTAL @@ -469,21 +467,21 @@ module Ameba::Rule::Lint {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %} it "fails if a regex literal is unused" do expect_issue subject, <<-'CRYSTAL' - a = /hello world/ + foo = /hello world/ /goodnight moon/ # ^^^^^^^^^^^^^^ error: Literal value is not used - b = /goodnight moon, #{a}/ - /goodnight moon, #{a}/ - # ^^^^^^^^^^^^^^^^^^^^ error: Literal value is not used + bar = /goodnight moon, #{foo}/ + /goodnight moon, #{foo}/ + # ^^^^^^^^^^^^^^^^^^^^^^ error: Literal value is not used CRYSTAL end {% else %} it "passes if a regex literal is unused" do expect_no_issues subject, <<-'CRYSTAL' - a = /hello world/ + foo = /hello world/ /goodnight moon/ - b = /goodnight moon, #{a}/ - /goodnight moon, #{a}/ + bar = /goodnight moon, #{foo}/ + /goodnight moon, #{foo}/ CRYSTAL end {% end %} diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 2e948660b..2c4572ce7 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -67,7 +67,7 @@ module Ameba getter call_queue = {} of AST::Scope => Array(Crystal::Call) properties do - description "Internal rule to self calls in test scopes" + description "Internal rule to test calls to `self` in scopes" end def test(source, node : Crystal::Call, scope : AST::Scope) From fc19f01d9a2d3d07ce85c98e468525638af15e41 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Feb 2025 18:33:49 +0100 Subject: [PATCH 04/42] Use trailing `if` for better readability --- src/ameba/rule/typing/macro_call_argument_type_restriction.cr | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr index 7f7805a73..5dd903105 100644 --- a/src/ameba/rule/typing/macro_call_argument_type_restriction.cr +++ b/src/ameba/rule/typing/macro_call_argument_type_restriction.cr @@ -82,9 +82,7 @@ module Ameba::Rule::Typing node.args.each do |arg| case arg when Crystal::Assign - next unless default_value? - - issue_for arg.target, MSG + issue_for arg.target, MSG if default_value? when Crystal::Var, Crystal::Call, Crystal::StringLiteral, Crystal::SymbolLiteral issue_for arg, MSG end From 269871ff19bc5e00439e8a1b042586b34027d031 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Feb 2025 18:58:22 +0100 Subject: [PATCH 05/42] Add additional test case --- spec/ameba/rule/style/multiline_curly_block_spec.cr | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/ameba/rule/style/multiline_curly_block_spec.cr b/spec/ameba/rule/style/multiline_curly_block_spec.cr index fc902b82a..edf3e3c0b 100644 --- a/spec/ameba/rule/style/multiline_curly_block_spec.cr +++ b/spec/ameba/rule/style/multiline_curly_block_spec.cr @@ -32,5 +32,14 @@ module Ameba::Rule::Style } CRYSTAL end + + it "reports if there is a multi-line curly block with arguments" do + expect_issue subject, <<-CRYSTAL + foo(:bar) { + # ^ error: Use `do`...`end` instead of curly brackets for multi-line blocks + :baz + } + CRYSTAL + end end end From 216e13212ba2cd6b8ee8acff4e51cbbb23046aa2 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 15 Feb 2025 00:11:22 +0100 Subject: [PATCH 06/42] Call `super` instead of duplicating the method body --- src/ameba/rule/lint/spec_focus.cr | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ameba/rule/lint/spec_focus.cr b/src/ameba/rule/lint/spec_focus.cr index 09507e965..4bafbe225 100644 --- a/src/ameba/rule/lint/spec_focus.cr +++ b/src/ameba/rule/lint/spec_focus.cr @@ -55,9 +55,7 @@ module Ameba::Rule::Lint SPEC_ITEM_NAMES = %w[describe context it pending] def test(source) - return unless source.spec? - - AST::NodeVisitor.new self, source + return super if source.spec? end def test(source, node : Crystal::Call) From ba14e7787b0576e84c1427254570ea6c34929fa4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 16 Feb 2025 02:00:16 +0100 Subject: [PATCH 07/42] Fix example in docs for `Lint/AmbiguousAssignment` rule --- src/ameba/rule/lint/ambiguous_assignment.cr | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/ameba/rule/lint/ambiguous_assignment.cr b/src/ameba/rule/lint/ambiguous_assignment.cr index 6cacc0186..e9c371a25 100644 --- a/src/ameba/rule/lint/ambiguous_assignment.cr +++ b/src/ameba/rule/lint/ambiguous_assignment.cr @@ -3,19 +3,15 @@ module Ameba::Rule::Lint # # This is considered invalid: # - # ``` - # x = -y - # x = +y - # x = !y - # ``` + # x =- y + # x =+ y + # x =! y # # And this is valid: # - # ``` - # x -= y # or x = -y - # x += y # or x = +y - # x != y # or x = !y - # ``` + # x -= y # or x = -y + # x += y # or x = +y + # x != y # or x = !y # # YAML configuration example: # From ded99dad6fb7819cd2f556c00b985ff4abb58bb5 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 16 Feb 2025 02:00:56 +0100 Subject: [PATCH 08/42] Fix case where only named arguments are passed to `issue_for` macro --- src/ameba/rule/base.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 242c86f0b..d46208c0c 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -115,7 +115,7 @@ module Ameba::Rule # Adds an issue to the *source* macro issue_for(*args, **kwargs, &block) - source.add_issue(self, {{ args.splat }}, {{ kwargs.double_splat }}) {{ block }} + source.add_issue(self, {{ args.splat(", ") }}{{ kwargs.double_splat }}) {{ block }} end protected def self.rule_name From 9ee7f06837cc41ed38d48110142688af96a6e26f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 17 Feb 2025 02:17:33 +0100 Subject: [PATCH 09/42] Tweak specs for `Style/ParenthesesAroundCondition` rule --- .../parentheses_around_condition_spec.cr | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/ameba/rule/style/parentheses_around_condition_spec.cr b/spec/ameba/rule/style/parentheses_around_condition_spec.cr index 9ea8b2304..a6ce82bb0 100644 --- a/spec/ameba/rule/style/parentheses_around_condition_spec.cr +++ b/spec/ameba/rule/style/parentheses_around_condition_spec.cr @@ -44,17 +44,8 @@ module Ameba::Rule::Style context "properties" do context "#exclude_ternary" do - it "skips ternary control expressions by default" do - expect_no_issues subject, <<-CRYSTAL - (foo > bar) ? true : false - CRYSTAL - end - - it "allows to configure assignments" do - rule = ParenthesesAroundCondition.new - rule.exclude_ternary = false - - expect_issue rule, <<-CRYSTAL + it "reports ternary control expressions by default" do + expect_issue subject, <<-CRYSTAL (foo.empty?) ? true : false # ^^^^^^^^^^ error: Redundant parentheses CRYSTAL @@ -73,6 +64,15 @@ module Ameba::Rule::Style (foo rescue 42) ? true : false CRYSTAL end + + it "allows to skips ternary control expressions" do + rule = ParenthesesAroundCondition.new + rule.exclude_ternary = true + + expect_no_issues rule, <<-CRYSTAL + (foo.empty?) ? true : false + CRYSTAL + end end context "#allow_safe_assignment" do From cee0d8b502df4e7c6567ee82914152ee763c31e4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 18 Feb 2025 18:53:25 +0100 Subject: [PATCH 10/42] Optimize a bit `Lint/DuplicatedRequire` rule --- src/ameba/rule/lint/duplicated_require.cr | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/ameba/rule/lint/duplicated_require.cr b/src/ameba/rule/lint/duplicated_require.cr index 50c8f5c0c..325351c5f 100644 --- a/src/ameba/rule/lint/duplicated_require.cr +++ b/src/ameba/rule/lint/duplicated_require.cr @@ -23,9 +23,13 @@ module Ameba::Rule::Lint def test(source) nodes = AST::TopLevelNodesVisitor.new(source.ast).require_nodes - nodes.each_with_object([] of String) do |node, processed_require_strings| - issue_for(node, MSG % node.string) if node.string.in?(processed_require_strings) - processed_require_strings << node.string + nodes.each_with_object(Set(String).new) do |node, processed_require_strings| + node_s = node.string + if processed_require_strings.includes?(node_s) + issue_for node, MSG % node_s + else + processed_require_strings << node_s + end end end end From ef01426b68ab5d4b9b5ba04eef5ba16319672417 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 18 Feb 2025 18:58:53 +0100 Subject: [PATCH 11/42] =?UTF-8?q?Don=E2=80=99t=20use=20parentheses=20when?= =?UTF-8?q?=20calling=20`issue=5Ffor`=20unless=20necessary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/spec_helper.cr | 14 +++++++------- src/ameba/rule/documentation/documentation.cr | 2 +- src/ameba/rule/layout/trailing_blank_lines.cr | 6 ++++-- src/ameba/rule/style/is_a_filter.cr | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 2c4572ce7..169049c17 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -136,7 +136,7 @@ module Ameba return unless name = node_source(node.name, source.lines) 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| corrector.replace(node.name, name.sub("A", "AA")) end end @@ -153,7 +153,7 @@ module Ameba return unless name = node_source(node.name, source.lines) 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| corrector.replace(node.name, name.tr("A", "B")) end end @@ -170,7 +170,7 @@ module Ameba return unless name = node_source(node.name, source.lines) 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| corrector.replace(node.name, name.tr("B", "A")) end end @@ -187,7 +187,7 @@ module Ameba return unless name = node_source(node.name, source.lines) 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| corrector.replace(node.name, name.tr("B", "C")) end end @@ -204,7 +204,7 @@ module Ameba return unless name = node_source(node.name, source.lines) 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| corrector.replace(node.name, name.tr("C", "A")) end end @@ -222,7 +222,7 @@ module Ameba end_location = location.adjust(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") end end @@ -240,7 +240,7 @@ module Ameba end_location = location.adjust(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") end end diff --git a/src/ameba/rule/documentation/documentation.cr b/src/ameba/rule/documentation/documentation.cr index dd1f69456..2311a18d2 100644 --- a/src/ameba/rule/documentation/documentation.cr +++ b/src/ameba/rule/documentation/documentation.cr @@ -75,7 +75,7 @@ module Ameba::Rule::Documentation # bail out if the node has the documentation present return if node.doc.presence - issue_for(node, MSG) + issue_for node, MSG end end end diff --git a/src/ameba/rule/layout/trailing_blank_lines.cr b/src/ameba/rule/layout/trailing_blank_lines.cr index b8b727d8c..5632d73a7 100644 --- a/src/ameba/rule/layout/trailing_blank_lines.cr +++ b/src/ameba/rule/layout/trailing_blank_lines.cr @@ -28,10 +28,12 @@ module Ameba::Rule::Layout return if source_lines_size.zero? || (source_lines.last(2).join.presence && last_line_empty) + location = {source_lines_size, 1} + if last_line_empty - issue_for({source_lines_size, 1}, MSG) + issue_for location, MSG else - issue_for({source_lines_size, 1}, MSG_FINAL_NEWLINE) do |corrector| + issue_for location, MSG_FINAL_NEWLINE do |corrector| corrector.insert_before({source_lines_size + 1, 1}, '\n') end end diff --git a/src/ameba/rule/style/is_a_filter.cr b/src/ameba/rule/style/is_a_filter.cr index 07821d4d8..bef841347 100644 --- a/src/ameba/rule/style/is_a_filter.cr +++ b/src/ameba/rule/style/is_a_filter.cr @@ -72,11 +72,11 @@ module Ameba::Rule::Style msg = MSG % {new, old} if end_location = node.end_location - issue_for(location, end_location, msg) do |corrector| + issue_for location, end_location, msg do |corrector| corrector.replace(location, end_location, new) end else - issue_for(location, nil, msg) + issue_for location, nil, msg end end end From a3dcc396112f84fea90425d7e524a6b53f0ab782 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 18 Feb 2025 19:06:50 +0100 Subject: [PATCH 12/42] Simplify code in `Lint/ShadowedException` rule --- src/ameba/rule/lint/shadowed_exception.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ameba/rule/lint/shadowed_exception.cr b/src/ameba/rule/lint/shadowed_exception.cr index 0a65c4ebb..fa59ca5a5 100644 --- a/src/ameba/rule/lint/shadowed_exception.cr +++ b/src/ameba/rule/lint/shadowed_exception.cr @@ -42,8 +42,7 @@ module Ameba::Rule::Lint MSG = "Shadowed exception found: `%s`" def test(source, node : Crystal::ExceptionHandler) - rescues = node.rescues - return if rescues.nil? + return unless rescues = node.rescues shadowed(rescues).each do |path| issue_for path, MSG % path.names.join("::") From 671d97876e78276ab1525f6d8856babeb6e01986 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 19 Feb 2025 16:03:28 +0100 Subject: [PATCH 13/42] Use `%{}` interpolation variant consistently --- .../documentation_admonition_spec.cr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/ameba/rule/documentation/documentation_admonition_spec.cr b/spec/ameba/rule/documentation/documentation_admonition_spec.cr index 9387961f1..53d82564c 100644 --- a/spec/ameba/rule/documentation/documentation_admonition_spec.cr +++ b/spec/ameba/rule/documentation/documentation_admonition_spec.cr @@ -19,19 +19,19 @@ module Ameba::Rule::Documentation subject.admonitions.each do |admonition| expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition}: Single-line comment - # ^{admonition} error: Found a #{admonition} admonition in a comment + # ^{admonition} error: Found a %{admonition} admonition in a comment CRYSTAL expect_issue subject, <<-CRYSTAL, admonition: admonition # Text before ... # %{admonition}(some context): Part of multi-line comment - # ^{admonition} error: Found a #{admonition} admonition in a comment + # ^{admonition} error: Found a %{admonition} admonition in a comment # Text after ... CRYSTAL expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition} - # ^{admonition} error: Found a #{admonition} admonition in a comment + # ^{admonition} error: Found a %{admonition} admonition in a comment if rand > 0.5 end CRYSTAL @@ -53,7 +53,7 @@ module Ameba::Rule::Documentation past_date = (Time.utc - 21.days).to_s(format: "%F") expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition}(#{past_date}): sth in the past - # ^{admonition} error: Found a #{admonition} admonition in a comment (21 days past) + # ^{admonition} error: Found a %{admonition} admonition in a comment (21 days past) CRYSTAL end end @@ -63,7 +63,7 @@ module Ameba::Rule::Documentation yesterday_date = (Time.utc - 1.day).to_s(format: "%F") expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition}(#{yesterday_date}): sth in the past - # ^{admonition} error: Found a #{admonition} admonition in a comment (1 day past) + # ^{admonition} error: Found a %{admonition} admonition in a comment (1 day past) CRYSTAL end end @@ -73,7 +73,7 @@ module Ameba::Rule::Documentation today_date = Time.utc.to_s(format: "%F") expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition}(#{today_date}): sth in the past - # ^{admonition} error: Found a #{admonition} admonition in a comment (today is the day!) + # ^{admonition} error: Found a %{admonition} admonition in a comment (today is the day!) CRYSTAL end end @@ -82,7 +82,7 @@ module Ameba::Rule::Documentation subject.admonitions.each do |admonition| expect_issue subject, <<-CRYSTAL, admonition: admonition # %{admonition}(0000-00-00): sth wrong - # ^{admonition} error: #{admonition} admonition error: Invalid time: "0000-00-00" + # ^{admonition} error: %{admonition} admonition error: Invalid time: "0000-00-00" CRYSTAL end end @@ -97,7 +97,7 @@ module Ameba::Rule::Documentation rule.admonitions.each do |admonition| expect_issue rule, <<-CRYSTAL, admonition: admonition # %{admonition} - # ^{admonition} error: Found a #{admonition} admonition in a comment + # ^{admonition} error: Found a %{admonition} admonition in a comment CRYSTAL end From e15a59d00322f3beed45a6ff4515baaa18fe23f4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 19 Feb 2025 18:49:12 +0100 Subject: [PATCH 14/42] Use `ASTNode#accept(Visitor)` consistently --- src/ameba/ast/branch.cr | 2 +- src/contrib/read_type_doc.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ameba/ast/branch.cr b/src/ameba/ast/branch.cr index e2413d766..94d8dc92a 100644 --- a/src/ameba/ast/branch.cr +++ b/src/ameba/ast/branch.cr @@ -76,7 +76,7 @@ module Ameba::AST property branch : Branch? def initialize(@node : Crystal::ASTNode, parent_node : Crystal::ASTNode) - accept parent_node + parent_node.accept self end private def on_branchable_start(node, *branches) diff --git a/src/contrib/read_type_doc.cr b/src/contrib/read_type_doc.cr index 035a67344..fbdf02601 100644 --- a/src/contrib/read_type_doc.cr +++ b/src/contrib/read_type_doc.cr @@ -5,7 +5,7 @@ private class DocFinder < Crystal::Visitor getter doc : String? def initialize(nodes, @type_name) - accept(nodes) + nodes.accept self end def visit(node : Crystal::ASTNode) From 58ee592b0ec39ff09506146e9ec9f1988ac6fec1 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 19 Feb 2025 18:52:44 +0100 Subject: [PATCH 15/42] Make `Typos::Typo` record private --- src/ameba/rule/lint/typos.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/lint/typos.cr b/src/ameba/rule/lint/typos.cr index 28e389241..7b6daf567 100644 --- a/src/ameba/rule/lint/typos.cr +++ b/src/ameba/rule/lint/typos.cr @@ -29,7 +29,7 @@ module Ameba::Rule::Lint @@mutex = Mutex.new - protected record Typo, + private record Typo, typo : String, corrections : Array(String), location : {Int32, Int32}, From b806aa1c2ae6a45cefe25b8e52d8a6df2d4a4f46 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 25 Feb 2025 22:53:57 +0100 Subject: [PATCH 16/42] Spec helper cleanups --- spec/ameba/formatter/todo_formatter_spec.cr | 2 +- spec/spec_helper.cr | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index 00afbc898..72b9910ca 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -106,7 +106,7 @@ module Ameba # Problems found: 3 # Run `ameba --only Ameba/DummyRule` for details Ameba/DummyRule: - Description: Dummy rule that does nothing. + Description: Dummy rule that does nothing Dummy: true Excluded: - source1.cr diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 169049c17..3e4114d4d 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -6,7 +6,7 @@ module Ameba # Dummy Rule which does nothing. class DummyRule < Rule::Base properties do - description "Dummy rule that does nothing." + description "Dummy rule that does nothing" dummy true end @@ -16,7 +16,7 @@ module Ameba class NamedRule < Rule::Base properties do - description "A rule with a custom name." + description "A rule with a custom name" end def self.name @@ -27,7 +27,7 @@ module Ameba class VersionedRule < Rule::Base properties do since_version "1.5.0" - description "Rule with a custom version." + description "Rule with a custom version" end def test(source) From 956e0c6932859f6826345d9d576c1d715352e740 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 25 Feb 2025 22:54:16 +0100 Subject: [PATCH 17/42] Assign variables within the `describe ` block --- spec/ameba/ast/visitors/node_visitor_spec.cr | 7 +++---- .../visitors/redundant_control_expression_visitor_spec.cr | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/spec/ameba/ast/visitors/node_visitor_spec.cr b/spec/ameba/ast/visitors/node_visitor_spec.cr index 38535a9c4..667e703d2 100644 --- a/spec/ameba/ast/visitors/node_visitor_spec.cr +++ b/spec/ameba/ast/visitors/node_visitor_spec.cr @@ -1,13 +1,12 @@ require "../../../spec_helper" module Ameba::AST - rule = DummyRule.new - source = Source.new "" - describe NodeVisitor do describe "visit" do it "allow to visit ASTNode" do - visitor = NodeVisitor.new rule, source + rule = DummyRule.new + visitor = NodeVisitor.new rule, Source.new "" + nodes = Crystal::Parser.new("").parse nodes.accept visitor end diff --git a/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr b/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr index a9f8ae8cb..19ec13562 100644 --- a/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr +++ b/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr @@ -1,10 +1,10 @@ require "../../../spec_helper" module Ameba::AST - source = Source.new "" - rule = RedundantControlExpressionRule.new - describe RedundantControlExpressionVisitor do + source = Source.new "" + rule = RedundantControlExpressionRule.new + node = as_node <<-CRYSTAL a = 1 b = 2 From 1b5bd5d09ffa0b0ffe7846089dfdbb9f129211d2 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 26 Feb 2025 12:44:29 +0100 Subject: [PATCH 18/42] Tweak code comment --- src/ameba/rule/lint/unused_local_variable_access.cr | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ameba/rule/lint/unused_local_variable_access.cr b/src/ameba/rule/lint/unused_local_variable_access.cr index a0c3b6511..f63b6c9af 100644 --- a/src/ameba/rule/lint/unused_local_variable_access.cr +++ b/src/ameba/rule/lint/unused_local_variable_access.cr @@ -70,7 +70,9 @@ module Ameba::Rule::Lint end def test(source, node : Crystal::Var, in_macro : Bool) - return if node.name == "self" # This case will be reported by `Lint/UnusedSelf` rule + # This case will be reported by `Lint/UnusedSelfAccess` rule + return if node.name == "self" + # Ignore `debug` and `skip_file` macro methods return if in_macro && node.name.in?("debug", "skip_file") issue_for node, MSG From bc4a444dedeedccf73889a5a2f71cc89b2ca51f4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 02:34:59 +0100 Subject: [PATCH 19/42] Fix `Style/HeredocIndent` rule docs --- src/ameba/rule/style/heredoc_indent.cr | 37 +++++++++++++------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/ameba/rule/style/heredoc_indent.cr b/src/ameba/rule/style/heredoc_indent.cr index 1c5df6a84..6b491527e 100644 --- a/src/ameba/rule/style/heredoc_indent.cr +++ b/src/ameba/rule/style/heredoc_indent.cr @@ -1,32 +1,31 @@ module Ameba::Rule::Style - # A rule that enforces _Heredoc_ bodies be indented one level above the indentation of the - # line they're used on. + # A rule that enforces _heredoc_ bodies be indented one level above the + # indentation of the line they're used on. # # For example, this is considered invalid: # - # ``` - # <<-HERERDOC - # hello world - # HEREDOC + # <<-HEREDOC + # hello world + # HEREDOC # - # <<-HERERDOC - # hello world - # HEREDOC - # ``` + # <<-HEREDOC + # hello world + # HEREDOC # # And should be written as: # - # ``` - # <<-HERERDOC - # hello world - # HEREDOC + # <<-HEREDOC + # hello world + # HEREDOC # - # <<-HERERDOC - # hello world - # HEREDOC - # ``` + # <<-HEREDOC + # hello world + # HEREDOC + # + # The `IndentBy` configuration option changes the enforced indentation level + # of the _heredoc_. # - # The `IndentBy` configuration option changes the enforced indentation level of the _heredoc_. + # YAML configuration example: # # ``` # Style/HeredocIndent: From 44f5f24cb43de4f8744b84441876dc4e818667ce Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 02:43:35 +0100 Subject: [PATCH 20/42] Cleanup doc comments in `Crystal::Location` extensions --- src/ameba/ext/location.cr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ameba/ext/location.cr b/src/ameba/ext/location.cr index 88404b8de..6d5c97f56 100644 --- a/src/ameba/ext/location.cr +++ b/src/ameba/ext/location.cr @@ -1,13 +1,13 @@ -# Extensions to Crystal::Location +# 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). + # 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). + # 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 From fdefe164b09f2785efa189e670b0a13834aa0c72 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 02:44:01 +0100 Subject: [PATCH 21/42] Simplify code in `Style/HeredocIndent` rule --- src/ameba/rule/style/heredoc_indent.cr | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/ameba/rule/style/heredoc_indent.cr b/src/ameba/rule/style/heredoc_indent.cr index 6b491527e..d8d699b9a 100644 --- a/src/ameba/rule/style/heredoc_indent.cr +++ b/src/ameba/rule/style/heredoc_indent.cr @@ -39,25 +39,24 @@ module Ameba::Rule::Style indent_by 2 end - MSG = "Heredoc body should be indented by %s spaces" + MSG = "Heredoc body should be indented by %d spaces" def test(source, node : Crystal::StringInterpolation) - return unless start_location = node.location + return unless location = node.location - start_location_pos = source.pos(start_location) - return unless source.code[start_location_pos..(start_location_pos + 2)]? == "<<-" + location_pos = source.pos(location) + return unless source.code[location_pos..(location_pos + 2)]? == "<<-" - correct_indent = line_indent(source, start_location) + indent_by + correct_indent = line_indent(source, location) + indent_by + return if node.heredoc_indent == correct_indent - unless node.heredoc_indent == correct_indent - issue_for node, MSG % indent_by - end + issue_for node, MSG % indent_by end - private def line_indent(source, start_location) : Int32 - line_location = Crystal::Location.new(nil, start_location.line_number, 1) + private def line_indent(source, location) : Int32 + line_location = location.with(column_number: 1) line_location_pos = source.pos(line_location) - line = source.code[line_location_pos..(line_location_pos + start_location.column_number)] + line = source.code[line_location_pos..(line_location_pos + location.column_number)] line.size - line.lstrip.size end From f9947d4aeceb2b2af7c7c272b05d1a93c102c467 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 03:14:25 +0100 Subject: [PATCH 22/42] Simplify code in `Lint/AmbiguousAssignment` rule --- src/ameba/rule/lint/ambiguous_assignment.cr | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ameba/rule/lint/ambiguous_assignment.cr b/src/ameba/rule/lint/ambiguous_assignment.cr index e9c371a25..03e0b1065 100644 --- a/src/ameba/rule/lint/ambiguous_assignment.cr +++ b/src/ameba/rule/lint/ambiguous_assignment.cr @@ -38,11 +38,7 @@ module Ameba::Rule::Lint def test(source, node : Crystal::Assign) return unless op_end_location = node.value.location - op_location = Crystal::Location.new( - op_end_location.filename, - op_end_location.line_number, - op_end_location.column_number - 1 - ) + op_location = op_end_location.adjust(column_number: -1) op_text = source_between(op_location, op_end_location, source.lines) return unless op_text From e6faeca0d01c661dbe9ad18b627507b52ba26fac Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 21:17:56 +0100 Subject: [PATCH 23/42] Remove unused code --- src/ameba/rule/lint/unused_self_access.cr | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ameba/rule/lint/unused_self_access.cr b/src/ameba/rule/lint/unused_self_access.cr index 3a5624f0f..3b88eeb13 100644 --- a/src/ameba/rule/lint/unused_self_access.cr +++ b/src/ameba/rule/lint/unused_self_access.cr @@ -32,10 +32,6 @@ module Ameba::Rule::Lint AST::ImplicitReturnVisitor.new(self, source) end - def test(source, node : Crystal::Self, in_macro : Bool) - issue_for node, MSG - end - def test(source, node : Crystal::Var, in_macro : Bool) issue_for node, MSG if node.name == "self" end From 1c043e8ca62dced861a94d778962cd94f74b68aa Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 21:19:38 +0100 Subject: [PATCH 24/42] Add backticks around a keyword in rule description --- src/ameba/rule/lint/unused_self_access.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/lint/unused_self_access.cr b/src/ameba/rule/lint/unused_self_access.cr index 3b88eeb13..c849c69ed 100644 --- a/src/ameba/rule/lint/unused_self_access.cr +++ b/src/ameba/rule/lint/unused_self_access.cr @@ -23,7 +23,7 @@ module Ameba::Rule::Lint class UnusedSelfAccess < Base properties do since_version "1.7.0" - description "Disallows unused self" + description "Disallows unused `self`" end MSG = "`self` is not used" From 0b64799706bc4eeab8802a10b9a461851f1bdb48 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 21:33:50 +0100 Subject: [PATCH 25/42] Call `#accept_children(visitor)` instead --- .../ast/visitors/implicit_return_visitor.cr | 36 +++++-------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr index 6eb222267..4dbf99168 100644 --- a/src/ameba/ast/visitors/implicit_return_visitor.cr +++ b/src/ameba/ast/visitors/implicit_return_visitor.cr @@ -56,13 +56,7 @@ module Ameba::AST def visit(node : Crystal::Call) : Bool report_implicit_return(node) - incr_stack do - node.obj.try &.accept(self) - node.args.each &.accept(self) - node.named_args.try &.each &.accept(self) - node.block_arg.try &.accept(self) - node.block.try &.accept(self) - end + incr_stack { node.accept_children(self) } false end @@ -168,10 +162,7 @@ module Ameba::AST def visit(node : Crystal::FunDef) : Bool report_implicit_return(node) - incr_stack do - node.args.each &.accept(self) - node.body.try &.accept(self) - end + incr_stack { node.accept_children(self) } false end @@ -187,7 +178,7 @@ module Ameba::AST def visit(node : Crystal::UnaryExpression) : Bool report_implicit_return(node) - incr_stack { node.exp.accept(self) } + incr_stack { node.accept_children(self) } false end @@ -195,10 +186,7 @@ module Ameba::AST def visit(node : Crystal::Annotation) : Bool report_implicit_return(node) - incr_stack do - node.args.each &.accept(self) - node.named_args.try &.each &.accept(self) - end + incr_stack { node.accept_children(self) } false end @@ -222,9 +210,7 @@ module Ameba::AST def visit(node : Crystal::StringInterpolation) : Bool report_implicit_return(node) - node.expressions.each do |exp| - incr_stack { exp.accept(self) } - end + incr_stack { node.accept_children(self) } false end @@ -250,8 +236,7 @@ module Ameba::AST def visit(node : Crystal::Select) : Bool report_implicit_return(node) - node.whens.each &.accept(self) - node.else.try &.accept(self) + node.accept_children(self) false end @@ -303,7 +288,7 @@ module Ameba::AST def visit(node : Crystal::ControlExpression) : Bool report_implicit_return(node) - incr_stack { node.exp.try &.accept(self) } + incr_stack { node.accept_children(self) } false end @@ -311,10 +296,7 @@ module Ameba::AST def visit(node : Crystal::RangeLiteral) : Bool report_implicit_return(node) - incr_stack do - node.from.accept(self) - node.to.accept(self) - end + incr_stack { node.accept_children(self) } false end @@ -324,7 +306,7 @@ module Ameba::AST # Regex literals either contain string literals or string interpolations, # both of which are "captured" by the parent regex literal - incr_stack { node.value.accept(self) } + incr_stack { node.accept_children(self) } false end From 2eebf9916d70d539537c42f27ad6bd3d69a212bb Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 28 Feb 2025 21:34:11 +0100 Subject: [PATCH 26/42] Cleanup redundant definitions in `AST::ImplicitReturnVisitor` --- .../ast/visitors/implicit_return_visitor.cr | 45 +------------------ 1 file changed, 2 insertions(+), 43 deletions(-) diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr index 4dbf99168..11a129de4 100644 --- a/src/ameba/ast/visitors/implicit_return_visitor.cr +++ b/src/ameba/ast/visitors/implicit_return_visitor.cr @@ -311,15 +311,6 @@ module Ameba::AST false end - def visit( - node : Crystal::BoolLiteral | Crystal::CharLiteral | Crystal::NumberLiteral | - Crystal::StringLiteral | Crystal::SymbolLiteral | Crystal::ProcLiteral, - ) : Bool - report_implicit_return(node) - - true - end - def visit(node : Crystal::Yield) : Bool report_implicit_return(node) @@ -366,48 +357,16 @@ module Ameba::AST false end - def visit(node : Crystal::MacroVar) - false - end - - def visit(node : Crystal::Generic | Crystal::Path | Crystal::Union) : Bool - report_implicit_return(node) - - false - end - - def visit(node : Crystal::UninitializedVar) : Bool - report_implicit_return(node) - + def visit(node : Crystal::Alias | Crystal::TypeDef | Crystal::MacroVar) false end - def visit(node : Crystal::OffsetOf) : Bool + def visit(node : Crystal::Generic | Crystal::Path | Crystal::Union | Crystal::UninitializedVar | Crystal::OffsetOf | Crystal::LibDef | Crystal::Include | Crystal::Extend) : Bool report_implicit_return(node) false end - def visit(node : Crystal::LibDef) : Bool - report_implicit_return(node) - - false - end - - def visit(node : Crystal::Include | Crystal::Extend) : Bool - report_implicit_return(node) - - false - end - - def visit(node : Crystal::Alias) - false - end - - def visit(node : Crystal::TypeDef) - false - end - def visit(node) report_implicit_return(node) From 362a032f5e921c65f15a6cc3f1b4432d0e2878e1 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 1 Mar 2025 01:08:57 +0100 Subject: [PATCH 27/42] Simplify code in `AST::ImplicitReturnVisitor` --- src/ameba/ast/visitors/implicit_return_visitor.cr | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr index 11a129de4..32ff12a33 100644 --- a/src/ameba/ast/visitors/implicit_return_visitor.cr +++ b/src/ameba/ast/visitors/implicit_return_visitor.cr @@ -40,14 +40,12 @@ module Ameba::AST def visit(node : Crystal::BinaryOp) : Bool report_implicit_return(node) - if node.right.is_a?(Crystal::Call) || - node.right.is_a?(Crystal::Expressions) || - node.right.is_a?(Crystal::ControlExpression) + case node.right + when Crystal::Call, Crystal::Expressions, Crystal::ControlExpression incr_stack { node.left.accept(self) } else node.left.accept(self) end - node.right.accept(self) false From 0c87d17eb2915fd2660ce588a26819b463a95bfa Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 1 Mar 2025 01:14:21 +0100 Subject: [PATCH 28/42] =?UTF-8?q?Add=20`begin`=E2=80=A6`ensure`=20to=20be?= =?UTF-8?q?=20on=20the=20safe=20side?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ameba/ast/visitors/implicit_return_visitor.cr | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/ameba/ast/visitors/implicit_return_visitor.cr b/src/ameba/ast/visitors/implicit_return_visitor.cr index 32ff12a33..cf242aaa1 100644 --- a/src/ameba/ast/visitors/implicit_return_visitor.cr +++ b/src/ameba/ast/visitors/implicit_return_visitor.cr @@ -379,21 +379,28 @@ module Ameba::AST private def incr_stack(&) : Nil @stack += 1 yield + ensure @stack -= 1 end private def swap_stack(& : Int32 -> Nil) : Nil old_stack = @stack @stack = 0 - yield old_stack - @stack = old_stack + begin + yield old_stack + ensure + @stack = old_stack + end end private def in_macro(&) : Nil old_value = @in_macro @in_macro = true - yield - @in_macro = old_value + begin + yield + ensure + @in_macro = old_value + end end end end From c3e152a8e9b484f251de6377fdd7fbc55c4b7a04 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 1 Mar 2025 02:05:55 +0100 Subject: [PATCH 29/42] Stylistic refactors --- spec/ameba/rule/lint/bad_directive_spec.cr | 2 +- src/ameba/ast/util.cr | 2 +- src/ameba/rule/lint/bad_directive.cr | 27 ++++++++++--------- .../rule/lint/unneeded_disable_directive.cr | 9 +++---- src/ameba/rule/style/multiline_curly_block.cr | 6 ++--- src/ameba/severity.cr | 2 +- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/spec/ameba/rule/lint/bad_directive_spec.cr b/spec/ameba/rule/lint/bad_directive_spec.cr index 29e7a1625..e9ff89f74 100644 --- a/spec/ameba/rule/lint/bad_directive_spec.cr +++ b/spec/ameba/rule/lint/bad_directive_spec.cr @@ -13,7 +13,7 @@ module Ameba::Rule::Lint it "reports if there is incorrect action" do expect_issue subject, <<-CRYSTAL # ameba:foo Lint/BadDirective - # ^^^ error: Bad action in comment directive: 'foo'. Possible values: disable, enable + # ^^^ error: Bad action in comment directive: `foo`. Possible values: `disable`, `enable` CRYSTAL end diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 8d2453cfd..984e0375a 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -188,7 +188,7 @@ module Ameba::AST::Util when Crystal::While, Crystal::Until true when Crystal::Call - node.name == "loop" && node.args.size == 0 && node.obj.nil? + node.name == "loop" && node.args.empty? && node.obj.nil? else false end diff --git a/src/ameba/rule/lint/bad_directive.cr b/src/ameba/rule/lint/bad_directive.cr index e25fc8621..64cd29eb5 100644 --- a/src/ameba/rule/lint/bad_directive.cr +++ b/src/ameba/rule/lint/bad_directive.cr @@ -23,6 +23,9 @@ module Ameba::Rule::Lint description "Reports bad comment directives" end + MSG_INVALID_ACTION = "Bad action in comment directive: `%s`. Possible values: %s" + MSG_NONEXISTENT_RULES = "Such rules do not exist: %s" + AVAILABLE_ACTIONS = InlineComments::Action.names.map(&.downcase) ALL_RULE_NAMES = Rule.rules.map(&.rule_name) ALL_GROUP_NAMES = Rule.rules.map(&.group_name).uniq! @@ -41,14 +44,12 @@ module Ameba::Rule::Lint return if InlineComments::Action.parse?(action) # See `InlineComments::COMMENT_DIRECTIVE_REGEX` - start_location = token.location.adjust(column_number: {{ "# ameba:".size }}) - token_location = { - start_location, - start_location.adjust(column_number: action.size - 1), - } - issue_for *token_location, - "Bad action in comment directive: '%s'. Possible values: %s" % { - action, AVAILABLE_ACTIONS.join(", "), + location = token.location.adjust(column_number: {{ "# ameba:".size }}) + end_location = location.adjust(column_number: action.size - 1) + + issue_for location, end_location, + MSG_INVALID_ACTION % { + action, AVAILABLE_ACTIONS.map { |name| "`#{name}`" }.join(", "), } end @@ -56,11 +57,11 @@ module Ameba::Rule::Lint bad_names = rules - ALL_RULE_NAMES - ALL_GROUP_NAMES return if bad_names.empty? - token_location = { - token.location, - token.location.adjust(column_number: token.value.to_s.size - 1), - } - issue_for *token_location, "Such rules do not exist: %s" % bad_names.join(", ") + location = token.location + end_location = location.adjust(column_number: token.value.to_s.size - 1) + + issue_for location, end_location, + MSG_NONEXISTENT_RULES % bad_names.join(", ") end end end diff --git a/src/ameba/rule/lint/unneeded_disable_directive.cr b/src/ameba/rule/lint/unneeded_disable_directive.cr index 6080af754..4eede517e 100644 --- a/src/ameba/rule/lint/unneeded_disable_directive.cr +++ b/src/ameba/rule/lint/unneeded_disable_directive.cr @@ -39,11 +39,10 @@ module Ameba::Rule::Lint next unless names = unneeded_disables(source, directive, token.location) next if names.empty? - token_location = { - token.location, - token.location.adjust(column_number: token.value.to_s.size - 1), - } - issue_for *token_location, MSG % names.join(", ") + location = token.location + end_location = location.adjust(column_number: token.value.to_s.size - 1) + + issue_for location, end_location, MSG % names.join(", ") end end diff --git a/src/ameba/rule/style/multiline_curly_block.cr b/src/ameba/rule/style/multiline_curly_block.cr index 45c2130ce..69b5db683 100644 --- a/src/ameba/rule/style/multiline_curly_block.cr +++ b/src/ameba/rule/style/multiline_curly_block.cr @@ -35,10 +35,10 @@ module Ameba::Rule::Style 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 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)]? == '{' + return if location.line_number == end_location.line_number + return unless source.code[source.pos(location)]? == '{' issue_for node, MSG end diff --git a/src/ameba/severity.cr b/src/ameba/severity.cr index 52dd74550..622840982 100644 --- a/src/ameba/severity.cr +++ b/src/ameba/severity.cr @@ -41,7 +41,7 @@ module Ameba def self.parse(name : String) super name rescue ArgumentError - raise "Incorrect severity name #{name}. Try one of: #{values.map(&.to_s).join(", ")}" + raise "Incorrect severity name #{name}. Try one of: #{values.join(", ")}" end end From 55b136ae426881565f471ad473c41085e32edc1d Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 1 Mar 2025 17:36:40 +0100 Subject: [PATCH 30/42] Cleanup `Config#update_rules` API --- src/ameba/cli/cmd.cr | 4 +++- src/ameba/config.cr | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 16732b906..97445d9fa 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -185,7 +185,9 @@ module Ameba::Cli when opts.all? config.rules.each(&.enabled = true) end - config.update_rules(opts.except, enabled: false) + if except = opts.except + config.update_rules(except, enabled: false) + end end private def configure_formatter(config, opts) : Nil diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 1a470c522..0878aa233 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -247,8 +247,8 @@ class Ameba::Config # ``` # config.update_rules %w[Group1 Group2], enabled: true # ``` - def update_rules(names, enabled = true, excluded = nil) - names.try &.each do |name| + def update_rules(names : Enumerable(String), enabled = true, excluded = nil) + names.each do |name| if rules = @rule_groups[name]? rules.each do |rule| rule.enabled = enabled From 98b76d7c8c5369e6fd4346023130f4ba6be54aad Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 3 Mar 2025 14:55:24 +0100 Subject: [PATCH 31/42] Cleanup `Source::Rewriter::Action` a bit --- src/ameba/source/rewriter/action.cr | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ameba/source/rewriter/action.cr b/src/ameba/source/rewriter/action.cr index ceffcf0a1..34c582519 100644 --- a/src/ameba/source/rewriter/action.cr +++ b/src/ameba/source/rewriter/action.cr @@ -43,6 +43,7 @@ class Ameba::Source::Rewriter def ordered_replacements replacement = @replacement + reps = [] of {Int32, Int32, String} reps << {@begin_pos, @begin_pos, @insert_before} unless @insert_before.empty? reps << {@begin_pos, @end_pos, replacement} if replacement @@ -126,14 +127,19 @@ class Ameba::Source::Rewriter # # In case a child has equal range to *action*, it is returned as `:parent` # - # Reminder: an empty range 1...1 is considered disjoint from 1...10 + # NOTE: an empty range `1...1` is considered disjoint from `1...10` protected def analyze_hierarchy(action) # left_index is the index of the first child that isn't completely to the left of action - left_index = bsearch_child_index { |child| child.end_pos > action.begin_pos } + left_index = bsearch_child_index do |child| + child.end_pos > action.begin_pos + end # right_index is the index of the first child that is completely on the right of action start = left_index == 0 ? 0 : left_index - 1 # See "corner case" below for reason of -1 - right_index = bsearch_child_index(start) { |child| child.begin_pos >= action.end_pos } + right_index = bsearch_child_index(start) do |child| + child.begin_pos >= action.end_pos + end center = right_index - left_index + case center when 0 # All children are disjoint from action, nothing else to do @@ -149,8 +155,8 @@ class Ameba::Source::Rewriter overlap_left = @children[left_index].begin_pos <=> action.begin_pos overlap_right = @children[right_index - 1].end_pos <=> action.end_pos - raise "Unable to compare begin pos" if overlap_left.nil? - raise "Unable to compare end pos" if overlap_right.nil? + raise "Unable to compare begin pos" unless overlap_left + raise "Unable to compare end pos" unless overlap_right # For one child to be the parent of action, we must have: if center == 1 && overlap_left <= 0 && overlap_right >= 0 From c436ac0e5a531f6f1f548dd6ff2e893015d57d1c Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 3 Mar 2025 15:09:40 +0100 Subject: [PATCH 32/42] Reorder method definitions in `Style/VerboseBlock` --- src/ameba/rule/style/verbose_block.cr | 248 +++++++++++++------------- 1 file changed, 124 insertions(+), 124 deletions(-) diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr index f7294d413..a032f4a9e 100644 --- a/src/ameba/rule/style/verbose_block.cr +++ b/src/ameba/rule/style/verbose_block.cr @@ -47,87 +47,86 @@ module Ameba::Rule::Style MSG = "Use short block notation instead: `%s`" CALL_PATTERN = "%s(%s&.%s)" - private def same_location_lines?(a, b) - return unless a_location = name_location(a) - return unless b_location = b.location - - a_location.line_number == b_location.line_number - end - private PREFIX_OPERATORS = {"+", "-", "~"} private OPERATOR_CHARS = {'[', ']', '!', '=', '>', '<', '~', '+', '-', '*', '/', '%', '^', '|', '&'} - private def prefix_operator?(node) - node.name.in?(PREFIX_OPERATORS) && node.args.empty? - end + def test(source, node : Crystal::Call) + # we are interested only in calls with block taking a single argument + # + # ``` + # (1..3).any? { |i| i.to_i64.odd? } + # ^--- ^ ^------------ + # block arg body + # ``` + return unless (block = node.block) && block.args.size == 1 - private def operator?(name) - !name.empty? && name[0].in?(OPERATOR_CHARS) - end + arg = block.args.first - private def setter?(name) - !name.empty? && name[0].letter? && name.ends_with?('=') - end + # we filter out the blocks that are of call type - `i.to_i64.odd?` + return unless (body = block.body).is_a?(Crystal::Call) - private def valid_length?(code) - if max_length = self.max_length - return code.size <= max_length + # we need to "unwind" the call chain, so the final receiver object + # ends up being a variable - `i` + obj = body.obj + while obj.is_a?(Crystal::Call) + obj = obj.obj end - true - end - private def valid_line_length?(node, code) - if max_line_length = self.max_line_length - if location = name_location(node) - final_line_length = location.column_number + code.size - return final_line_length <= max_line_length - end - end - true + # only calls with a first argument used as a receiver are the valid game + return unless obj == arg + + # we bail out if the block node include the block argument + return if reference_count(body, arg) > 1 + + # add issue if the given nodes pass all of the checks + issue_for_valid source, node, block, body end - private def reference_count(node, obj : Crystal::Var) - i = 0 - case node - when Crystal::Call - i += reference_count(node.obj, obj) - i += reference_count(node.block, obj) + # ameba:disable Metrics/CyclomaticComplexity + private def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call) + return if exclude_calls_with_block? && body.block + return if exclude_multiple_line_blocks? && !same_location_lines?(call, body) + return if exclude_prefix_operators? && prefix_operator?(body) + return if exclude_operators? && operator?(body.name) + return if exclude_setters? && setter?(body.name) - node.args.each do |arg| - i += reference_count(arg, obj) - end - node.named_args.try &.each do |arg| - i += reference_count(arg.value, obj) + call_code = + call_code(source, call, body) + + return unless valid_line_length?(call, call_code) + return unless valid_length?(call_code) + + return unless location = name_location(call) + return unless end_location = block.end_location + + 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 - when Crystal::BinaryOp - i += reference_count(node.left, obj) - i += reference_count(node.right, obj) - when Crystal::Block - i += reference_count(node.body, obj) - when Crystal::Var - i += 1 if node == obj end - i end - private def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false) : Nil - args = node.args.dup - args.pop? if skip_last_arg - args.join io, ", " + private def call_code(source, call, body) + args = String.build { |io| args_to_s(io, call) }.presence + args += ", " if 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 + call_chain = %w[].tap do |arr| + obj = body.obj + while obj.is_a?(Crystal::Call) + arr << node_to_s(source, obj) + obj = obj.obj end + arr.reverse! + arr << node_to_s(source, body) end - if short_block - io << ", " unless args.empty? && (named_args.nil? || named_args.empty?) - io << short_block - end + name = + call_chain.join('.') + + CALL_PATTERN % {call.name, args, name} end private def node_to_s(source, node : Crystal::Call) @@ -158,6 +157,25 @@ module Ameba::Rule::Style end end + private def args_to_s(io : IO, node : Crystal::Call, short_block = nil, skip_last_arg = false) : Nil + args = node.args.dup + args.pop? if skip_last_arg + args.join io, ", " + + 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 + private def short_block_code(source, node : Crystal::Call) return unless block = node.block return unless block_location = block.location @@ -167,82 +185,64 @@ module Ameba::Rule::Style block_code if block_code.try(&.starts_with?("&.")) end - private 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(source, obj) - obj = obj.obj - end - arr.reverse! - arr << node_to_s(source, body) - end - - name = - call_chain.join('.') + private def same_location_lines?(a, b) + return unless a_location = name_location(a) + return unless b_location = b.location - CALL_PATTERN % {call.name, args, name} + a_location.line_number == b_location.line_number end - # ameba:disable Metrics/CyclomaticComplexity - private def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call) - return if exclude_calls_with_block? && body.block - return if exclude_multiple_line_blocks? && !same_location_lines?(call, body) - return if exclude_prefix_operators? && prefix_operator?(body) - return if exclude_operators? && operator?(body.name) - return if exclude_setters? && setter?(body.name) + private def prefix_operator?(node) + node.name.in?(PREFIX_OPERATORS) && node.args.empty? + end - call_code = - call_code(source, call, body) + private def operator?(name) + !name.empty? && name[0].in?(OPERATOR_CHARS) + end - return unless valid_line_length?(call, call_code) - return unless valid_length?(call_code) + private def setter?(name) + !name.empty? && name[0].letter? && name.ends_with?('=') + end - return unless location = name_location(call) - return unless end_location = block.end_location + private def valid_length?(code) + if max_length = self.max_length + return code.size <= max_length + end + true + end - 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) + private def valid_line_length?(node, code) + if max_line_length = self.max_line_length + if location = name_location(node) + final_line_length = location.column_number + code.size + return final_line_length <= max_line_length end end + true end - def test(source, node : Crystal::Call) - # we are interested only in calls with block taking a single argument - # - # ``` - # (1..3).any? { |i| i.to_i64.odd? } - # ^--- ^ ^------------ - # block arg body - # ``` - return unless (block = node.block) && block.args.size == 1 - - arg = block.args.first - - # we filter out the blocks that are of call type - `i.to_i64.odd?` - return unless (body = block.body).is_a?(Crystal::Call) + private def reference_count(node, obj : Crystal::Var) + i = 0 + case node + when Crystal::Call + i += reference_count(node.obj, obj) + i += reference_count(node.block, obj) - # we need to "unwind" the call chain, so the final receiver object - # ends up being a variable - `i` - obj = body.obj - while obj.is_a?(Crystal::Call) - obj = obj.obj + node.args.each do |arg| + i += reference_count(arg, obj) + end + node.named_args.try &.each do |arg| + i += reference_count(arg.value, obj) + end + when Crystal::BinaryOp + i += reference_count(node.left, obj) + i += reference_count(node.right, obj) + when Crystal::Block + i += reference_count(node.body, obj) + when Crystal::Var + i += 1 if node == obj end - - # only calls with a first argument used as a receiver are the valid game - return unless obj == arg - - # we bail out if the block node include the block argument - return if reference_count(body, arg) > 1 - - # add issue if the given nodes pass all of the checks - issue_for_valid source, node, block, body + i end end end From 1d3e1f20a182dcebe1e6f179b7c05378075c59ad Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Mar 2025 01:11:29 +0100 Subject: [PATCH 33/42] Tweak reported location in `Documentation/DocumentationAdmonition` rule --- .../documentation_admonition_spec.cr | 24 ++++++++++++------- .../documentation/documentation_admonition.cr | 6 +++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/spec/ameba/rule/documentation/documentation_admonition_spec.cr b/spec/ameba/rule/documentation/documentation_admonition_spec.cr index 53d82564c..79cffada2 100644 --- a/spec/ameba/rule/documentation/documentation_admonition_spec.cr +++ b/spec/ameba/rule/documentation/documentation_admonition_spec.cr @@ -18,21 +18,27 @@ module Ameba::Rule::Documentation it "fails for comments with admonition" do subject.admonitions.each do |admonition| expect_issue subject, <<-CRYSTAL, admonition: admonition - # %{admonition}: Single-line comment - # ^{admonition} error: Found a %{admonition} admonition in a comment + def foo + # %{admonition}: Single-line comment + # ^{admonition} error: Found a %{admonition} admonition in a comment + end CRYSTAL expect_issue subject, <<-CRYSTAL, admonition: admonition - # Text before ... - # %{admonition}(some context): Part of multi-line comment - # ^{admonition} error: Found a %{admonition} admonition in a comment - # Text after ... + def foo + # Text before ... + # %{admonition}(some context): Part of multi-line comment + # ^{admonition} error: Found a %{admonition} admonition in a comment + # Text after ... + end CRYSTAL expect_issue subject, <<-CRYSTAL, admonition: admonition - # %{admonition} - # ^{admonition} error: Found a %{admonition} admonition in a comment - if rand > 0.5 + def foo + # %{admonition} + # ^{admonition} error: Found a %{admonition} admonition in a comment + if rand > 0.5 + end end CRYSTAL end diff --git a/src/ameba/rule/documentation/documentation_admonition.cr b/src/ameba/rule/documentation/documentation_admonition.cr index 615cfa0d7..b408e88aa 100644 --- a/src/ameba/rule/documentation/documentation_admonition.cr +++ b/src/ameba/rule/documentation/documentation_admonition.cr @@ -62,9 +62,11 @@ module Ameba::Rule::Documentation matches.each do |match| admonition = match["admonition"] + begin_location = + token.location.adjust(column_number: 2) # adjust for "# " end_location = - token.location.adjust(column_number: admonition.size + 1) - token_location = {token.location, end_location} + begin_location.adjust(column_number: admonition.size - 1) + token_location = {begin_location, end_location} begin case expr = match["context"]?.presence From fb1a8218c692f3b33e94dbbf2dce7b3189483a19 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Mar 2025 01:12:11 +0100 Subject: [PATCH 34/42] Fix spec to use interpolation-less variant of heredoc --- spec/ameba/rule/lint/literal_in_interpolation_spec.cr | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/ameba/rule/lint/literal_in_interpolation_spec.cr b/spec/ameba/rule/lint/literal_in_interpolation_spec.cr index 5572d8848..4c89fd754 100644 --- a/spec/ameba/rule/lint/literal_in_interpolation_spec.cr +++ b/spec/ameba/rule/lint/literal_in_interpolation_spec.cr @@ -5,12 +5,9 @@ module Ameba::Rule::Lint subject = LiteralInInterpolation.new it "passes with good interpolation examples" do - expect_no_issues subject, <<-CRYSTAL - name = "Ary" + expect_no_issues subject, <<-'CRYSTAL' "Hello, #{name}" - "#{name}" - "Name size: #{name.size}" CRYSTAL end From bf04db8075ba729e9f27bedb8dd426971c53ba08 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Mar 2025 02:27:43 +0100 Subject: [PATCH 35/42] Slight test case refactor --- .../rule/style/percent_literal_delimiters_spec.cr | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/ameba/rule/style/percent_literal_delimiters_spec.cr b/spec/ameba/rule/style/percent_literal_delimiters_spec.cr index 520038026..0dbe412a1 100644 --- a/spec/ameba/rule/style/percent_literal_delimiters_spec.cr +++ b/spec/ameba/rule/style/percent_literal_delimiters_spec.cr @@ -60,10 +60,12 @@ module Ameba::Rule::Style it "reports rule, location and message" do expect_issue subject, <<-CRYSTAL - puts %[one two three] - # ^ error: `%`-literals should be delimited by `(` and `)` - puts %w(one two three) - # ^^ error: `%w`-literals should be delimited by `[` and `]` + def foo + %[one two three] + # ^ error: `%`-literals should be delimited by `(` and `)` + %w(one two three) + # ^^ error: `%w`-literals should be delimited by `[` and `]` + end CRYSTAL end From 8bea2495ae88aae9e7cf3ffa973b13030da833a2 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 14 Mar 2025 04:49:29 +0100 Subject: [PATCH 36/42] Add missing `:` in YAML configuration examples --- src/ameba/rule/lint/empty_ensure.cr | 2 +- src/ameba/rule/lint/literal_in_interpolation.cr | 2 +- src/ameba/rule/lint/redundant_string_coercion.cr | 2 +- src/ameba/rule/performance/first_last_after_filter.cr | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ameba/rule/lint/empty_ensure.cr b/src/ameba/rule/lint/empty_ensure.cr index 122b8fd57..e8adc52ef 100644 --- a/src/ameba/rule/lint/empty_ensure.cr +++ b/src/ameba/rule/lint/empty_ensure.cr @@ -34,7 +34,7 @@ module Ameba::Rule::Lint # YAML configuration example: # # ``` - # Lint/EmptyEnsure + # Lint/EmptyEnsure: # Enabled: true # ``` class EmptyEnsure < Base diff --git a/src/ameba/rule/lint/literal_in_interpolation.cr b/src/ameba/rule/lint/literal_in_interpolation.cr index 237bc9fdf..914d78117 100644 --- a/src/ameba/rule/lint/literal_in_interpolation.cr +++ b/src/ameba/rule/lint/literal_in_interpolation.cr @@ -12,7 +12,7 @@ module Ameba::Rule::Lint # YAML configuration example: # # ``` - # Lint/LiteralInInterpolation + # Lint/LiteralInInterpolation: # Enabled: true # ``` class LiteralInInterpolation < Base diff --git a/src/ameba/rule/lint/redundant_string_coercion.cr b/src/ameba/rule/lint/redundant_string_coercion.cr index ba39cb0ba..7d4940f07 100644 --- a/src/ameba/rule/lint/redundant_string_coercion.cr +++ b/src/ameba/rule/lint/redundant_string_coercion.cr @@ -17,7 +17,7 @@ module Ameba::Rule::Lint # YAML configuration example: # # ``` - # Lint/RedundantStringCoercion + # Lint/RedundantStringCoercion: # Enabled: true # ``` class RedundantStringCoercion < Base diff --git a/src/ameba/rule/performance/first_last_after_filter.cr b/src/ameba/rule/performance/first_last_after_filter.cr index c260bb55e..5c067b4ea 100644 --- a/src/ameba/rule/performance/first_last_after_filter.cr +++ b/src/ameba/rule/performance/first_last_after_filter.cr @@ -20,7 +20,7 @@ module Ameba::Rule::Performance # YAML configuration example: # # ``` - # Performance/FirstLastAfterFilter + # Performance/FirstLastAfterFilter: # Enabled: true # FilterNames: # - select From 2a04355b8d2c95629b202b71250bf92e33bbc95d Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 23 Mar 2025 00:53:54 +0100 Subject: [PATCH 37/42] Apply suggestions from code review Co-authored-by: Vitalii Elenhaupt <3624712+veelenga@users.noreply.github.com> --- spec/ameba/ast/visitors/node_visitor_spec.cr | 2 +- spec/ameba/rule/style/parentheses_around_condition_spec.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ameba/ast/visitors/node_visitor_spec.cr b/spec/ameba/ast/visitors/node_visitor_spec.cr index 667e703d2..989580e3d 100644 --- a/spec/ameba/ast/visitors/node_visitor_spec.cr +++ b/spec/ameba/ast/visitors/node_visitor_spec.cr @@ -5,7 +5,7 @@ module Ameba::AST describe "visit" do it "allow to visit ASTNode" do rule = DummyRule.new - visitor = NodeVisitor.new rule, Source.new "" + visitor = NodeVisitor.new rule, Source.new("") nodes = Crystal::Parser.new("").parse nodes.accept visitor diff --git a/spec/ameba/rule/style/parentheses_around_condition_spec.cr b/spec/ameba/rule/style/parentheses_around_condition_spec.cr index a6ce82bb0..597723500 100644 --- a/spec/ameba/rule/style/parentheses_around_condition_spec.cr +++ b/spec/ameba/rule/style/parentheses_around_condition_spec.cr @@ -65,7 +65,7 @@ module Ameba::Rule::Style CRYSTAL end - it "allows to skips ternary control expressions" do + it "allows to skip ternary control expressions" do rule = ParenthesesAroundCondition.new rule.exclude_ternary = true From ce5fcedf949b60f0a50231380d3d40dd90fc791b Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sat, 5 Apr 2025 19:17:49 +0200 Subject: [PATCH 38/42] Cleanup test case --- spec/ameba/rule/lint/spec_focus_spec.cr | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/ameba/rule/lint/spec_focus_spec.cr b/spec/ameba/rule/lint/spec_focus_spec.cr index 43cb80346..5c4a55187 100644 --- a/spec/ameba/rule/lint/spec_focus_spec.cr +++ b/spec/ameba/rule/lint/spec_focus_spec.cr @@ -78,11 +78,10 @@ module Ameba::Rule::Lint subject.catch(s).should be_valid end - it "does not report if is a parameterized focused spec item" do + it "does not report if there is a parameterized focused spec item" do s = Source.new %( def assert_foo(focus = false) - it "foo", focus: focus do - end + it "foo", focus: focus { yield } end ), path: "source_spec.cr" From 8f889c7be769574805d9445d65d9ad127297c84e Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 7 Apr 2025 22:23:31 +0200 Subject: [PATCH 39/42] Make `@code` argument passed to `Source#initialize` optional --- spec/ameba/ast/visitors/node_visitor_spec.cr | 2 +- ...dundant_control_expression_visitor_spec.cr | 2 +- spec/ameba/base_spec.cr | 8 ++++---- .../formatter/disabled_formatter_spec.cr | 6 +++--- spec/ameba/formatter/dot_formatter_spec.cr | 14 ++++++------- .../formatter/flycheck_formatter_spec.cr | 2 +- .../github_actions_formatter_spec.cr | 10 +++++----- spec/ameba/formatter/json_formatter_spec.cr | 20 +++++++++---------- spec/ameba/formatter/todo_formatter_spec.cr | 4 ++-- spec/ameba/reportable_spec.cr | 8 ++++---- spec/ameba/rule/base_spec.cr | 2 +- spec/ameba/rule/performance/base_spec.cr | 4 ++-- spec/ameba/runner_spec.cr | 4 ++-- spec/ameba/source_spec.cr | 12 +++++------ src/ameba/source.cr | 2 +- 15 files changed, 50 insertions(+), 50 deletions(-) diff --git a/spec/ameba/ast/visitors/node_visitor_spec.cr b/spec/ameba/ast/visitors/node_visitor_spec.cr index 989580e3d..0ab454cb1 100644 --- a/spec/ameba/ast/visitors/node_visitor_spec.cr +++ b/spec/ameba/ast/visitors/node_visitor_spec.cr @@ -5,7 +5,7 @@ module Ameba::AST describe "visit" do it "allow to visit ASTNode" do rule = DummyRule.new - visitor = NodeVisitor.new rule, Source.new("") + visitor = NodeVisitor.new rule, Source.new nodes = Crystal::Parser.new("").parse nodes.accept visitor diff --git a/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr b/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr index 19ec13562..6c08eeb7d 100644 --- a/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr +++ b/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr @@ -2,7 +2,7 @@ require "../../../spec_helper" module Ameba::AST describe RedundantControlExpressionVisitor do - source = Source.new "" + source = Source.new rule = RedundantControlExpressionRule.new node = as_node <<-CRYSTAL diff --git a/spec/ameba/base_spec.cr b/spec/ameba/base_spec.cr index 2502a0592..cda5b9773 100644 --- a/spec/ameba/base_spec.cr +++ b/spec/ameba/base_spec.cr @@ -46,19 +46,19 @@ module Ameba::Rule describe "#excluded?" do it "returns false if a rule does no have a list of excluded source" do - DummyRule.new.excluded?(Source.new "", "source.cr").should_not be_true + DummyRule.new.excluded?(Source.new path: "source.cr").should_not be_true end it "returns false if source is not excluded from this rule" do rule = DummyRule.new rule.excluded = %w[some_source.cr] - rule.excluded?(Source.new "", "another_source.cr").should_not be_true + rule.excluded?(Source.new path: "another_source.cr").should_not be_true end it "returns true if source is excluded from this rule" do rule = DummyRule.new rule.excluded = %w[source.cr] - rule.excluded?(Source.new "", "source.cr").should be_true + rule.excluded?(Source.new path: "source.cr").should be_true end it "returns true if source matches the wildcard" do @@ -70,7 +70,7 @@ module Ameba::Rule it "returns false if source does not match the wildcard" do rule = DummyRule.new rule.excluded = %w[*_spec.cr] - rule.excluded?(Source.new "", "source.cr").should be_false + rule.excluded?(Source.new path: "source.cr").should be_false end end diff --git a/spec/ameba/formatter/disabled_formatter_spec.cr b/spec/ameba/formatter/disabled_formatter_spec.cr index e2abbaaa6..a2294b8eb 100644 --- a/spec/ameba/formatter/disabled_formatter_spec.cr +++ b/spec/ameba/formatter/disabled_formatter_spec.cr @@ -11,7 +11,7 @@ module Ameba::Formatter describe "#finished" do it "writes a final message" do - subject.finished [Source.new ""] + subject.finished [Source.new] output.to_s.should contain "Disabled rules using inline directives:" end @@ -19,7 +19,7 @@ module Ameba::Formatter Colorize.enabled = false path = "source.cr" - s = Source.new("", path).tap do |source| + s = Source.new(path: path).tap do |source| source.add_issue(ErrorRule.new, {1, 2}, message: "ErrorRule", status: :disabled) source.add_issue(NamedRule.new, location: {2, 2}, message: "NamedRule", status: :disabled) end @@ -32,7 +32,7 @@ module Ameba::Formatter end it "does not write not-disabled rules" do - s = Source.new("", "source.cr").tap do |source| + s = Source.new(path: "source.cr").tap do |source| source.add_issue(ErrorRule.new, {1, 2}, "ErrorRule") source.add_issue(NamedRule.new, location: {2, 2}, message: "NamedRule", status: :disabled) diff --git a/spec/ameba/formatter/dot_formatter_spec.cr b/spec/ameba/formatter/dot_formatter_spec.cr index 4ede4cf75..e0ccfe9bf 100644 --- a/spec/ameba/formatter/dot_formatter_spec.cr +++ b/spec/ameba/formatter/dot_formatter_spec.cr @@ -11,19 +11,19 @@ module Ameba::Formatter describe "#started" do it "writes started message" do - subject.started [Source.new ""] + subject.started [Source.new] output.to_s.should eq "Inspecting 1 file\n\n" end end describe "#source_finished" do it "writes valid source" do - subject.source_finished Source.new "" + subject.source_finished Source.new output.to_s.should contain "." end it "writes invalid source" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, Crystal::Nop.new, "message" subject.source_finished s output.to_s.should contain "F" @@ -32,18 +32,18 @@ module Ameba::Formatter describe "#finished" do it "writes a final message" do - subject.finished [Source.new ""] + subject.finished [Source.new] output.to_s.should contain "1 inspected, 0 failures" end it "writes the elapsed time" do - subject.finished [Source.new ""] + subject.finished [Source.new] output.to_s.should contain "Finished in" end context "when issues found" do it "writes each issue" do - s = Source.new("").tap do |source| + s = Source.new.tap do |source| source.add_issue(DummyRule.new, {1, 1}, "DummyRuleError") source.add_issue(NamedRule.new, {1, 2}, "NamedRuleError") end @@ -96,7 +96,7 @@ module Ameba::Formatter end it "does not write disabled issues" do - s = Source.new("").tap do |source| + s = Source.new.tap do |source| source.add_issue(DummyRule.new, {1, 1}, "DummyRuleError", status: :disabled) source.add_issue(NamedRule.new, {1, 2}, "NamedRuleError") end diff --git a/spec/ameba/formatter/flycheck_formatter_spec.cr b/spec/ameba/formatter/flycheck_formatter_spec.cr index 2d45b8356..cfef432c7 100644 --- a/spec/ameba/formatter/flycheck_formatter_spec.cr +++ b/spec/ameba/formatter/flycheck_formatter_spec.cr @@ -11,7 +11,7 @@ module Ameba::Formatter context "problems not found" do it "reports nothing" do - subject.source_finished Source.new "" + subject.source_finished Source.new subject.output.to_s.empty?.should be_true end end diff --git a/spec/ameba/formatter/github_actions_formatter_spec.cr b/spec/ameba/formatter/github_actions_formatter_spec.cr index 1d4a7db90..4146ecc8d 100644 --- a/spec/ameba/formatter/github_actions_formatter_spec.cr +++ b/spec/ameba/formatter/github_actions_formatter_spec.cr @@ -11,14 +11,14 @@ module Ameba::Formatter describe "#source_finished" do it "writes valid source" do - source = Source.new "", "/path/to/file.cr" + source = Source.new path: "/path/to/file.cr" subject.source_finished(source) output.to_s.should be_empty end it "writes invalid source" do - source = Source.new "", "/path/to/file.cr" + source = Source.new path: "/path/to/file.cr" location = Crystal::Location.new("/path/to/file.cr", 1, 2) source.add_issue DummyRule.new, location, location, "message\n2nd line" @@ -33,7 +33,7 @@ module Ameba::Formatter describe "#finished" do it "doesn't do anything if 'GITHUB_STEP_SUMMARY' ENV var is not set" do - subject.finished [Source.new ""] + subject.finished [Source.new] output.to_s.should be_empty end @@ -41,7 +41,7 @@ module Ameba::Formatter prev_summary = ENV["GITHUB_STEP_SUMMARY"]? ENV["GITHUB_STEP_SUMMARY"] = summary_filename = File.tempname begin - sources = [Source.new ""] + sources = [Source.new] subject.started(sources) subject.finished(sources) @@ -67,7 +67,7 @@ module Ameba::Formatter repo = ENV["GITHUB_REPOSITORY"]? sha = ENV["GITHUB_SHA"]? begin - source = Source.new("", "src/source.cr") + source = Source.new path: "src/source.cr" source.add_issue(DummyRule.new, {1, 1}, {2, 1}, "DummyRuleError") source.add_issue(DummyRule.new, {1, 1}, "DummyRuleError 2") source.add_issue(NamedRule.new, {1, 2}, "NamedRuleError", status: :disabled) diff --git a/spec/ameba/formatter/json_formatter_spec.cr b/spec/ameba/formatter/json_formatter_spec.cr index fca016bcf..ea9bc8c0d 100644 --- a/spec/ameba/formatter/json_formatter_spec.cr +++ b/spec/ameba/formatter/json_formatter_spec.cr @@ -1,6 +1,6 @@ require "../../spec_helper" -private def get_result(sources = [Ameba::Source.new ""]) +private def get_result(sources = [Ameba::Source.new]) output = IO::Memory.new formatter = Ameba::Formatter::JSONFormatter.new output @@ -25,12 +25,12 @@ module Ameba::Formatter context "sources" do it "shows path to the source" do - result = get_result [Source.new "", "source.cr"] + result = get_result [Source.new path: "source.cr"] result["sources"][0]["path"].should eq "source.cr" end it "shows rule name" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, {1, 2}, "message1" result = get_result [s] @@ -38,7 +38,7 @@ module Ameba::Formatter end it "shows severity" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, {1, 2}, "message" result = get_result [s] @@ -46,7 +46,7 @@ module Ameba::Formatter end it "shows a message" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, {1, 2}, "message" result = get_result [s] @@ -54,7 +54,7 @@ module Ameba::Formatter end it "shows issue location" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, {1, 2}, "message" result = get_result [s] @@ -64,7 +64,7 @@ module Ameba::Formatter end it "shows issue end_location" do - s = Source.new "" + s = Source.new s.add_issue DummyRule.new, Crystal::Location.new("path", 3, 3), Crystal::Location.new("path", 5, 4), @@ -79,16 +79,16 @@ module Ameba::Formatter context "summary" do it "shows a target sources count" do - result = get_result [Source.new(""), Source.new("")] + result = get_result [Source.new, Source.new] result["summary"]["target_sources_count"].should eq 2 end it "shows issues count" do - s1 = Source.new "" + s1 = Source.new s1.add_issue DummyRule.new, {1, 2}, "message1" s1.add_issue DummyRule.new, {1, 2}, "message2" - s2 = Source.new "" + s2 = Source.new s2.add_issue DummyRule.new, {1, 2}, "message3" result = get_result [s1, s2] diff --git a/spec/ameba/formatter/todo_formatter_spec.cr b/spec/ameba/formatter/todo_formatter_spec.cr index 72b9910ca..c27ec82fe 100644 --- a/spec/ameba/formatter/todo_formatter_spec.cr +++ b/spec/ameba/formatter/todo_formatter_spec.cr @@ -30,14 +30,14 @@ module Ameba context "problems not found" do it "does not create file" do with_formatter do |formatter| - file = formatter.finished [Source.new ""] + file = formatter.finished [Source.new] file.should be_nil end end it "reports a message saying file is not created" do with_formatter do |formatter, io| - formatter.finished [Source.new ""] + formatter.finished [Source.new] io.to_s.should contain "No issues found. File is not generated" end end diff --git a/spec/ameba/reportable_spec.cr b/spec/ameba/reportable_spec.cr index 8d85a55aa..8b34d62f4 100644 --- a/spec/ameba/reportable_spec.cr +++ b/spec/ameba/reportable_spec.cr @@ -4,7 +4,7 @@ module Ameba describe Reportable do describe "#add_issue" do it "adds a new issue for node" do - s = Source.new "", "source.cr" + s = Source.new path: "source.cr" s.add_issue(DummyRule.new, Crystal::Nop.new, "Error!") issue = s.issues.first @@ -14,7 +14,7 @@ module Ameba end it "adds a new issue by line and column number" do - s = Source.new "", "source.cr" + s = Source.new path: "source.cr" s.add_issue(DummyRule.new, {23, 2}, "Error!") issue = s.issues.first @@ -26,12 +26,12 @@ module Ameba describe "#valid?" do it "returns true if no issues added" do - s = Source.new "", "source.cr" + s = Source.new path: "source.cr" s.should be_valid end it "returns false if there are issues added" do - s = Source.new "", "source.cr" + s = Source.new path: "source.cr" s.add_issue DummyRule.new, {22, 2}, "ERROR!" s.should_not be_valid end diff --git a/spec/ameba/rule/base_spec.cr b/spec/ameba/rule/base_spec.cr index 166e1e262..496b196e8 100644 --- a/spec/ameba/rule/base_spec.cr +++ b/spec/ameba/rule/base_spec.cr @@ -4,7 +4,7 @@ module Ameba describe Rule::Base do describe "#catch" do it "accepts and returns source" do - s = Source.new "", "" + s = Source.new DummyRule.new.catch(s).should eq s end end diff --git a/spec/ameba/rule/performance/base_spec.cr b/spec/ameba/rule/performance/base_spec.cr index 7c9aa08b4..56a4f593e 100644 --- a/spec/ameba/rule/performance/base_spec.cr +++ b/spec/ameba/rule/performance/base_spec.cr @@ -6,12 +6,12 @@ module Ameba::Rule::Performance describe "#catch" do it "ignores spec files" do - source = Source.new("", "source_spec.cr") + source = Source.new path: "source_spec.cr" subject.catch(source).should be_valid end it "reports perf issues for non-spec files" do - source = Source.new("", "source.cr") + source = Source.new path: "source.cr" subject.catch(source).should_not be_valid end end diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 17aa97c8c..1d86cb095 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -51,7 +51,7 @@ module Ameba it "checks accordingly to the rule #since_version" do rules = [VersionedRule.new] of Rule::Base - source = Source.new "", "source.cr" + source = Source.new path: "source.cr" v1_0_0 = SemanticVersion.parse("1.0.0") Runner.new(rules, [source], formatter, default_severity, false, v1_0_0).run.success?.should be_true @@ -91,7 +91,7 @@ module Ameba rule = RaiseRule.new rule.should_raise = true rules = [rule] of Rule::Base - source = Source.new "", "source.cr" + source = Source.new path: "source.cr" expect_raises(Exception, "something went wrong") do Runner.new(rules, [source], formatter, default_severity).run diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index 6238cd144..57e844d23 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -13,36 +13,36 @@ module Ameba describe "#fullpath" do it "returns a relative path of the source" do - source = Source.new "", "./source_spec.cr" + source = Source.new path: "./source_spec.cr" source.fullpath.should contain "source_spec.cr" end it "returns fullpath if path is blank" do - source = Source.new "", "" + source = Source.new source.fullpath.should_not be_nil end end describe "#spec?" do it "returns true if the source is a spec file" do - source = Source.new "", "./source_spec.cr" + source = Source.new path: "./source_spec.cr" source.spec?.should be_true end it "returns false if the source is not a spec file" do - source = Source.new "", "./source.cr" + source = Source.new path: "./source.cr" source.spec?.should be_false end end describe "#matches_path?" do it "returns true if source's path is matched" do - source = Source.new "", "source.cr" + source = Source.new path: "source.cr" source.matches_path?("source.cr").should be_true end it "returns false if source's path is not matched" do - source = Source.new "", "source.cr" + source = Source.new path: "source.cr" source.matches_path?("new_source.cr").should be_false end end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 13682a459..01c167336 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -19,7 +19,7 @@ module Ameba # path = "./src/source.cr" # Ameba::Source.new File.read(path), path # ``` - def initialize(@code, @path = "") + def initialize(@code = "", @path = "") end # Corrects any correctable issues and updates `code`. From 79b21369505e1da166ef0575936b5c4fa2e4a52f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 7 Apr 2025 22:24:23 +0200 Subject: [PATCH 40/42] Make the error message include the end range --- src/ameba/source/rewriter.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ameba/source/rewriter.cr b/src/ameba/source/rewriter.cr index ec71bd132..446ced809 100644 --- a/src/ameba/source/rewriter.cr +++ b/src/ameba/source/rewriter.cr @@ -129,7 +129,8 @@ class Ameba::Source private def check_range_validity(begin_pos, end_pos) return unless begin_pos < 0 || end_pos > code.size raise IndexError.new( - "The range #{begin_pos}...#{end_pos} is outside the bounds of the source" + "The range #{begin_pos}...#{end_pos} is outside the bounds of " \ + "the source (0...#{code.size})" ) end end From 3b9c8dc568ded576477b190b1f0102d4a70cc379 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 7 Apr 2025 22:25:36 +0200 Subject: [PATCH 41/42] Add pending test case for autocorrect-incompatible rules correction --- spec/ameba/runner_spec.cr | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 1d86cb095..db9ad8277 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -135,6 +135,14 @@ module Ameba source.issues.first.rule.name.should eq Rule::Lint::UnneededDisableDirective.rule_name end end + + pending "handles rules with incompatible autocorrect" do + rules = [Rule::Performance::MinMaxAfterMap.new, Rule::Style::VerboseBlock.new] + source = Source.new "list.map { |i| i.size }.max", "source.cr" + + Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run + source.code.should eq "list.max_of(&.size)" + end end describe "#explain" do From 83d6fc17e55be3bd75edd81dca3d8fc755bf001a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 7 Apr 2025 22:53:15 +0200 Subject: [PATCH 42/42] =?UTF-8?q?Rename=20`Source#correct=3F`=20to=20`#cor?= =?UTF-8?q?rect!`=20to=20avoid=20confusion=20re:=20whether=20it=E2=80=99s?= =?UTF-8?q?=20a=20verb=20or=20an=20adjective?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ameba/runner.cr | 2 +- src/ameba/source.cr | 2 +- src/ameba/spec/expect_issue.cr | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index ebca6278c..1a3f0abba 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -146,7 +146,7 @@ module Ameba rule.test(source) end check_unneeded_directives(source) - break unless autocorrect? && source.correct? + break unless autocorrect? && source.correct! # The issues that couldn't be corrected will be found again so we # only keep the corrected ones in order to avoid duplicate reporting. diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 01c167336..1606c5747 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -24,7 +24,7 @@ module Ameba # Corrects any correctable issues and updates `code`. # Returns `false` if no issues were corrected. - def correct? + def correct! corrector = Corrector.new(code) issues.each(&.correct(corrector)) diff --git a/src/ameba/spec/expect_issue.cr b/src/ameba/spec/expect_issue.cr index dc5cfee22..ab410138c 100644 --- a/src/ameba/spec/expect_issue.cr +++ b/src/ameba/spec/expect_issue.cr @@ -129,7 +129,7 @@ module Ameba::Spec::ExpectIssue end def expect_correction(source, correction, *, file = __FILE__, line = __LINE__) - raise "Use `expect_no_corrections` if the code will not change" unless source.correct? + raise "Use `expect_no_corrections` if the code will not change" unless source.correct! return if correction == source.code fail <<-MSG, file, line @@ -144,7 +144,7 @@ module Ameba::Spec::ExpectIssue end def expect_no_corrections(source, *, file = __FILE__, line = __LINE__) - return unless source.correct? + return unless source.correct! fail <<-MSG, file, line Expected no corrections, but got: