From 94a271b2a191a3da1ba90a25da861dc9eca031df Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 14 Nov 2022 16:30:32 +0100 Subject: [PATCH 1/4] Add `Style/ParenthesizedAssignments` rule --- .../rule/style/parenthesized_assignments.cr | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 src/ameba/rule/style/parenthesized_assignments.cr diff --git a/src/ameba/rule/style/parenthesized_assignments.cr b/src/ameba/rule/style/parenthesized_assignments.cr new file mode 100644 index 00000000..784d58b6 --- /dev/null +++ b/src/ameba/rule/style/parenthesized_assignments.cr @@ -0,0 +1,43 @@ +module Ameba::Rule::Style + # A rule that disallows assignments without parens in control expressions. + # + # For example, this is considered invalid: + # + # ``` + # if foo = @foo + # do_something + # end + # ``` + # + # And should be replaced by the following: + # + # ``` + # if (foo = @foo) + # do_something + # end + # ``` + # + # YAML configuration example: + # + # ``` + # Style/ParenthesizedAssignments: + # Enabled: true + # ``` + class ParenthesizedAssignments < Base + properties do + enabled false + description "Disallows assignments without parens in control expressions" + end + + MSG = "Missing parentheses around assignment" + + def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until) + return unless (cond = node.cond).is_a?(Crystal::Assign) + + issue_for cond, MSG do |corrector| + corrector.insert_before(cond, '(') + corrector.insert_after(cond, ')') + end + end + end +end From eabe46338617b8a3906f2236ea9be2e185c77f2a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 18 Nov 2022 05:27:05 +0100 Subject: [PATCH 2/4] Instead of adding the new rule to support enforcing parens around assignments, refactor existing `RedundantParentheses` rule --- .../rule/style/redundant_parentheses_spec.cr | 17 +++++++- .../rule/style/parenthesized_assignments.cr | 43 ------------------- src/ameba/rule/style/redundant_parentheses.cr | 23 +++++++--- 3 files changed, 32 insertions(+), 51 deletions(-) delete mode 100644 src/ameba/rule/style/parenthesized_assignments.cr diff --git a/spec/ameba/rule/style/redundant_parentheses_spec.cr b/spec/ameba/rule/style/redundant_parentheses_spec.cr index c6890332..c356b63d 100644 --- a/spec/ameba/rule/style/redundant_parentheses_spec.cr +++ b/spec/ameba/rule/style/redundant_parentheses_spec.cr @@ -75,7 +75,7 @@ module Ameba::Rule::Style end end - context "#exclude_assignments=" do + context "#parenthesized_assignments=" do it "reports assignments by default" do expect_issue subject, <<-CRYSTAL if (foo = @foo) @@ -83,11 +83,24 @@ module Ameba::Rule::Style foo end CRYSTAL + + expect_no_issues subject, <<-CRYSTAL + if foo = @foo + foo + end + CRYSTAL end it "allows to configure assignments" do rule = Rule::Style::RedundantParentheses.new - rule.exclude_assignments = true + rule.parenthesized_assignments = true + + expect_issue rule, <<-CRYSTAL + if foo = @foo + # ^^^^^^^^^^ error: Missing parentheses + foo + end + CRYSTAL expect_no_issues rule, <<-CRYSTAL if (foo = @foo) diff --git a/src/ameba/rule/style/parenthesized_assignments.cr b/src/ameba/rule/style/parenthesized_assignments.cr deleted file mode 100644 index 784d58b6..00000000 --- a/src/ameba/rule/style/parenthesized_assignments.cr +++ /dev/null @@ -1,43 +0,0 @@ -module Ameba::Rule::Style - # A rule that disallows assignments without parens in control expressions. - # - # For example, this is considered invalid: - # - # ``` - # if foo = @foo - # do_something - # end - # ``` - # - # And should be replaced by the following: - # - # ``` - # if (foo = @foo) - # do_something - # end - # ``` - # - # YAML configuration example: - # - # ``` - # Style/ParenthesizedAssignments: - # Enabled: true - # ``` - class ParenthesizedAssignments < Base - properties do - enabled false - description "Disallows assignments without parens in control expressions" - end - - MSG = "Missing parentheses around assignment" - - def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until) - return unless (cond = node.cond).is_a?(Crystal::Assign) - - issue_for cond, MSG do |corrector| - corrector.insert_before(cond, '(') - corrector.insert_after(cond, ')') - end - end - end -end diff --git a/src/ameba/rule/style/redundant_parentheses.cr b/src/ameba/rule/style/redundant_parentheses.cr index b0c1ec15..07e97532 100644 --- a/src/ameba/rule/style/redundant_parentheses.cr +++ b/src/ameba/rule/style/redundant_parentheses.cr @@ -23,17 +23,18 @@ module Ameba::Rule::Style # Style/RedundantParentheses: # Enabled: true # ExcludeTernary: false - # ExcludeAssignments: false + # ParenthesizedAssignments: false # ``` class RedundantParentheses < Base properties do description "Disallows redundant parentheses around control expressions" exclude_ternary false - exclude_assignments false + parenthesized_assignments false end - MSG = "Redundant parentheses" + MSG_REDUNDANT = "Redundant parentheses" + MSG_MISSING = "Missing parentheses" protected def strip_parentheses?(node, in_ternary) : Bool case node @@ -44,24 +45,34 @@ module Ameba::Rule::Style when Crystal::Yield !in_ternary || node.has_parentheses? || node.exps.empty? when Crystal::Assign, Crystal::OpAssign, Crystal::MultiAssign - !in_ternary && !exclude_assignments + !in_ternary && !parenthesized_assignments else true end end def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until) + cond = node.cond + + if cond.is_a?(Crystal::Assign) && parenthesized_assignments + issue_for cond, MSG_MISSING do |corrector| + corrector.insert_before(cond, '(') + corrector.insert_after(cond, ')') + end + return + end + is_ternary = node.is_a?(Crystal::If) && node.ternary? return if is_ternary && exclude_ternary - return unless (cond = node.cond).is_a?(Crystal::Expressions) + return unless cond.is_a?(Crystal::Expressions) return unless cond.keyword.paren? return unless exp = cond.single_expression? return unless strip_parentheses?(exp, is_ternary) - issue_for cond, MSG do |corrector| + issue_for cond, MSG_REDUNDANT do |corrector| corrector.remove_trailing(cond, 1) corrector.remove_leading(cond, 1) end From 0b0a815c31928670622d89f387634bdb18994dbd Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 18 Nov 2022 07:04:51 +0100 Subject: [PATCH 3/4] Add additional test case --- spec/ameba/rule/style/redundant_parentheses_spec.cr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/ameba/rule/style/redundant_parentheses_spec.cr b/spec/ameba/rule/style/redundant_parentheses_spec.cr index c356b63d..92aa9cea 100644 --- a/spec/ameba/rule/style/redundant_parentheses_spec.cr +++ b/spec/ameba/rule/style/redundant_parentheses_spec.cr @@ -84,6 +84,12 @@ module Ameba::Rule::Style end CRYSTAL + expect_no_issues subject, <<-CRYSTAL + if !(foo = @foo) + foo + end + CRYSTAL + expect_no_issues subject, <<-CRYSTAL if foo = @foo foo From 5ee4074c1bd542935c3147d46bde85445839149a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 18 Nov 2022 21:00:54 +0100 Subject: [PATCH 4/4] Rename `RedundantParentheses` -> `ParenthesesAroundCondition` Also rename the option: `parenthesized_assignments` -> `allow_safe_assignment` --- ...ec.cr => parentheses_around_condition_spec.cr} | 12 ++++++------ ...ntheses.cr => parentheses_around_condition.cr} | 15 ++++++++------- 2 files changed, 14 insertions(+), 13 deletions(-) rename spec/ameba/rule/style/{redundant_parentheses_spec.cr => parentheses_around_condition_spec.cr} (91%) rename src/ameba/rule/style/{redundant_parentheses.cr => parentheses_around_condition.cr} (82%) diff --git a/spec/ameba/rule/style/redundant_parentheses_spec.cr b/spec/ameba/rule/style/parentheses_around_condition_spec.cr similarity index 91% rename from spec/ameba/rule/style/redundant_parentheses_spec.cr rename to spec/ameba/rule/style/parentheses_around_condition_spec.cr index 92aa9cea..6312fe0e 100644 --- a/spec/ameba/rule/style/redundant_parentheses_spec.cr +++ b/spec/ameba/rule/style/parentheses_around_condition_spec.cr @@ -1,9 +1,9 @@ require "../../../spec_helper" module Ameba::Rule::Style - subject = RedundantParentheses.new + subject = ParenthesesAroundCondition.new - describe RedundantParentheses do + describe ParenthesesAroundCondition do {% for keyword in %w(if unless while until) %} context "{{ keyword.id }}" do it "reports if redundant parentheses are found" do @@ -51,7 +51,7 @@ module Ameba::Rule::Style end it "allows to configure assignments" do - rule = Rule::Style::RedundantParentheses.new + rule = Rule::Style::ParenthesesAroundCondition.new rule.exclude_ternary = false expect_issue rule, <<-CRYSTAL @@ -75,7 +75,7 @@ module Ameba::Rule::Style end end - context "#parenthesized_assignments=" do + context "#allow_safe_assignment=" do it "reports assignments by default" do expect_issue subject, <<-CRYSTAL if (foo = @foo) @@ -98,8 +98,8 @@ module Ameba::Rule::Style end it "allows to configure assignments" do - rule = Rule::Style::RedundantParentheses.new - rule.parenthesized_assignments = true + rule = Rule::Style::ParenthesesAroundCondition.new + rule.allow_safe_assignment = true expect_issue rule, <<-CRYSTAL if foo = @foo diff --git a/src/ameba/rule/style/redundant_parentheses.cr b/src/ameba/rule/style/parentheses_around_condition.cr similarity index 82% rename from src/ameba/rule/style/redundant_parentheses.cr rename to src/ameba/rule/style/parentheses_around_condition.cr index 07e97532..1b347e2a 100644 --- a/src/ameba/rule/style/redundant_parentheses.cr +++ b/src/ameba/rule/style/parentheses_around_condition.cr @@ -1,5 +1,6 @@ module Ameba::Rule::Style - # A rule that disallows redundant parentheses around control expressions. + # A rule that checks for the presence of superfluous parentheses + # around the condition of `if`, `unless`, `case, `while` and `until`. # # For example, this is considered invalid: # @@ -20,17 +21,17 @@ module Ameba::Rule::Style # YAML configuration example: # # ``` - # Style/RedundantParentheses: + # Style/ParenthesesAroundCondition: # Enabled: true # ExcludeTernary: false - # ParenthesizedAssignments: false + # AllowSafeAssignment: false # ``` - class RedundantParentheses < Base + class ParenthesesAroundCondition < Base properties do description "Disallows redundant parentheses around control expressions" exclude_ternary false - parenthesized_assignments false + allow_safe_assignment false end MSG_REDUNDANT = "Redundant parentheses" @@ -45,7 +46,7 @@ module Ameba::Rule::Style when Crystal::Yield !in_ternary || node.has_parentheses? || node.exps.empty? when Crystal::Assign, Crystal::OpAssign, Crystal::MultiAssign - !in_ternary && !parenthesized_assignments + !in_ternary && !allow_safe_assignment else true end @@ -54,7 +55,7 @@ module Ameba::Rule::Style def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until) cond = node.cond - if cond.is_a?(Crystal::Assign) && parenthesized_assignments + if cond.is_a?(Crystal::Assign) && allow_safe_assignment issue_for cond, MSG_MISSING do |corrector| corrector.insert_before(cond, '(') corrector.insert_after(cond, ')')