From 01dfcbe76a63c0a99bb3236ccee019a4757d1e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?= <70830482+FnControlOption@users.noreply.github.com> Date: Tue, 9 Nov 2021 11:45:38 -0800 Subject: [PATCH] Add tests for `AST::Util#control_exp_code` --- spec/ameba/ast/util_spec.cr | 79 +++++++++++++++--------- src/ameba/ast/util.cr | 6 +- src/ameba/rule/style/redundant_next.cr | 2 +- src/ameba/rule/style/redundant_return.cr | 2 +- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/spec/ameba/ast/util_spec.cr b/spec/ameba/ast/util_spec.cr index 17f5649b..be47dd70 100644 --- a/spec/ameba/ast/util_spec.cr +++ b/spec/ameba/ast/util_spec.cr @@ -39,31 +39,31 @@ module Ameba::AST describe "#node_source" do it "returns original source of the node" do - s = %( + s = <<-CRYSTAL a = 1 - ) + CRYSTAL node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") source.should eq "a = 1" end it "returns original source of multiline node" do - s = %( + s = <<-CRYSTAL if () :ok end - ) + CRYSTAL node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") source.should eq <<-CRYSTAL if () - :ok - end + :ok + end CRYSTAL end it "does not report source of node which has incorrect location" do - s = %q( + s = <<-'CRYSTAL' module MyModule macro conditional_error_for_inline_callbacks \{% @@ -74,7 +74,7 @@ module Ameba::AST macro before_save(x = nil) end end - ) + CRYSTAL node = as_nodes(s).nil_literal_nodes.first source = subject.node_source node, s.split("\n") @@ -140,29 +140,29 @@ module Ameba::AST end it "returns true if this is if-else consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL if foo return :foo else return :bar end - ) + CRYSTAL subject.flow_expression?(node, false).should eq true end it "returns true if this is unless-else consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL unless foo return :foo else return :bar end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this is case consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL case when 1 return 1 @@ -171,71 +171,71 @@ module Ameba::AST else return 3 end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this is exception handler consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL begin raise "exp" rescue e return e end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this while consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL while true return end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false if this while with break" do - node = as_node %( + node = as_node <<-CRYSTAL while true break end - ) + CRYSTAL subject.flow_expression?(node).should eq false end it "returns true if this until consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL until false return end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false if this until with break" do - node = as_node %( + node = as_node <<-CRYSTAL until false break end - ) + CRYSTAL subject.flow_expression?(node).should eq false end it "returns true if this expressions consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL exp1 exp2 return - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false otherwise" do - node = as_node %( + node = as_node <<-CRYSTAL exp1 exp2 - ) + CRYSTAL subject.flow_expression?(node).should eq false end end @@ -322,5 +322,28 @@ module Ameba::AST subject.loop?(node).should eq false end end + + describe "#control_exp_code" do + it "returns the exp code of a control expression" do + s = "return 1" + node = as_node(s).as Crystal::ControlExpression + exp_code = subject.control_exp_code node, [s] + exp_code.should eq "1" + end + + it "wraps implicit tuple literal with curly brackets" do + s = "return 1, 2" + node = as_node(s).as Crystal::ControlExpression + exp_code = subject.control_exp_code node, [s] + exp_code.should eq "{1, 2}" + end + + it "accepts explicit tuple literal" do + s = "return {1, 2}" + node = as_node(s).as Crystal::ControlExpression + exp_code = subject.control_exp_code node, [s] + exp_code.should eq "{1, 2}" + end + end end end diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 0d17d857..2965247f 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -161,13 +161,13 @@ module Ameba::AST::Util # Returns the exp code of a control expression. # Wraps implicit tuple literal with curly brackets (e.g. multi-return). - def control_exp_code(source, node : Crystal::ControlExpression) + def control_exp_code(node : Crystal::ControlExpression, code_lines) return unless (exp = node.exp) - return unless (exp_code = node_source(exp, source.lines)) + return unless (exp_code = node_source(exp, code_lines)) return exp_code unless exp.is_a?(Crystal::TupleLiteral) && exp_code[0] != '{' return unless (exp_start = exp.elements.first.location) return unless (exp_end = exp.end_location) - "{#{source_between(exp_start, exp_end, source.lines)}}" + "{#{source_between(exp_start, exp_end, code_lines)}}" end end diff --git a/src/ameba/rule/style/redundant_next.cr b/src/ameba/rule/style/redundant_next.cr index 4748b7c1..214690b5 100644 --- a/src/ameba/rule/style/redundant_next.cr +++ b/src/ameba/rule/style/redundant_next.cr @@ -116,7 +116,7 @@ module Ameba::Rule::Style return if allow_multi_next && node.exp.is_a?(Crystal::TupleLiteral) return if allow_empty_next && (node.exp.nil? || node.exp.not_nil!.nop?) - if (exp_code = control_exp_code(source, node)) + if (exp_code = control_exp_code(node, source.lines)) issue_for node, MSG do |corrector| corrector.replace(node, exp_code) end diff --git a/src/ameba/rule/style/redundant_return.cr b/src/ameba/rule/style/redundant_return.cr index 32f1c86b..77eafe73 100644 --- a/src/ameba/rule/style/redundant_return.cr +++ b/src/ameba/rule/style/redundant_return.cr @@ -113,7 +113,7 @@ module Ameba::Rule::Style return if allow_multi_return && node.exp.is_a?(Crystal::TupleLiteral) return if allow_empty_return && (node.exp.nil? || node.exp.not_nil!.nop?) - if (exp_code = control_exp_code(source, node)) + if (exp_code = control_exp_code(node, source.lines)) issue_for node, MSG do |corrector| corrector.replace(node, exp_code) end