Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Style/RedundantParentheses rule #301

Merged
merged 3 commits into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions spec/ameba/rule/style/redundant_parentheses_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
require "../../../spec_helper"

module Ameba::Rule::Style
subject = RedundantParentheses.new

describe RedundantParentheses do
{% for keyword in %w(if unless while until) %}
context "{{ keyword.id }}" do
it "reports if redundant parentheses are found" do
source = expect_issue subject, <<-CRYSTAL, keyword: {{ keyword }}
%{keyword} (foo > 10)
_{keyword} # ^^^^^^^^^^ error: Redundant parentheses
foo
end
CRYSTAL

expect_correction source, <<-CRYSTAL
{{ keyword.id }} foo > 10
foo
end
CRYSTAL
end
end
{% end %}

context "case" do
it "reports if redundant parentheses are found" do
source = expect_issue subject, <<-CRYSTAL
case (foo = @foo)
# ^^^^^^^^^^^^ error: Redundant parentheses
when String then "string"
when Symbol then "symbol"
end
CRYSTAL

expect_correction source, <<-CRYSTAL
case foo = @foo
when String then "string"
when Symbol then "symbol"
end
CRYSTAL
end
end

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 = Rule::Style::RedundantParentheses.new
rule.exclude_ternary = false

expect_issue rule, <<-CRYSTAL
(foo.empty?) ? true : false
# ^^^^^^^^^^ error: Redundant parentheses
CRYSTAL

expect_no_issues subject, <<-CRYSTAL
(foo && bar) ? true : false
(foo || bar) ? true : false
(foo = @foo) ? true : false
foo == 42 ? true : false
(foo = 42) ? true : false
(foo > 42) ? true : false
(foo >= 42) ? true : false
(3 >= foo >= 42) ? true : false
(3.in? 0..42) ? true : false
(yield 42) ? true : false
(foo rescue 42) ? true : false
CRYSTAL
end
end

context "#exclude_assignments=" do
it "reports assignments by default" do
expect_issue subject, <<-CRYSTAL
if (foo = @foo)
# ^^^^^^^^^^^^ error: Redundant parentheses
foo
end
CRYSTAL
end

it "allows to configure assignments" do
rule = Rule::Style::RedundantParentheses.new
rule.exclude_assignments = true

expect_no_issues rule, <<-CRYSTAL
if (foo = @foo)
foo
end
CRYSTAL
end
end
end
end
end
70 changes: 70 additions & 0 deletions src/ameba/rule/style/redundant_parentheses.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
module Ameba::Rule::Style
# A rule that disallows redundant parentheses around control expressions.
#
# For example, this is considered invalid:
#
# ```
# if (foo == 42)
# do_something
# end
# ```
#
# And should be replaced by the following:
#
# ```
# if foo == 42
# do_something
# end
# ```
#
# YAML configuration example:
#
# ```
# Style/RedundantParentheses:
# Enabled: true
# ExcludeTernary: false
# ExcludeAssignments: false
# ```
class RedundantParentheses < Base
properties do
description "Disallows redundant parentheses around control expressions"

exclude_ternary false
exclude_assignments false
end

MSG = "Redundant parentheses"

protected def strip_parentheses?(node, in_ternary) : Bool
case node
when Crystal::BinaryOp, Crystal::ExceptionHandler
!in_ternary
when Crystal::Call
!in_ternary || node.has_parentheses? || node.args.empty?
when Crystal::Yield
!in_ternary || node.has_parentheses? || node.exps.empty?
when Crystal::Assign, Crystal::OpAssign, Crystal::MultiAssign
!in_ternary && !exclude_assignments
else
true
end
end

def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case | Crystal::While | Crystal::Until)
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.keyword.paren?

return unless exp = cond.single_expression?
return unless strip_parentheses?(exp, is_ternary)

issue_for cond, MSG do |corrector|
corrector.remove_trailing(cond, 1)
corrector.remove_leading(cond, 1)
end
end
end
end
2 changes: 1 addition & 1 deletion src/ameba/spec/annotated_source.cr
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class Ameba::Spec::AnnotatedSource
private def message_to_regex(expected_annotation)
String.build do |io|
offset = 0
while (index = expected_annotation.index(ABBREV, offset))
while index = expected_annotation.index(ABBREV, offset)
io << Regex.escape(expected_annotation[offset...index])
io << ".*?"
offset = index + ABBREV.size
Expand Down