From d7795c0d7d5bd6f3009907309883a68ab485bb1e 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: Mon, 19 Dec 2022 06:27:20 -0800 Subject: [PATCH 01/32] Add autocorrect for `Style/UnlessElse` --- spec/ameba/rule/style/unless_else_spec.cr | 10 +++- spec/ameba/source_spec.cr | 17 ++++++ src/ameba/rule/style/unless_else.cr | 25 ++++++++- src/ameba/source.cr | 8 +++ src/ameba/source/corrector.cr | 64 +++++++++++++++++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) diff --git a/spec/ameba/rule/style/unless_else_spec.cr b/spec/ameba/rule/style/unless_else_spec.cr index cf1c065d..f79575a2 100644 --- a/spec/ameba/rule/style/unless_else_spec.cr +++ b/spec/ameba/rule/style/unless_else_spec.cr @@ -13,7 +13,7 @@ module Ameba::Rule::Style end it "fails if unless has else" do - expect_issue subject, <<-CRYSTAL + source = expect_issue subject, <<-CRYSTAL unless something # ^^^^^^^^^^^^^^ error: Favour if over unless with else :one @@ -21,6 +21,14 @@ module Ameba::Rule::Style :two end CRYSTAL + + expect_correction source, <<-CRYSTAL + if something + :two + else + :one + end + CRYSTAL end it "reports rule, pos and message" do diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index fd4781fa..e50b499b 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -46,5 +46,22 @@ module Ameba s.matches_path?("new_source.cr").should be_false end end + + describe "#pos" do + it "works" do + s = Source.new <<-EOS + foo + bar + fizz + buzz + EOS + loc = Crystal::Location.new("", 2, 1) + end_loc = Crystal::Location.new("", 3, 4) + s.code[s.pos(loc)...s.pos(end_loc, end: true)].should eq <<-EOS + bar + fizz + EOS + end + end end end diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index cdb4b966..20fa0e08 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -50,7 +50,30 @@ module Ameba::Rule::Style MSG = "Favour if over unless with else" def test(source, node : Crystal::Unless) - issue_for node, MSG unless node.else.nop? + return if node.else.nop? + return unless location = node.location + return unless cond_end_location = node.cond.end_location + return unless else_location = node.else_location + return unless end_location = node.end_location + + issue_for node, MSG do |corrector| + keyword_begin_pos = source.pos(location) + keyword_end_pos = keyword_begin_pos + {{ "unless".size }} + keyword_range = keyword_begin_pos...keyword_end_pos + + cond_end_pos = source.pos(cond_end_location, end: true) + else_begin_pos = source.pos(else_location) + body_range = cond_end_pos...else_begin_pos + + else_end_pos = else_begin_pos + {{ "else".size }} + end_end_pos = source.pos(end_location, end: true) + end_begin_pos = end_end_pos - {{ "end".size }} + else_range = else_end_pos...end_begin_pos + + corrector.replace(keyword_range, "if") + corrector.replace(body_range, source.code[else_range]) + corrector.replace(else_range, source.code[body_range]) + end end end end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 38ce9b82..03aa3ac3 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -75,5 +75,13 @@ module Ameba def matches_path?(filepath) path.in?(filepath, File.expand_path(filepath)) end + + # Converts an AST location to a string position. + def pos(location : Crystal::Location, end end_pos = false) : Int32 + line, column = location.line_number, location.column_number + pos = lines[0...line - 1].sum(&.size) + line + column - 2 + pos += 1 if end_pos + pos + end end end diff --git a/src/ameba/source/corrector.cr b/src/ameba/source/corrector.cr index 646594be..33e722d2 100644 --- a/src/ameba/source/corrector.cr +++ b/src/ameba/source/corrector.cr @@ -18,41 +18,92 @@ class Ameba::Source @rewriter.replace(loc_to_pos(location), loc_to_pos(end_location) + 1, content) end + # :ditto: + def replace(range : Range(Int32, Int32), content) + begin_pos, end_pos = range.begin, range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.replace(begin_pos, end_pos, content) + end + # Inserts the given strings before and after the given range. def wrap(location, end_location, insert_before, insert_after) @rewriter.wrap(loc_to_pos(location), loc_to_pos(end_location) + 1, insert_before, insert_after) end + # :ditto: + def wrap(range : Range(Int32, Int32), insert_before, insert_after) + begin_pos, end_pos = range.begin, range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.wrap(begin_pos, end_pos, insert_before, insert_after) + end + # Shortcut for `replace(location, end_location, "")` def remove(location, end_location) @rewriter.remove(loc_to_pos(location), loc_to_pos(end_location) + 1) end + # Shortcut for `replace(range, "")` + def remove(range : Range(Int32, Int32)) + begin_pos, end_pos = range.begin, range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.remove(begin_pos, end_pos) + end + # Shortcut for `wrap(location, end_location, content, nil)` def insert_before(location, end_location, content) @rewriter.insert_before(loc_to_pos(location), loc_to_pos(end_location) + 1, content) end + # Shortcut for `wrap(range, content, nil)` + def insert_before(range : Range(Int32, Int32), content) + begin_pos, end_pos = range.begin, range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.insert_before(begin_pos, end_pos, content) + end + # Shortcut for `wrap(location, end_location, nil, content)` def insert_after(location, end_location, content) @rewriter.insert_after(loc_to_pos(location), loc_to_pos(end_location) + 1, content) end + # Shortcut for `wrap(range, nil, content)` + def insert_after(range : Range(Int32, Int32), content) + begin_pos, end_pos = range.begin, range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.insert_after(begin_pos, end_pos, content) + end + # Shortcut for `insert_before(location, location, content)` def insert_before(location, content) @rewriter.insert_before(loc_to_pos(location), content) end + # Shortcut for `insert_before(pos.., content)` + def insert_before(pos : Int32, content) + @rewriter.insert_before(pos, content) + end + # Shortcut for `insert_after(location, location, content)` def insert_after(location, content) @rewriter.insert_after(loc_to_pos(location) + 1, content) end + # Shortcut for `insert_after(...pos, content)` + def insert_after(pos : Int32, content) + @rewriter.insert_after(pos, content) + end + # Removes *size* characters prior to the source range. def remove_preceding(location, end_location, size) @rewriter.remove(loc_to_pos(location) - size, loc_to_pos(location)) end + # :ditto: + def remove_preceding(range : Range(Int32, Int32), size) + begin_pos = range.begin + @rewriter.remove(begin_pos - size, begin_pos) + end + # Removes *size* characters from the beginning of the given range. # If *size* is greater than the size of the range, the removed region can # overrun the end of the range. @@ -60,6 +111,12 @@ class Ameba::Source @rewriter.remove(loc_to_pos(location), loc_to_pos(location) + size) end + # :ditto: + def remove_leading(range : Range(Int32, Int32), size) + begin_pos = range.begin + @rewriter.remove(begin_pos, begin_pos + size) + end + # Removes *size* characters from the end of the given range. # If *size* is greater than the size of the range, the removed region can # overrun the beginning of the range. @@ -67,6 +124,13 @@ class Ameba::Source @rewriter.remove(loc_to_pos(end_location) + 1 - size, loc_to_pos(end_location) + 1) end + # :ditto: + def remove_trailing(range : Range(Int32, Int32), size) + end_pos = range.end + end_pos -= 1 unless range.excludes_end? + @rewriter.remove(end_pos - size, end_pos) + end + private def loc_to_pos(location : Crystal::Location | {Int32, Int32}) if location.is_a?(Crystal::Location) line, column = location.line_number, location.column_number From 8112dddc8f5ba90fa517d51d5e1bea304c3c8ed3 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 19 Dec 2022 18:03:11 +0100 Subject: [PATCH 02/32] Specs cleanup --- spec/ameba/ast/branch_spec.cr | 120 +++++++++--------- spec/ameba/ast/branchable_spec.cr | 14 +- spec/ameba/ast/flow_expression_spec.cr | 14 +- spec/ameba/ast/scope_spec.cr | 34 +++-- spec/ameba/ast/variabling/assignment_spec.cr | 24 ++-- spec/ameba/ast/variabling/variable_spec.cr | 10 +- .../ast/visitors/counting_visitor_spec.cr | 16 +-- .../visitors/flow_expression_visitor_spec.cr | 22 ++-- ...dundant_control_expression_visitor_spec.cr | 4 +- spec/ameba/ast/visitors/scope_visitor_spec.cr | 20 +-- .../visitors/top_level_nodes_visitor_spec.cr | 8 +- spec/ameba/inline_comments_spec.cr | 120 +++++++++--------- .../rule/lint/duplicated_require_spec.cr | 4 +- spec/ameba/rule/lint/empty_expression_spec.cr | 4 +- .../rule/performance/any_after_filter_spec.cr | 2 +- .../first_last_after_filter_spec.cr | 2 +- .../performance/size_after_filter_spec.cr | 2 +- spec/ameba/rule/style/is_a_filter_spec.cr | 1 + .../parentheses_around_condition_spec.cr | 4 +- spec/ameba/rule/style/redundant_next_spec.cr | 4 +- .../ameba/rule/style/redundant_return_spec.cr | 4 +- spec/ameba/rule/style/verbose_block_spec.cr | 19 ++- spec/ameba/runner_spec.cr | 40 +++--- src/ameba/rule/lint/ambiguous_assignment.cr | 26 ++-- 24 files changed, 267 insertions(+), 251 deletions(-) diff --git a/spec/ameba/ast/branch_spec.cr b/spec/ameba/ast/branch_spec.cr index e15f44af..4da80472 100644 --- a/spec/ameba/ast/branch_spec.cr +++ b/spec/ameba/ast/branch_spec.cr @@ -10,29 +10,29 @@ module Ameba::AST describe ".of" do context "Crystal::If" do it "constructs a branch in If.cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method if a = get_something # --> Crystal::Assign puts a end end - ) + CRYSTAL branch.to_s.should eq "a = get_something" end it "constructs a branch in If.then" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method if true a = 2 # --> Crystal::Assign end end - ) + CRYSTAL branch.to_s.should eq "a = 2" end it "constructs a branch in If.else" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method if true nil @@ -40,45 +40,45 @@ module Ameba::AST a = 2 # --> Crystal::Assign end end - ) + CRYSTAL branch.to_s.should eq "a = 2" end it "constructs a branch in inline If" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) a = 0 if a == 2 # --> Crystal::Assign end - ) + CRYSTAL branch.to_s.should eq "a = 0" end end context "Crystal::Unless" do it "constructs a branch in Unless.cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method unless a = get_something # --> Crystal::Assign puts a end end - ) + CRYSTAL branch.to_s.should eq "a = get_something" end it "constructs a branch in Unless.then" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method unless true a = 2 # --> Crystal::Assign end end - ) + CRYSTAL branch.to_s.should eq "a = 2" end it "constructs a new branch in Unless.else" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method unless true nil @@ -86,188 +86,188 @@ module Ameba::AST a = 2 # --> Crystal::Assign end end - ) + CRYSTAL branch.to_s.should eq "a = 2" end it "constructs a branch in inline Unless" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) (a = 0; b = 3) unless a == 2 # --> Crystal::Expressions end - ) + CRYSTAL branch.to_s.should eq "(a = 0\nb = 3)" end end context "Crystal::BinaryOp" do it "constructs a branch in left node" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) (a = 2) && do_something end - ) + CRYSTAL branch.to_s.should eq "(a = 2)" end it "constructs a branch in right node" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) do_something || (a = 0) end - ) + CRYSTAL branch.to_s.should eq "(a = 0)" end end context "Crystal::Case" do it "constructs a branch in cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) case (a = 2) when true then nil end end - ) + CRYSTAL branch.to_s.should eq "(a = 2)" end it "constructs a branch in when" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) case a when a = 3 then nil end end - ) + CRYSTAL branch.to_s.should eq "when a = 3\n nil\n" end it "constructs a branch in else" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) case a when true then nil else a = 4 end end - ) + CRYSTAL branch.to_s.should eq "a = 4" end end context "Crystal::While" do it "constructs a branch in cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) while a = 1 nil end end - ) + CRYSTAL branch.to_s.should eq "a = 1" end it "constructs a branch in body" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) while true b = (a = 1) end end - ) + CRYSTAL branch.to_s.should eq "b = (a = 1)" end end context "Crystal::Until" do it "constructs a branch in cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) until a = 1 nil end end - ) + CRYSTAL branch.to_s.should eq "a = 1" end it "constructs a branch in body" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) until false b = (a = 1) end end - ) + CRYSTAL branch.to_s.should eq "b = (a = 1)" end end context "Crystal::ExceptionHandler" do it "constructs a branch in body" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) a = 1 rescue nil end - ) + CRYSTAL branch.to_s.should eq "a = 1" end it "constructs a branch in a rescue" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) rescue a = 1 end - ) + CRYSTAL branch.to_s.should eq "a = 1" end it "constructs a branch in else" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) rescue else a = 1 end - ) + CRYSTAL branch.to_s.should eq "a = 1" end it "constructs a branch in ensure" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) rescue ensure a = 1 end - ) + CRYSTAL branch.to_s.should eq "a = 1" end end context "Crystal::MacroIf" do it "constructs a branch in cond" do - branch = branch_of_assign_in_def %( + branch = branch_of_assign_in_def <<-CRYSTAL def method(a) {% if a = 2 %} {% end %} end - ) + CRYSTAL branch.to_s.should eq "a = 2" end it "constructs a branch in then" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a) {% if true %} a = 2 {% end %} end - ) + CRYSTAL branch = Branch.of(nodes.macro_literal_nodes.first, nodes.def_nodes.first) branch.to_s.strip.should eq "a = 2" end @@ -275,24 +275,24 @@ module Ameba::AST context "Crystal::MacroFor" do it "constructs a branch in body" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a) {% for x in [1, 2, 3] %} a = 2 {% end %} end - ) + CRYSTAL branch = Branch.of(nodes.macro_literal_nodes.first, nodes.def_nodes.first) branch.to_s.strip.should eq "a = 2" end end it "returns nil if branch does not exist" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method a = 2 end - ) + CRYSTAL branch = Branch.of(nodes.assign_nodes.first, nodes.def_nodes.first) branch.should be_nil end @@ -300,11 +300,11 @@ module Ameba::AST describe "#initialize" do it "creates new branch" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL if true a = 2 end - ) + CRYSTAL branchable = Branchable.new nodes.if_nodes.first branch = Branch.new nodes.assign_nodes.first, branchable branch.node.should_not be_nil @@ -313,22 +313,22 @@ module Ameba::AST describe "delegation" do it "delegates to_s to node" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL if true a = 2 end - ) + CRYSTAL branchable = Branchable.new nodes.if_nodes.first branch = Branch.new nodes.assign_nodes.first, branchable branch.to_s.should eq branch.node.to_s end it "delegates locations to node" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL if true a = 2 end - ) + CRYSTAL branchable = Branchable.new nodes.if_nodes.first branch = Branch.new nodes.assign_nodes.first, branchable branch.location.should eq branch.node.location @@ -338,22 +338,22 @@ module Ameba::AST describe "#in_loop?" do it "returns true if branch is in a loop" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL while true a = 1 end - ) + CRYSTAL branchable = Branchable.new nodes.while_nodes.first branch = Branch.new nodes.assign_nodes.first, branchable branch.in_loop?.should be_true end it "returns false if branch is not in a loop" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL if a > 2 a = 1 end - ) + CRYSTAL branchable = Branchable.new nodes.if_nodes.first branch = Branch.new nodes.assign_nodes.first, branchable branch.in_loop?.should be_false diff --git a/spec/ameba/ast/branchable_spec.cr b/spec/ameba/ast/branchable_spec.cr index d8cf7b24..6e1e2a2c 100644 --- a/spec/ameba/ast/branchable_spec.cr +++ b/spec/ameba/ast/branchable_spec.cr @@ -4,20 +4,20 @@ module Ameba::AST describe Branchable do describe "#initialize" do it "creates a new branchable" do - branchable = Branchable.new as_node %(a = 2 if true) + branchable = Branchable.new as_node "a = 2 if true" branchable.node.should_not be_nil end end describe "delegation" do it "delegates to_s to @node" do - node = as_node %(a = 2 if true) + node = as_node "a = 2 if true" branchable = Branchable.new node branchable.to_s.should eq node.to_s end it "delegates locations to @node" do - node = as_node %(a = 2 if true) + node = as_node "a = 2 if true" branchable = Branchable.new node branchable.location.should eq node.location branchable.end_location.should eq node.end_location @@ -26,22 +26,22 @@ module Ameba::AST describe "#loop?" do it "returns true if it is a while loop" do - branchable = Branchable.new as_node %(while true; a = 2; end) + branchable = Branchable.new as_node "while true; a = 2; end" branchable.loop?.should be_true end it "returns true if it is the until loop" do - branchable = Branchable.new as_node %(until false; a = 2; end) + branchable = Branchable.new as_node "until false; a = 2; end" branchable.loop?.should be_true end it "returns true if it is loop" do - branchable = Branchable.new as_node %(loop {}) + branchable = Branchable.new as_node "loop {}" branchable.loop?.should be_true end it "returns false otherwise" do - branchable = Branchable.new as_node %(a = 2 if true) + branchable = Branchable.new as_node "a = 2 if true" branchable.loop?.should be_false end end diff --git a/spec/ameba/ast/flow_expression_spec.cr b/spec/ameba/ast/flow_expression_spec.cr index d2d697ed..6d28111d 100644 --- a/spec/ameba/ast/flow_expression_spec.cr +++ b/spec/ameba/ast/flow_expression_spec.cr @@ -18,7 +18,7 @@ module Ameba::AST end it "delegates locations to @node" do - node = as_node %(break if true) + node = as_node("break if true") flow_expression = FlowExpression.new node, false flow_expression.location.should eq node.location flow_expression.end_location.should eq node.end_location @@ -27,20 +27,20 @@ module Ameba::AST describe "#unreachable_nodes" do it "returns unreachable nodes" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def foobar return a = 1 a = 2 end - ) + CRYSTAL node = nodes.expressions_nodes.first flow_expression = FlowExpression.new node, false flow_expression.unreachable_nodes.should eq nodes.assign_nodes end it "returns nil if there is no unreachable node after loop" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def run idx = items.size - 1 while 0 <= idx @@ -49,19 +49,19 @@ module Ameba::AST puts "foo" end - ) + CRYSTAL node = nodes.expressions_nodes.first flow_expression = FlowExpression.new node, false flow_expression.unreachable_nodes.empty?.should eq true end it "returns nil if there is no unreachable node" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def foobar a = 1 return a end - ) + CRYSTAL node = nodes.expressions_nodes.first flow_expression = FlowExpression.new node, false flow_expression.unreachable_nodes.empty?.should eq true diff --git a/spec/ameba/ast/scope_spec.cr b/spec/ameba/ast/scope_spec.cr index f7f080fe..c36ef62d 100644 --- a/spec/ameba/ast/scope_spec.cr +++ b/spec/ameba/ast/scope_spec.cr @@ -49,14 +49,14 @@ module Ameba::AST describe "#references?" do it "returns true if current scope references variable" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method a = 2 block do 3.times { |i| a = a + i } end end - ) + CRYSTAL scope = Scope.new nodes.def_nodes.first var_node = nodes.var_nodes.first scope.add_variable var_node @@ -69,14 +69,14 @@ module Ameba::AST end it "returns false if inner scopes are not checked" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method a = 2 block do 3.times { |i| a = a + i } end end - ) + CRYSTAL scope = Scope.new nodes.def_nodes.first var_node = nodes.var_nodes.first scope.add_variable var_node @@ -89,7 +89,7 @@ module Ameba::AST end it "returns false if current scope does not reference variable" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method a = 2 block do @@ -97,7 +97,7 @@ module Ameba::AST 3.times { |i| b = b + i } end end - ) + CRYSTAL scope = Scope.new nodes.def_nodes.first var_node = nodes.var_nodes.first scope.add_variable var_node @@ -150,57 +150,53 @@ module Ameba::AST describe "#block?" do it "returns true if Crystal::Block" do - nodes = as_nodes %( - 3.times {} - ) + nodes = as_nodes("3.times {}") scope = Scope.new nodes.block_nodes.first scope.block?.should be_true end it "returns false otherwise" do - scope = Scope.new as_node "a = 1" + scope = Scope.new as_node("a = 1") scope.block?.should be_false end end describe "#spawn_block?" do it "returns true if a node is a spawn block" do - nodes = as_nodes %( - spawn {} - ) + nodes = as_nodes("spawn {}") scope = Scope.new nodes.block_nodes.first scope.spawn_block?.should be_true end it "returns false otherwise" do - scope = Scope.new as_node "a = 1" + scope = Scope.new as_node("a = 1") scope.spawn_block?.should be_false end end describe "#in_macro?" do it "returns true if Crystal::Macro" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL macro included end - ) + CRYSTAL scope = Scope.new nodes.macro_nodes.first scope.in_macro?.should be_true end it "returns true if node is nested to Crystal::Macro" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL macro included {{ @type.each do |type| a = type end }} end - ) + CRYSTAL outer_scope = Scope.new nodes.macro_nodes.first scope = Scope.new nodes.block_nodes.first, outer_scope scope.in_macro?.should be_true end it "returns false otherwise" do - scope = Scope.new as_node "a = 1" + scope = Scope.new as_node("a = 1") scope.in_macro?.should be_false end end diff --git a/spec/ameba/ast/variabling/assignment_spec.cr b/spec/ameba/ast/variabling/assignment_spec.cr index fc91348a..354494ac 100644 --- a/spec/ameba/ast/variabling/assignment_spec.cr +++ b/spec/ameba/ast/variabling/assignment_spec.cr @@ -41,15 +41,14 @@ module Ameba::AST describe "#branch" do it "returns the branch of the assignment" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a) if a a = 3 # --> Crystal::Expressions puts a end end - ) - + CRYSTAL scope = Scope.new nodes.def_nodes.first variable = Variable.new(nodes.var_nodes.first, scope) assignment = Assignment.new(nodes.assign_nodes.first, variable, scope) @@ -58,7 +57,7 @@ module Ameba::AST end it "returns inner branch" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a, b) if a if b @@ -66,7 +65,7 @@ module Ameba::AST end end end - ) + CRYSTAL scope = Scope.new nodes.def_nodes.first variable = Variable.new(nodes.var_nodes.first, scope) assignment = Assignment.new(nodes.assign_nodes.first, variable, scope) @@ -75,12 +74,11 @@ module Ameba::AST end it "returns nil if assignment does not have a branch" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a) a = 2 end - ) - + CRYSTAL scope = Scope.new nodes.def_nodes.first variable = Variable.new(nodes.var_nodes.first, scope) assignment = Assignment.new(nodes.assign_nodes.first, variable, scope) @@ -90,12 +88,11 @@ module Ameba::AST describe "#transformed?" do it "returns false if the assignment is not transformed by the compiler" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method(a) a = 2 end - ) - + CRYSTAL scope = Scope.new nodes.def_nodes.first variable = Variable.new(nodes.var_nodes.first, scope) assignment = Assignment.new(nodes.assign_nodes.first, variable, scope) @@ -103,11 +100,10 @@ module Ameba::AST end it "returns true if the assignment is transformed by the compiler" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL array.each do |(a, b)| end - ) - + CRYSTAL scope = Scope.new nodes.block_nodes.first variable = Variable.new(nodes.var_nodes.first, scope) assignment = Assignment.new(nodes.assign_nodes.first, variable, scope) diff --git a/spec/ameba/ast/variabling/variable_spec.cr b/spec/ameba/ast/variabling/variable_spec.cr index 66b1a188..83792e2e 100644 --- a/spec/ameba/ast/variabling/variable_spec.cr +++ b/spec/ameba/ast/variabling/variable_spec.cr @@ -79,12 +79,12 @@ module Ameba::AST describe "#captured_by_block?" do it "returns truthy if the variable is captured by block" do - nodes = as_nodes %( + nodes = as_nodes <<-CRYSTAL def method a = 2 3.times { |i| a = a + i } end - ) + CRYSTAL scope = Scope.new nodes.def_nodes.first var_node = nodes.var_nodes.first scope.add_variable var_node @@ -95,12 +95,12 @@ module Ameba::AST variable.captured_by_block?.should be_truthy end - it "returns falsey if the variable is not captured by the block" do - scope = Scope.new as_node %( + it "returns falsy if the variable is not captured by the block" do + scope = Scope.new as_node <<-CRYSTAL def method a = 1 end - ) + CRYSTAL scope.add_variable Crystal::Var.new "a" variable = scope.variables.first variable.captured_by_block?.should be_falsey diff --git a/spec/ameba/ast/visitors/counting_visitor_spec.cr b/spec/ameba/ast/visitors/counting_visitor_spec.cr index db121152..3f9b343b 100644 --- a/spec/ameba/ast/visitors/counting_visitor_spec.cr +++ b/spec/ameba/ast/visitors/counting_visitor_spec.cr @@ -19,33 +19,33 @@ module Ameba::AST end it "is 1 if there is Macro::For" do - code = %( - def initialize() + code = <<-CRYSTAL + def initialize {% for c in ALL_NODES %} true || false {% end %} end - ) + CRYSTAL node = Crystal::Parser.new(code).parse visitor = CountingVisitor.new node visitor.count.should eq 1 end it "is 1 if there is Macro::If" do - code = %( - def initialize() + code = <<-CRYSTAL + def initialize {% if foo.bar? %} true || false {% end %} end - ) + CRYSTAL node = Crystal::Parser.new(code).parse visitor = CountingVisitor.new node visitor.count.should eq 1 end it "increases count for every exhaustive case" do - code = %( + code = <<-CRYSTAL def hello(a : Int32 | Int64 | Float32 | Float64) case a in Int32 then "int32" @@ -54,7 +54,7 @@ module Ameba::AST in Float64 then "float64" end end - ) + CRYSTAL node = Crystal::Parser.new(code).parse visitor = CountingVisitor.new node visitor.count.should eq 2 diff --git a/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr b/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr index 2e41ce53..5053ff67 100644 --- a/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr +++ b/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr @@ -6,17 +6,17 @@ module Ameba::AST describe FlowExpressionVisitor do it "creates an expression for return" do rule = FlowExpressionRule.new - FlowExpressionVisitor.new rule, Source.new %( + FlowExpressionVisitor.new rule, Source.new <<-CRYSTAL def foo return :bar end - ) + CRYSTAL rule.expressions.size.should eq 1 end it "can create multiple expressions" do rule = FlowExpressionRule.new - FlowExpressionVisitor.new rule, Source.new %( + FlowExpressionVisitor.new rule, Source.new <<-CRYSTAL def foo if bar return :baz @@ -24,42 +24,42 @@ module Ameba::AST return :foobar end end - ) + CRYSTAL rule.expressions.size.should eq 3 end it "properly creates nested flow expressions" do rule = FlowExpressionRule.new - FlowExpressionVisitor.new rule, Source.new %( + FlowExpressionVisitor.new rule, Source.new <<-CRYSTAL def foo - return( + return ( loop do break if a > 1 return a end ) end - ) + CRYSTAL rule.expressions.size.should eq 4 end it "creates an expression for break" do rule = FlowExpressionRule.new - FlowExpressionVisitor.new rule, Source.new %( + FlowExpressionVisitor.new rule, Source.new <<-CRYSTAL while true break end - ) + CRYSTAL rule.expressions.size.should eq 1 end it "creates an expression for next" do rule = FlowExpressionRule.new - FlowExpressionVisitor.new rule, Source.new %( + FlowExpressionVisitor.new rule, Source.new <<-CRYSTAL while true next if something end - ) + CRYSTAL rule.expressions.size.should eq 1 end 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 a523ad83..a9f8ae8c 100644 --- a/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr +++ b/spec/ameba/ast/visitors/redundant_control_expression_visitor_spec.cr @@ -5,11 +5,11 @@ module Ameba::AST rule = RedundantControlExpressionRule.new describe RedundantControlExpressionVisitor do - node = as_node %( + node = as_node <<-CRYSTAL a = 1 b = 2 return a + b - ) + CRYSTAL subject = RedundantControlExpressionVisitor.new(rule, source, node) it "assigns valid attributes" do diff --git a/spec/ameba/ast/visitors/scope_visitor_spec.cr b/spec/ameba/ast/visitors/scope_visitor_spec.cr index 98818fa5..a15402ee 100644 --- a/spec/ameba/ast/visitors/scope_visitor_spec.cr +++ b/spec/ameba/ast/visitors/scope_visitor_spec.cr @@ -4,37 +4,37 @@ module Ameba::AST describe ScopeVisitor do it "creates a scope for the def" do rule = ScopeRule.new - ScopeVisitor.new rule, Source.new %( + ScopeVisitor.new rule, Source.new <<-CRYSTAL def method end - ) + CRYSTAL rule.scopes.size.should eq 1 end it "creates a scope for the proc" do rule = ScopeRule.new - ScopeVisitor.new rule, Source.new %( + ScopeVisitor.new rule, Source.new <<-CRYSTAL -> {} - ) + CRYSTAL rule.scopes.size.should eq 1 end it "creates a scope for the block" do rule = ScopeRule.new - ScopeVisitor.new rule, Source.new %( + ScopeVisitor.new rule, Source.new <<-CRYSTAL 3.times {} - ) + CRYSTAL rule.scopes.size.should eq 2 end context "inner scopes" do it "creates scope for block inside def" do rule = ScopeRule.new - ScopeVisitor.new rule, Source.new %( + ScopeVisitor.new rule, Source.new <<-CRYSTAL def method 3.times {} end - ) + CRYSTAL rule.scopes.size.should eq 2 rule.scopes.last.outer_scope.should_not be_nil rule.scopes.first.outer_scope.should eq rule.scopes.last @@ -42,11 +42,11 @@ module Ameba::AST it "creates scope for block inside block" do rule = ScopeRule.new - ScopeVisitor.new rule, Source.new %( + ScopeVisitor.new rule, Source.new <<-CRYSTAL 3.times do 2.times {} end - ) + CRYSTAL rule.scopes.size.should eq 3 inner_block = rule.scopes.first outer_block = rule.scopes.last diff --git a/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr b/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr index 0575ff00..87f1ba59 100644 --- a/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr +++ b/spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr @@ -4,10 +4,12 @@ module Ameba::AST describe TopLevelNodesVisitor do describe "#require_nodes" do it "returns require node" do - source = Source.new %( + source = Source.new <<-CRYSTAL require "foo" - def bar; end - ) + + def bar + end + CRYSTAL visitor = TopLevelNodesVisitor.new(source.ast) visitor.require_nodes.size.should eq 1 visitor.require_nodes.first.to_s.should eq %q(require "foo") diff --git a/spec/ameba/inline_comments_spec.cr b/spec/ameba/inline_comments_spec.cr index 3f3b2e18..a9a6149f 100644 --- a/spec/ameba/inline_comments_spec.cr +++ b/spec/ameba/inline_comments_spec.cr @@ -26,135 +26,135 @@ module Ameba end it "disables a rule with a comment directive" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # ameba:disable #{NamedRule.name} Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {1, 12}, message: "Error!") - s.should be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {1, 12}, message: "Error!") + source.should be_valid end it "disables a rule with a line that ends with a comment directive" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL Time.epoch(1483859302) # ameba:disable #{NamedRule.name} - ) - s.add_issue(NamedRule.new, location: {1, 12}, message: "Error!") - s.should be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {1, 12}, message: "Error!") + source.should be_valid end it "does not disable a rule of a different name" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # ameba:disable WrongName Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "Error!") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "Error!") + source.should_not be_valid end it "disables a rule if multiple rule names provided" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # ameba:disable SomeRule LargeNumbers #{NamedRule.name} SomeOtherRule Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "") - s.should be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "") + source.should be_valid end it "disables a rule if multiple rule names are separated by comma" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # ameba:disable SomeRule, LargeNumbers, #{NamedRule.name}, SomeOtherRule Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "") - s.should be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "") + source.should be_valid end it "does not disable if multiple rule names used without required one" do - s = Source.new %( + source = Source.new <<-CRYSTAL # ameba:disable SomeRule, SomeOtherRule LargeNumbers Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "") + source.should_not be_valid end it "does not disable if comment directive has wrong place" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # ameba:disable #{NamedRule.name} # Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {3, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {3, 12}, message: "") + source.should_not be_valid end it "does not disable if comment directive added to the wrong line" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL if use_epoch? # ameba:disable #{NamedRule.name} Time.epoch(1483859302) end - ) - s.add_issue(NamedRule.new, location: {3, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {3, 12}, message: "") + source.should_not be_valid end it "does not disable if that is not a comment directive" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL "ameba:disable #{NamedRule.name}" Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {3, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {3, 12}, message: "") + source.should_not be_valid end it "does not disable if that is a commented out directive" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL # # ameba:disable #{NamedRule.name} Time.epoch(1483859302) - ) - s.add_issue(NamedRule.new, location: {3, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {3, 12}, message: "") + source.should_not be_valid end it "does not disable if that is an inline commented out directive" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL a = 1 # Disable it: # ameba:disable #{NamedRule.name} - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "") + source.should_not be_valid end context "with group name" do it "disables one rule with a group" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL a = 1 # ameba:disable #{DummyRule.rule_name} - ) - s.add_issue(DummyRule.new, location: {1, 12}, message: "") - s.should be_valid + CRYSTAL + source.add_issue(DummyRule.new, location: {1, 12}, message: "") + source.should be_valid end it "doesn't disable others rules" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL a = 1 # ameba:disable #{DummyRule.rule_name} - ) - s.add_issue(NamedRule.new, location: {2, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(NamedRule.new, location: {2, 12}, message: "") + source.should_not be_valid end it "disables a hole group of rules" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL a = 1 # ameba:disable #{DummyRule.group_name} - ) - s.add_issue(DummyRule.new, location: {1, 12}, message: "") - s.should be_valid + CRYSTAL + source.add_issue(DummyRule.new, location: {1, 12}, message: "") + source.should be_valid end it "does not disable rules which do not belong to the group" do - s = Source.new %Q( + source = Source.new <<-CRYSTAL a = 1 # ameba:disable Lint - ) - s.add_issue(DummyRule.new, location: {2, 12}, message: "") - s.should_not be_valid + CRYSTAL + source.add_issue(DummyRule.new, location: {2, 12}, message: "") + source.should_not be_valid end end end diff --git a/spec/ameba/rule/lint/duplicated_require_spec.cr b/spec/ameba/rule/lint/duplicated_require_spec.cr index 45d7961e..14eee4d0 100644 --- a/spec/ameba/rule/lint/duplicated_require_spec.cr +++ b/spec/ameba/rule/lint/duplicated_require_spec.cr @@ -24,12 +24,12 @@ module Ameba::Rule::Lint end it "reports rule, pos and message" do - source = Source.new %( + source = Source.new <<-CRYSTAL, "source.cr" require "./thing" require "./thing" require "./another_thing" require "./another_thing" - ), "source.cr" + CRYSTAL subject.catch(source).should_not be_valid diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index a7f86ef0..2c7b8988 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -13,7 +13,7 @@ module Ameba describe Rule::Lint::EmptyExpression do it "passes if there is no empty expression" do - s = Source.new %( + s = Source.new <<-CRYSTAL def method() end @@ -30,7 +30,7 @@ module Ameba begin "" end [nil] << nil - ) + CRYSTAL subject.catch(s).should be_valid end diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index a7758533..16ed0a24 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/spec/ameba/rule/performance/any_after_filter_spec.cr @@ -47,7 +47,7 @@ module Ameba::Rule::Performance context "properties" do it "allows to configure object_call_names" do - rule = Rule::Performance::AnyAfterFilter.new + rule = AnyAfterFilter.new rule.filter_names = %w(select) expect_no_issues rule, <<-CRYSTAL diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index 55823d9c..27e73fc9 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr @@ -63,7 +63,7 @@ module Ameba::Rule::Performance context "properties" do it "allows to configure object_call_names" do - rule = Rule::Performance::FirstLastAfterFilter.new + rule = FirstLastAfterFilter.new rule.filter_names = %w(reject) expect_no_issues rule, <<-CRYSTAL diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index 27034bbf..048a19ce 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/spec/ameba/rule/performance/size_after_filter_spec.cr @@ -45,7 +45,7 @@ module Ameba::Rule::Performance context "properties" do it "allows to configure object caller names" do - rule = Rule::Performance::SizeAfterFilter.new + rule = SizeAfterFilter.new rule.filter_names = %w(select) expect_no_issues rule, <<-CRYSTAL diff --git a/spec/ameba/rule/style/is_a_filter_spec.cr b/spec/ameba/rule/style/is_a_filter_spec.cr index ff53cfcb..c96c6b97 100644 --- a/spec/ameba/rule/style/is_a_filter_spec.cr +++ b/spec/ameba/rule/style/is_a_filter_spec.cr @@ -45,6 +45,7 @@ module Ameba::Rule::Style it "allows to configure filter_names" do rule = IsAFilter.new rule.filter_names = %w(select) + expect_no_issues rule, <<-CRYSTAL [1, 2, nil].reject(&.nil?) CRYSTAL diff --git a/spec/ameba/rule/style/parentheses_around_condition_spec.cr b/spec/ameba/rule/style/parentheses_around_condition_spec.cr index 6312fe0e..4db9c6b5 100644 --- a/spec/ameba/rule/style/parentheses_around_condition_spec.cr +++ b/spec/ameba/rule/style/parentheses_around_condition_spec.cr @@ -51,7 +51,7 @@ module Ameba::Rule::Style end it "allows to configure assignments" do - rule = Rule::Style::ParenthesesAroundCondition.new + rule = ParenthesesAroundCondition.new rule.exclude_ternary = false expect_issue rule, <<-CRYSTAL @@ -98,7 +98,7 @@ module Ameba::Rule::Style end it "allows to configure assignments" do - rule = Rule::Style::ParenthesesAroundCondition.new + rule = ParenthesesAroundCondition.new rule.allow_safe_assignment = true expect_issue rule, <<-CRYSTAL diff --git a/spec/ameba/rule/style/redundant_next_spec.cr b/spec/ameba/rule/style/redundant_next_spec.cr index 47fcec2d..f7eb0e50 100644 --- a/spec/ameba/rule/style/redundant_next_spec.cr +++ b/spec/ameba/rule/style/redundant_next_spec.cr @@ -211,7 +211,7 @@ module Ameba::Rule::Style end it "allows to configure multi next statements" do - rule = Rule::Style::RedundantNext.new + rule = RedundantNext.new rule.allow_multi_next = false source = expect_issue rule, <<-CRYSTAL block do |a, b| @@ -238,7 +238,7 @@ module Ameba::Rule::Style end it "allows to configure empty next statements" do - rule = Rule::Style::RedundantNext.new + rule = RedundantNext.new rule.allow_empty_next = false source = expect_issue rule, <<-CRYSTAL block do diff --git a/spec/ameba/rule/style/redundant_return_spec.cr b/spec/ameba/rule/style/redundant_return_spec.cr index 6fe207b8..82bc9658 100644 --- a/spec/ameba/rule/style/redundant_return_spec.cr +++ b/spec/ameba/rule/style/redundant_return_spec.cr @@ -294,7 +294,7 @@ module Ameba::Rule::Style end it "allows to configure multi returns" do - rule = Rule::Style::RedundantReturn.new + rule = RedundantReturn.new rule.allow_multi_return = false source = expect_issue rule, <<-CRYSTAL def method(a, b) @@ -321,7 +321,7 @@ module Ameba::Rule::Style end it "allows to configure empty returns" do - rule = Rule::Style::RedundantReturn.new + rule = RedundantReturn.new rule.allow_empty_return = false source = expect_issue rule, <<-CRYSTAL def method diff --git a/spec/ameba/rule/style/verbose_block_spec.cr b/spec/ameba/rule/style/verbose_block_spec.cr index 6d86e4b0..b638a65c 100644 --- a/spec/ameba/rule/style/verbose_block_spec.cr +++ b/spec/ameba/rule/style/verbose_block_spec.cr @@ -64,10 +64,12 @@ module Ameba::Rule::Style context "properties" do it "#exclude_calls_with_block" do rule = VerboseBlock.new + rule.exclude_calls_with_block = true expect_no_issues rule, <<-CRYSTAL (1..3).in_groups_of(1) { |i| i.map(&.to_s) } CRYSTAL + rule.exclude_calls_with_block = false source = expect_issue rule, <<-CRYSTAL (1..3).in_groups_of(1) { |i| i.map(&.to_s) } @@ -81,12 +83,14 @@ module Ameba::Rule::Style it "#exclude_multiple_line_blocks" do rule = VerboseBlock.new + rule.exclude_multiple_line_blocks = true expect_no_issues rule, <<-CRYSTAL (1..3).any? do |i| i.odd? end CRYSTAL + rule.exclude_multiple_line_blocks = false source = expect_issue rule, <<-CRYSTAL (1..3).any? do |i| @@ -102,12 +106,14 @@ module Ameba::Rule::Style it "#exclude_prefix_operators" do rule = VerboseBlock.new + rule.exclude_prefix_operators = true expect_no_issues rule, <<-CRYSTAL (1..3).sum { |i| +i } (1..3).sum { |i| -i } (1..3).sum { |i| ~i } CRYSTAL + rule.exclude_prefix_operators = false rule.exclude_operators = false source = expect_issue rule, <<-CRYSTAL @@ -128,10 +134,12 @@ module Ameba::Rule::Style it "#exclude_operators" do rule = VerboseBlock.new + rule.exclude_operators = true expect_no_issues rule, <<-CRYSTAL (1..3).sum { |i| i * 2 } CRYSTAL + rule.exclude_operators = false source = expect_issue rule, <<-CRYSTAL (1..3).sum { |i| i * 2 } @@ -145,10 +153,12 @@ module Ameba::Rule::Style it "#exclude_setters" do rule = VerboseBlock.new + rule.exclude_setters = true expect_no_issues rule, <<-CRYSTAL Char::Reader.new("abc").tap { |reader| reader.pos = 0 } CRYSTAL + rule.exclude_setters = false source = expect_issue rule, <<-CRYSTAL Char::Reader.new("abc").tap { |reader| reader.pos = 0 } @@ -163,12 +173,14 @@ module Ameba::Rule::Style it "#max_line_length" do rule = VerboseBlock.new rule.exclude_multiple_line_blocks = false + rule.max_line_length = 60 expect_no_issues rule, <<-CRYSTAL (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| i.to_s.reverse.strip.blank? end CRYSTAL + rule.max_line_length = nil source = expect_issue rule, <<-CRYSTAL (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| @@ -184,10 +196,12 @@ module Ameba::Rule::Style it "#max_length" do rule = VerboseBlock.new + rule.max_length = 30 expect_no_issues rule, <<-CRYSTAL (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } CRYSTAL + rule.max_length = nil source = expect_issue rule, <<-CRYSTAL (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } @@ -216,6 +230,7 @@ module Ameba::Rule::Style it "reports call args and named_args" do rule = VerboseBlock.new rule.exclude_operators = false + source = expect_issue rule, <<-CRYSTAL (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? } # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 0.to_i64, count: 3]?)` @@ -260,9 +275,9 @@ module Ameba::Rule::Style end it "reports rule, pos and message" do - source = Source.new path: "source.cr", code: %( + source = Source.new path: "source.cr", code: <<-CRYSTAL (1..3).any? { |i| i.odd? } - ) + CRYSTAL subject.catch(source).should_not be_valid source.issues.size.should eq 1 diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index f5ec70c7..9e9a3ade 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -65,6 +65,7 @@ module Ameba rules = [AtoAA.new] of Rule::Base source = Source.new "class A; end", "source.cr" message = "Infinite loop in source.cr caused by Ameba/AtoAA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run end @@ -95,11 +96,11 @@ module Ameba it "does not run other rules" do rules = [Rule::Lint::Syntax.new, Rule::Style::ConstantNames.new] of Rule::Base - source = Source.new %q( - MyBadConstant = 1 + source = Source.new <<-CRYSTAL + MyBadConstant = 1 - when my_bad_syntax - ) + when my_bad_syntax + CRYSTAL Runner.new(rules, [source], formatter, default_severity).run source.should_not be_valid @@ -110,9 +111,9 @@ module Ameba context "unneeded disables" do it "reports an issue if such disable exists" do rules = [Rule::Lint::UnneededDisableDirective.new] of Rule::Base - source = Source.new %( + source = Source.new <<-CRYSTAL a = 1 # ameba:disable LineLength - ) + CRYSTAL Runner.new(rules, [source], formatter, default_severity).run source.should_not be_valid @@ -134,9 +135,7 @@ module Ameba it "writes the explanation if sources are not valid and location found" do io.clear rules = [ErrorRule.new] of Rule::Base - source = Source.new %( - a = 1 - ), "source.cr" + source = Source.new "a = 1", "source.cr" runner = Runner.new(rules, [source], formatter, default_severity).run runner.explain({file: "source.cr", line: 1, column: 1}, io) @@ -146,9 +145,7 @@ module Ameba it "writes nothing if sources are not valid and location is not found" do io.clear rules = [ErrorRule.new] of Rule::Base - source = Source.new %( - a = 1 - ), "source.cr" + source = Source.new "a = 1", "source.cr" runner = Runner.new(rules, [source], formatter, default_severity).run runner.explain({file: "source.cr", line: 1, column: 2}, io) @@ -167,10 +164,9 @@ module Ameba it "returns false if there are invalid sources" do rules = Rule.rules.map &.new.as(Rule::Base) - s = Source.new %q( - WrongConstant = 5 - ) - Runner.new(rules, [s], formatter, default_severity).run.success?.should be_false + source = Source.new "WrongConstant = 5" + + Runner.new(rules, [source], formatter, default_severity).run.success?.should be_false end it "depends on the level of severity" do @@ -183,11 +179,11 @@ module Ameba it "returns false if issue is disabled" do rules = [NamedRule.new] of Rule::Base - source = Source.new %( + source = Source.new <<-CRYSTAL def foo bar = 1 # ameba:disable #{NamedRule.name} end - ) + CRYSTAL source.add_issue NamedRule.new, location: {2, 1}, message: "Useless assignment" @@ -204,6 +200,7 @@ module Ameba rules = [AtoB.new, BtoA.new] source = Source.new "class A; end", "source.cr" message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run end @@ -213,11 +210,12 @@ module Ameba context "if there are multiple offenses in an inspected file" do it "aborts because of an infinite loop" do rules = [AtoB.new, BtoA.new] - source = Source.new %( + source = Source.new <<-CRYSTAL, "source.cr" class A; end class A_A; end - ), "source.cr" + CRYSTAL message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run end @@ -230,6 +228,7 @@ module Ameba rules = [ClassToModule.new, ModuleToClass.new, AtoB.new, BtoA.new] source = Source.new "class A_A; end", "source.cr" message = "Infinite loop in source.cr caused by Ameba/ClassToModule, Ameba/AtoB -> Ameba/ModuleToClass, Ameba/BtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run end @@ -241,6 +240,7 @@ module Ameba rules = [AtoB.new, BtoC.new, CtoA.new] source = Source.new "class A; end", "source.cr" message = "Infinite loop in source.cr caused by Ameba/AtoB -> Ameba/BtoC -> Ameba/CtoA" + expect_raises(Runner::InfiniteCorrectionLoopError, message) do Runner.new(rules, [source], formatter, default_severity, autocorrect: true).run end diff --git a/src/ameba/rule/lint/ambiguous_assignment.cr b/src/ameba/rule/lint/ambiguous_assignment.cr index e07dc467..3fc0fd58 100644 --- a/src/ameba/rule/lint/ambiguous_assignment.cr +++ b/src/ameba/rule/lint/ambiguous_assignment.cr @@ -1,15 +1,21 @@ module Ameba::Rule::Lint # This rule checks for mistyped shorthand assignments. # - # # bad - # x =- y - # x =+ y - # x =! y + # This is considered invalid: # - # # good - # x -= y # or x = -y - # x += y # or x = +y - # x != y # or 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 + # ``` # # YAML configuration example: # @@ -43,9 +49,9 @@ module Ameba::Rule::Lint op_text = source_between(op_location, op_end_location, source.lines) return unless op_text - return unless MISTAKES.has_key?(op_text) + return unless suggestion = MISTAKES[op_text]? - issue_for op_location, op_end_location, MSG % MISTAKES[op_text] + issue_for op_location, op_end_location, MSG % suggestion end end end From 70078cf77f9e008f6a193cfdfca0d240d5b21018 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 19 Dec 2022 20:44:32 +0100 Subject: [PATCH 03/32] =?UTF-8?q?Synchronize=20properties=20context=20name?= =?UTF-8?q?s=20in=20rules=E2=80=99=20specs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/ameba/rule/layout/line_length_spec.cr | 2 +- spec/ameba/rule/lint/percent_arrays_spec.cr | 4 ++-- spec/ameba/rule/performance/any_after_filter_spec.cr | 2 +- spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr | 2 +- spec/ameba/rule/performance/first_last_after_filter_spec.cr | 2 +- spec/ameba/rule/performance/size_after_filter_spec.cr | 2 +- spec/ameba/rule/style/is_a_filter_spec.cr | 2 +- spec/ameba/rule/style/large_numbers_spec.cr | 2 +- spec/ameba/rule/style/parentheses_around_condition_spec.cr | 4 ++-- spec/ameba/rule/style/redundant_next_spec.cr | 2 +- spec/ameba/rule/style/redundant_return_spec.cr | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/ameba/rule/layout/line_length_spec.cr b/spec/ameba/rule/layout/line_length_spec.cr index 71a1b1ea..514f16a4 100644 --- a/spec/ameba/rule/layout/line_length_spec.cr +++ b/spec/ameba/rule/layout/line_length_spec.cr @@ -34,7 +34,7 @@ module Ameba::Rule::Layout end context "properties" do - it "allows to configure max length of the line" do + it "#max_length" do rule = LineLength.new rule.max_length = long_line.size diff --git a/spec/ameba/rule/lint/percent_arrays_spec.cr b/spec/ameba/rule/lint/percent_arrays_spec.cr index 42c46272..6f44367d 100644 --- a/spec/ameba/rule/lint/percent_arrays_spec.cr +++ b/spec/ameba/rule/lint/percent_arrays_spec.cr @@ -68,14 +68,14 @@ module Ameba::Rule::Lint end context "properties" do - it "allows to configure string_array_unwanted_symbols" do + it "#string_array_unwanted_symbols" do rule = PercentArrays.new rule.string_array_unwanted_symbols = "," s = Source.new %( %w("one") ) rule.catch(s).should be_valid end - it "allows to configure symbol_array_unwanted_symbols" do + it "#symbol_array_unwanted_symbols" do rule = PercentArrays.new rule.symbol_array_unwanted_symbols = "," s = Source.new %( %i(:one) ) diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index 16ed0a24..d99a6aa8 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/spec/ameba/rule/performance/any_after_filter_spec.cr @@ -46,7 +46,7 @@ module Ameba::Rule::Performance end context "properties" do - it "allows to configure object_call_names" do + it "#filter_names" do rule = AnyAfterFilter.new rule.filter_names = %w(select) diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr index 73100d39..123e36bf 100644 --- a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr +++ b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr @@ -44,7 +44,7 @@ module Ameba::Rule::Performance end context "properties" do - it "allows to configure `call_names`" do + it "#call_names" do rule = ChainedCallWithNoBang.new rule.call_names = %w(uniq) diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index 27e73fc9..4352c200 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr @@ -62,7 +62,7 @@ module Ameba::Rule::Performance end context "properties" do - it "allows to configure object_call_names" do + it "#filter_names" do rule = FirstLastAfterFilter.new rule.filter_names = %w(reject) diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index 048a19ce..fecedc5e 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/spec/ameba/rule/performance/size_after_filter_spec.cr @@ -44,7 +44,7 @@ module Ameba::Rule::Performance end context "properties" do - it "allows to configure object caller names" do + it "#filter_names" do rule = SizeAfterFilter.new rule.filter_names = %w(select) diff --git a/spec/ameba/rule/style/is_a_filter_spec.cr b/spec/ameba/rule/style/is_a_filter_spec.cr index c96c6b97..ed01d213 100644 --- a/spec/ameba/rule/style/is_a_filter_spec.cr +++ b/spec/ameba/rule/style/is_a_filter_spec.cr @@ -42,7 +42,7 @@ module Ameba::Rule::Style end context "properties" do - it "allows to configure filter_names" do + it "#filter_names" do rule = IsAFilter.new rule.filter_names = %w(select) diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index f7f87245..f29fc071 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -130,7 +130,7 @@ module Ameba end context "properties" do - it "allows to configure integer min digits" do + it "#int_min_digits" do rule = Rule::Style::LargeNumbers.new rule.int_min_digits = 10 expect_no_issues rule, %q(1200000) diff --git a/spec/ameba/rule/style/parentheses_around_condition_spec.cr b/spec/ameba/rule/style/parentheses_around_condition_spec.cr index 4db9c6b5..a8a174ad 100644 --- a/spec/ameba/rule/style/parentheses_around_condition_spec.cr +++ b/spec/ameba/rule/style/parentheses_around_condition_spec.cr @@ -43,7 +43,7 @@ module Ameba::Rule::Style end context "properties" do - context "#exclude_ternary=" do + context "#exclude_ternary" do it "skips ternary control expressions by default" do expect_no_issues subject, <<-CRYSTAL (foo > bar) ? true : false @@ -75,7 +75,7 @@ module Ameba::Rule::Style end end - context "#allow_safe_assignment=" do + context "#allow_safe_assignment" do it "reports assignments by default" do expect_issue subject, <<-CRYSTAL if (foo = @foo) diff --git a/spec/ameba/rule/style/redundant_next_spec.cr b/spec/ameba/rule/style/redundant_next_spec.cr index f7eb0e50..ed1e4123 100644 --- a/spec/ameba/rule/style/redundant_next_spec.cr +++ b/spec/ameba/rule/style/redundant_next_spec.cr @@ -201,7 +201,7 @@ module Ameba::Rule::Style end context "properties" do - context "#allow_multi_next=" do + context "#allow_multi_next" do it "allows multi next statements by default" do expect_no_issues subject, <<-CRYSTAL block do |a, b| diff --git a/spec/ameba/rule/style/redundant_return_spec.cr b/spec/ameba/rule/style/redundant_return_spec.cr index 82bc9658..634717d2 100644 --- a/spec/ameba/rule/style/redundant_return_spec.cr +++ b/spec/ameba/rule/style/redundant_return_spec.cr @@ -284,7 +284,7 @@ module Ameba::Rule::Style end context "properties" do - context "#allow_multi_return=" do + context "#allow_multi_return" do it "allows multi returns by default" do expect_no_issues subject, <<-CRYSTAL def method(a, b) From ab059616b565a9d647c529d1be9eaf531fc40452 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 19 Dec 2022 21:02:11 +0100 Subject: [PATCH 04/32] Switch from `Hash` to `NamedTuple` for `AmbiguousAssignment::MISTAKES` --- src/ameba/rule/lint/ambiguous_assignment.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ameba/rule/lint/ambiguous_assignment.cr b/src/ameba/rule/lint/ambiguous_assignment.cr index 3fc0fd58..f0f1bd7a 100644 --- a/src/ameba/rule/lint/ambiguous_assignment.cr +++ b/src/ameba/rule/lint/ambiguous_assignment.cr @@ -33,9 +33,9 @@ module Ameba::Rule::Lint MSG = "Suspicious assignment detected. Did you mean `%s`?" MISTAKES = { - "=-" => "-=", - "=+" => "+=", - "=!" => "!=", + "=-": "-=", + "=+": "+=", + "=!": "!=", } def test(source, node : Crystal::Assign) From 4e3caf2986456f7a74d7bdc6062a88c24bd491cb Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 20 Dec 2022 00:34:11 +0100 Subject: [PATCH 05/32] Few tweaks and readability refactors --- spec/ameba/rule/style/constant_names_spec.cr | 2 +- spec/spec_helper.cr | 4 ++++ src/ameba/rule/style/parentheses_around_condition.cr | 3 +-- src/ameba/source.cr | 1 + src/ameba/source/rewriter.cr | 4 +++- src/ameba/spec/annotated_source.cr | 12 +++++++++--- src/ameba/spec/be_valid.cr | 4 ++-- src/ameba/spec/support.cr | 4 ---- 8 files changed, 21 insertions(+), 13 deletions(-) diff --git a/spec/ameba/rule/style/constant_names_spec.cr b/spec/ameba/rule/style/constant_names_spec.cr index 0053ecf5..4629c0bc 100644 --- a/spec/ameba/rule/style/constant_names_spec.cr +++ b/spec/ameba/rule/style/constant_names_spec.cr @@ -17,7 +17,7 @@ module Ameba it "passes if type names are screaming-cased" do expect_no_issues subject, <<-CRYSTAL LUCKY_NUMBERS = [3, 7, 11] - DOCUMENTATION_URL = "http://crystal-lang.org/docs" + DOCUMENTATION_URL = "https://crystal-lang.org/docs" Int32 diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 60df2ed0..4a18746b 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -286,3 +286,7 @@ end def as_nodes(source) Ameba::TestNodeVisitor.new(as_node source) end + +def trailing_whitespace + ' ' +end diff --git a/src/ameba/rule/style/parentheses_around_condition.cr b/src/ameba/rule/style/parentheses_around_condition.cr index 5ddc15f4..a19dbabf 100644 --- a/src/ameba/rule/style/parentheses_around_condition.cr +++ b/src/ameba/rule/style/parentheses_around_condition.cr @@ -57,8 +57,7 @@ module Ameba::Rule::Style if cond.is_a?(Crystal::Assign) && allow_safe_assignment? issue_for cond, MSG_MISSING do |corrector| - corrector.insert_before(cond, '(') - corrector.insert_after(cond, ')') + corrector.wrap(cond, '(', ')') end return end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 38ce9b82..9811589e 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -27,6 +27,7 @@ module Ameba def correct corrector = Corrector.new(code) issues.each(&.correct(corrector)) + corrected_code = corrector.process return false if code == corrected_code diff --git a/src/ameba/source/rewriter.cr b/src/ameba/source/rewriter.cr index fc07c071..1deda8ec 100644 --- a/src/ameba/source/rewriter.cr +++ b/src/ameba/source/rewriter.cr @@ -125,7 +125,9 @@ 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") + raise IndexError.new( + "The range #{begin_pos}...#{end_pos} is outside the bounds of the source" + ) end end end diff --git a/src/ameba/spec/annotated_source.cr b/src/ameba/spec/annotated_source.cr index 874fd936..4f77e3d6 100644 --- a/src/ameba/spec/annotated_source.cr +++ b/src/ameba/spec/annotated_source.cr @@ -2,7 +2,8 @@ class Ameba::Spec::AnnotatedSource ANNOTATION_PATTERN_1 = /\A\s*(# )?(\^+|\^{})( error:)? / ANNOTATION_PATTERN_2 = " # error: " - ABBREV = "[...]" + + ABBREV = "[...]" getter lines : Array(String) @@ -15,6 +16,7 @@ class Ameba::Spec::AnnotatedSource def self.parse(annotated_code) lines = [] of String annotations = [] of {Int32, String, String} + code_lines = annotated_code.split('\n') # must preserve trailing newline code_lines.each do |code_line| case @@ -39,7 +41,9 @@ class Ameba::Spec::AnnotatedSource # NOTE: Annotations are sorted so that reconstructing the annotation # text via `#to_s` is deterministic. def initialize(@lines, annotations : Enumerable({Int32, String, String})) - @annotations = annotations.to_a.sort_by { |line, _, message| {line, message} } + @annotations = annotations.to_a.sort_by do |line, _, message| + {line, message} + end end # Annotates the source code with the Ameba issues provided. @@ -47,7 +51,9 @@ class Ameba::Spec::AnnotatedSource # NOTE: Annotations are sorted so that reconstructing the annotation # text via `#to_s` is deterministic. def initialize(@lines, issues : Enumerable(Issue)) - @annotations = issues_to_annotations(issues).sort_by { |line, _, message| {line, message} } + @annotations = issues_to_annotations(issues).sort_by do |line, _, message| + {line, message} + end end def ==(other) diff --git a/src/ameba/spec/be_valid.cr b/src/ameba/spec/be_valid.cr index 6247c697..f4eb3063 100644 --- a/src/ameba/spec/be_valid.cr +++ b/src/ameba/spec/be_valid.cr @@ -13,8 +13,8 @@ module Ameba::Spec def failure_message(source) String.build do |str| str << "Source expected to be valid, but there are issues: \n\n" - source.issues.reject(&.disabled?).each do |e| - str << " * #{e.rule.name}: #{e.message}\n" + source.issues.reject(&.disabled?).each do |issue| + str << " * #{issue.rule.name}: #{issue.message}\n" end end end diff --git a/src/ameba/spec/support.cr b/src/ameba/spec/support.cr index 2813ce15..851401f6 100644 --- a/src/ameba/spec/support.cr +++ b/src/ameba/spec/support.cr @@ -14,9 +14,5 @@ module Ameba end end -def trailing_whitespace - ' ' -end - include Ameba::Spec::BeValid include Ameba::Spec::ExpectIssue From 206b5ab604923ac1cc180a48a5e57a6e0b6f5fec Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 20 Dec 2022 14:53:23 +0100 Subject: [PATCH 06/32] Refactor: `Source#correct` -> `Source#correct?` --- 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 c2c5ac51..f8851d40 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -130,7 +130,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 9811589e..c985bd09 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 95d3b9ad..3cf1933a 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: From 6aa36f3d9eca8b5b310533b6f6d5ba71e42c8f1a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 20 Dec 2022 17:09:28 +0100 Subject: [PATCH 07/32] Remove OpenSSL from Docker image --- Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index f04f800b..472eb868 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,12 +1,12 @@ FROM alpine:edge as builder -RUN apk add --update crystal shards openssl-dev yaml-dev musl-dev make +RUN apk add --update crystal shards yaml-dev musl-dev make RUN mkdir /ameba WORKDIR /ameba COPY . /ameba/ RUN make clean && make FROM alpine:latest -RUN apk add --update openssl yaml pcre gc libevent libgcc +RUN apk add --update yaml pcre gc libevent libgcc RUN mkdir /src WORKDIR /src COPY --from=builder /ameba/bin/ameba /usr/bin/ From e76de37b7d27ce6613a171cae3858794c9c42f59 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 16:08:16 +0100 Subject: [PATCH 08/32] Update GitHub Actions --- .github/workflows/ci.yml | 6 ++---- .github/workflows/docs.yml | 21 +++++++++------------ 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0298a508..cd3f3546 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,17 +17,15 @@ jobs: steps: - name: Install Crystal - uses: oprypin/install-crystal@v1 + uses: crystal-lang/install-crystal@v1 with: crystal: ${{ matrix.crystal }} - name: Download source - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Install dependencies run: shards install - env: - SHARDS_OPTS: --ignore-crystal-version - name: Run specs run: make test diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 14b60f5f..8848dba4 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -9,20 +9,18 @@ jobs: runs-on: ubuntu-latest steps: - name: Inject slug/short variables - uses: rlespinasse/github-slug-action@v3.x + uses: rlespinasse/github-slug-action@v4 - name: Install Crystal - uses: oprypin/install-crystal@v1 + uses: crystal-lang/install-crystal@v1 - name: Download source - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: persist-credentials: false - name: Install dependencies run: shards install - env: - SHARDS_OPTS: --ignore-crystal-version - name: Build docs run: | @@ -30,7 +28,7 @@ jobs: crystal docs --project-version="${{ env.GITHUB_REF_SLUG }}" --source-refname="${{ env.GITHUB_SHA_SHORT }}" - name: Upload artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: docs path: docs @@ -40,15 +38,14 @@ jobs: runs-on: ubuntu-latest steps: - name: Download artifacts - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v3 with: name: docs path: docs - name: Deploy docs 🚀 - uses: JamesIves/github-pages-deploy-action@3.7.1 + uses: JamesIves/github-pages-deploy-action@4 with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - BRANCH: gh-pages - FOLDER: docs - CLEAN: true + branch: gh-pages + folder: docs + clean: true From 5ff8d2593afc19e984a6407f23915427c1aecdcd Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 16:09:05 +0100 Subject: [PATCH 09/32] Add Dependabot config --- .github/dependabot.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..52ded593 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,7 @@ +version: 2 +updates: + # Maintain dependencies for GitHub Actions + - package-ecosystem: github-actions + directory: / + schedule: + interval: daily From 7c617b5a7b4ee3401ce20e912c1b196112bf2314 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 17:05:28 +0100 Subject: [PATCH 10/32] Make spec helper methods private --- spec/ameba/rule/lint/empty_expression_spec.cr | 2 +- spec/ameba/rule/style/guard_clause_spec.cr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index 2c7b8988..5b6d92b0 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -3,7 +3,7 @@ require "../../../spec_helper" module Ameba subject = Rule::Lint::EmptyExpression.new - def it_detects_empty_expression(code) + private def it_detects_empty_expression(code) it "detects empty expression" do s = Source.new code rule = Rule::Lint::EmptyExpression.new diff --git a/spec/ameba/rule/style/guard_clause_spec.cr b/spec/ameba/rule/style/guard_clause_spec.cr index 6954b736..df372957 100644 --- a/spec/ameba/rule/style/guard_clause_spec.cr +++ b/spec/ameba/rule/style/guard_clause_spec.cr @@ -3,7 +3,7 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::GuardClause.new - def it_reports_body(body, *, line = __LINE__) + private def it_reports_body(body, *, line = __LINE__) rule = Rule::Style::GuardClause.new it "reports an issue if method body is if / unless without else" do @@ -75,7 +75,7 @@ module Ameba end end - def it_reports_control_expression(kw, *, line = __LINE__) + private def it_reports_control_expression(kw, *, line = __LINE__) rule = Rule::Style::GuardClause.new it "reports an issue with #{kw} in the if branch" do From e9f3bbaeffa9043335af1379ea28dd8c9d31fab0 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 17:09:42 +0100 Subject: [PATCH 11/32] Fail with the additional context --- spec/ameba/rule/lint/empty_expression_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index 5b6d92b0..c2635a5c 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -4,7 +4,7 @@ module Ameba subject = Rule::Lint::EmptyExpression.new private def it_detects_empty_expression(code) - it "detects empty expression" do + it %(detects empty expression "#{code}") do s = Source.new code rule = Rule::Lint::EmptyExpression.new rule.catch(s).should_not be_valid From 1ba6fcb55c8f18eb80ebc8d510b3999b16592098 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 17:23:56 +0100 Subject: [PATCH 12/32] Enhance spec helpers a bit in re: to the spec context --- spec/ameba/rule/lint/empty_expression_spec.cr | 6 +-- spec/ameba/rule/style/constant_names_spec.cr | 6 +-- spec/ameba/rule/style/guard_clause_spec.cr | 50 +++++++++---------- spec/ameba/rule/style/large_numbers_spec.cr | 6 +-- spec/ameba/rule/style/method_names_spec.cr | 6 +-- spec/ameba/rule/style/type_names_spec.cr | 6 +-- spec/ameba/rule/style/variable_names_spec.cr | 6 +-- spec/ameba/tokenizer_spec.cr | 8 +-- 8 files changed, 47 insertions(+), 47 deletions(-) diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index c2635a5c..7918d094 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -3,11 +3,11 @@ require "../../../spec_helper" module Ameba subject = Rule::Lint::EmptyExpression.new - private def it_detects_empty_expression(code) - it %(detects empty expression "#{code}") do + private def it_detects_empty_expression(code, *, file = __FILE__, line = __LINE__) + it %(detects empty expression "#{code}"), file, line do s = Source.new code rule = Rule::Lint::EmptyExpression.new - rule.catch(s).should_not be_valid + rule.catch(s).should_not be_valid, file: file, line: line end end diff --git a/spec/ameba/rule/style/constant_names_spec.cr b/spec/ameba/rule/style/constant_names_spec.cr index 4629c0bc..00267289 100644 --- a/spec/ameba/rule/style/constant_names_spec.cr +++ b/spec/ameba/rule/style/constant_names_spec.cr @@ -3,10 +3,10 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::ConstantNames.new - private def it_reports_constant(name, value, expected) - it "reports constant name #{expected}" do + private def it_reports_constant(name, value, expected, *, file = __FILE__, line = __LINE__) + it "reports constant name #{expected}", file, line do rule = Rule::Style::ConstantNames.new - expect_issue rule, <<-CRYSTAL, name: name + expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line %{name} = #{value} # ^{name} error: Constant name should be screaming-cased: #{expected}, not #{name} CRYSTAL diff --git a/spec/ameba/rule/style/guard_clause_spec.cr b/spec/ameba/rule/style/guard_clause_spec.cr index df372957..fc09fdef 100644 --- a/spec/ameba/rule/style/guard_clause_spec.cr +++ b/spec/ameba/rule/style/guard_clause_spec.cr @@ -3,11 +3,11 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::GuardClause.new - private def it_reports_body(body, *, line = __LINE__) + private def it_reports_body(body, *, file = __FILE__, line = __LINE__) rule = Rule::Style::GuardClause.new - it "reports an issue if method body is if / unless without else" do - source = expect_issue rule, <<-CRYSTAL, line: line + it "reports an issue if method body is if / unless without else", file, line do + source = expect_issue rule, <<-CRYSTAL, file: file, line: line def func if something # ^^ error: Use a guard clause (`return unless something`) instead of wrapping the code inside a conditional expression. @@ -23,7 +23,7 @@ module Ameba end CRYSTAL - expect_correction source, <<-CRYSTAL, line: line + expect_correction source, <<-CRYSTAL, file: file, line: line def func return unless something #{body} @@ -38,8 +38,8 @@ module Ameba CRYSTAL end - it "reports an issue if method body ends with if / unless without else" do - source = expect_issue rule, <<-CRYSTAL, line: line + it "reports an issue if method body ends with if / unless without else", file, line do + source = expect_issue rule, <<-CRYSTAL, file: file, line: line def func test if something @@ -57,7 +57,7 @@ module Ameba end CRYSTAL - expect_correction source, <<-CRYSTAL, line: line + expect_correction source, <<-CRYSTAL, file: file, line: line def func test return unless something @@ -75,11 +75,11 @@ module Ameba end end - private def it_reports_control_expression(kw, *, line = __LINE__) + private def it_reports_control_expression(kw, *, file = __FILE__, line = __LINE__) rule = Rule::Style::GuardClause.new - it "reports an issue with #{kw} in the if branch" do - source = expect_issue rule, <<-CRYSTAL, line: line + it "reports an issue with #{kw} in the if branch", file, line do + source = expect_issue rule, <<-CRYSTAL, file: file, line: line def func if something # ^^ error: Use a guard clause (`#{kw} if something`) instead of wrapping the code inside a conditional expression. @@ -90,11 +90,11 @@ module Ameba end CRYSTAL - expect_no_corrections source, line: line + expect_no_corrections source, file: file, line: line end - it "reports an issue with #{kw} in the else branch" do - source = expect_issue rule, <<-CRYSTAL, line: line + it "reports an issue with #{kw} in the else branch", file, line do + source = expect_issue rule, <<-CRYSTAL, file: file, line: line def func if something # ^^ error: Use a guard clause (`#{kw} unless something`) instead of wrapping the code inside a conditional expression. @@ -105,11 +105,11 @@ module Ameba end CRYSTAL - expect_no_corrections source, line: line + expect_no_corrections source, file: file, line: line end - it "doesn't report an issue if condition has multiple lines" do - expect_no_issues rule, <<-CRYSTAL, line: line + it "doesn't report an issue if condition has multiple lines", file, line do + expect_no_issues rule, <<-CRYSTAL, file: file, line: line def func if something && something_else @@ -121,8 +121,8 @@ module Ameba CRYSTAL end - it "does not report an issue if #{kw} is inside elsif" do - expect_no_issues rule, <<-CRYSTAL, line: line + it "does not report an issue if #{kw} is inside elsif", file, line do + expect_no_issues rule, <<-CRYSTAL, file: file, line: line def func if something a @@ -133,8 +133,8 @@ module Ameba CRYSTAL end - it "does not report an issue if #{kw} is inside if..elsif..else..end" do - expect_no_issues rule, <<-CRYSTAL, line: line + it "does not report an issue if #{kw} is inside if..elsif..else..end", file, line do + expect_no_issues rule, <<-CRYSTAL, file: file, line: line def func if something a @@ -147,8 +147,8 @@ module Ameba CRYSTAL end - it "doesn't report an issue if control flow expr has multiple lines" do - expect_no_issues rule, <<-CRYSTAL, line: line + it "doesn't report an issue if control flow expr has multiple lines", file, line do + expect_no_issues rule, <<-CRYSTAL, file: file, line: line def func if something #{kw} \\ @@ -161,8 +161,8 @@ module Ameba CRYSTAL end - it "reports an issue if non-control-flow branch has multiple lines" do - source = expect_issue rule, <<-CRYSTAL, line: line + it "reports an issue if non-control-flow branch has multiple lines", file, line do + source = expect_issue rule, <<-CRYSTAL, file: file, line: line def func if something # ^^ error: Use a guard clause (`#{kw} if something`) instead of wrapping the code inside a conditional expression. @@ -174,7 +174,7 @@ module Ameba end CRYSTAL - expect_no_corrections source, line: line + expect_no_corrections source, file: file, line: line end end diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index f29fc071..4682aae2 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -3,12 +3,12 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::LargeNumbers.new - private def it_transforms(number, expected) - it "transforms large number #{number}" do + private def it_transforms(number, expected, *, file = __FILE__, line = __LINE__) + it "transforms large number #{number}", file, line do rule = Rule::Style::LargeNumbers.new rule.int_min_digits = 5 - source = expect_issue rule, <<-CRYSTAL, number: number + source = expect_issue rule, <<-CRYSTAL, number: number, file: file, line: line number = %{number} # ^{number} error: Large numbers should be written with underscores: #{expected} CRYSTAL diff --git a/spec/ameba/rule/style/method_names_spec.cr b/spec/ameba/rule/style/method_names_spec.cr index 02893ee5..94d2608d 100644 --- a/spec/ameba/rule/style/method_names_spec.cr +++ b/spec/ameba/rule/style/method_names_spec.cr @@ -3,10 +3,10 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::MethodNames.new - private def it_reports_method_name(name, expected) - it "reports method name #{expected}" do + private def it_reports_method_name(name, expected, *, file = __FILE__, line = __LINE__) + it "reports method name #{expected}", file, line do rule = Rule::Style::MethodNames.new - expect_issue rule, <<-CRYSTAL, name: name + expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line def %{name}; end # ^{name} error: Method name should be underscore-cased: #{expected}, not %{name} CRYSTAL diff --git a/spec/ameba/rule/style/type_names_spec.cr b/spec/ameba/rule/style/type_names_spec.cr index b856ed2b..31abc89b 100644 --- a/spec/ameba/rule/style/type_names_spec.cr +++ b/spec/ameba/rule/style/type_names_spec.cr @@ -3,10 +3,10 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::TypeNames.new - private def it_reports_name(type, name, expected) - it "reports type name #{expected}" do + private def it_reports_name(type, name, expected, *, file = __FILE__, line = __LINE__) + it "reports type name #{expected}", file, line do rule = Rule::Style::TypeNames.new - expect_issue rule, <<-CRYSTAL, type: type, name: name + expect_issue rule, <<-CRYSTAL, type: type, name: name, file: file, line: line %{type} %{name}; end # ^{type}^{name}^^^^ error: Type name should be camelcased: #{expected}, but it was %{name} CRYSTAL diff --git a/spec/ameba/rule/style/variable_names_spec.cr b/spec/ameba/rule/style/variable_names_spec.cr index 2e28cf9c..3443ddcb 100644 --- a/spec/ameba/rule/style/variable_names_spec.cr +++ b/spec/ameba/rule/style/variable_names_spec.cr @@ -3,10 +3,10 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::VariableNames.new - private def it_reports_var_name(name, value, expected) - it "reports variable name #{expected}" do + private def it_reports_var_name(name, value, expected, *, file = __FILE__, line = __LINE__) + it "reports variable name #{expected}", file, line do rule = Rule::Style::VariableNames.new - expect_issue rule, <<-CRYSTAL, name: name + expect_issue rule, <<-CRYSTAL, name: name, file: file, line: line %{name} = #{value} # ^{name} error: Var name should be underscore-cased: #{expected}, not %{name} CRYSTAL diff --git a/spec/ameba/tokenizer_spec.cr b/spec/ameba/tokenizer_spec.cr index 674ade29..b014a30d 100644 --- a/spec/ameba/tokenizer_spec.cr +++ b/spec/ameba/tokenizer_spec.cr @@ -1,13 +1,13 @@ require "../spec_helper" module Ameba - private def it_tokenizes(str, expected) - it "tokenizes #{str}" do - ([] of String).tap do |token_types| + private def it_tokenizes(str, expected, *, file = __FILE__, line = __LINE__) + it "tokenizes #{str}", file, line do + %w[].tap do |token_types| Tokenizer.new(Source.new str, normalize: false) .run { |token| token_types << token.type.to_s } .should be_true - end.should eq expected + end.should eq(expected), file: file, line: line end end From 9926f0295aba0b5669c165508ec880ca5a9005dc Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 21:13:01 +0100 Subject: [PATCH 13/32] Remove most of the obsolete specs --- .../rule/layout/trailing_whitespace_spec.cr | 11 --------- .../rule/lint/comparison_to_boolean_spec.cr | 21 ----------------- spec/ameba/rule/lint/debug_calls_spec.cr | 11 --------- .../rule/lint/debugger_statement_spec.cr | 11 --------- .../rule/lint/duplicated_require_spec.cr | 23 ------------------- spec/ameba/rule/lint/empty_expression_spec.cr | 13 ----------- spec/ameba/rule/lint/empty_loop_spec.cr | 16 ------------- .../rule/lint/hash_duplicated_key_spec.cr | 12 ---------- .../rule/lint/literals_comparison_spec.cr | 13 ----------- .../rule/lint/not_nil_after_no_bang_spec.cr | 13 ----------- spec/ameba/rule/lint/not_nil_spec.cr | 13 ----------- spec/ameba/rule/lint/rand_zero_spec.cr | 11 --------- .../lint/shadowing_outer_local_var_spec.cr | 14 ----------- .../rule/lint/shared_var_in_fiber_spec.cr | 19 --------------- spec/ameba/rule/lint/syntax_spec.cr | 10 -------- .../lint/useless_condition_in_when_spec.cr | 17 -------------- .../rule/performance/any_after_filter_spec.cr | 9 -------- .../performance/any_instead_of_empty_spec.cr | 13 ----------- .../chained_call_with_no_bang_spec.cr | 16 ------------- .../performance/compact_after_map_spec.cr | 13 ----------- .../first_last_after_filter_spec.cr | 15 ------------ .../performance/flatten_after_map_spec.cr | 13 ----------- .../performance/map_instead_of_block_spec.cr | 13 ----------- .../performance/size_after_filter_spec.cr | 13 ----------- spec/ameba/rule/style/constant_names_spec.cr | 14 ----------- spec/ameba/rule/style/is_a_filter_spec.cr | 15 ------------ spec/ameba/rule/style/is_a_nil_spec.cr | 14 ----------- spec/ameba/rule/style/large_numbers_spec.cr | 12 ---------- spec/ameba/rule/style/method_names_spec.cr | 15 ------------ .../negated_conditions_in_unless_spec.cr | 11 --------- spec/ameba/rule/style/predicate_name_spec.cr | 18 --------------- spec/ameba/rule/style/redundant_begin_spec.cr | 19 --------------- spec/ameba/rule/style/type_names_spec.cr | 15 ------------ spec/ameba/rule/style/unless_else_spec.cr | 18 --------------- spec/ameba/rule/style/variable_names_spec.cr | 14 ----------- spec/ameba/rule/style/verbose_block_spec.cr | 15 ------------ 36 files changed, 513 deletions(-) diff --git a/spec/ameba/rule/layout/trailing_whitespace_spec.cr b/spec/ameba/rule/layout/trailing_whitespace_spec.cr index b6d9902d..9885e14f 100644 --- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr @@ -15,16 +15,5 @@ module Ameba::Rule::Layout expect_correction source, "whitespace at the end" end - - it "reports rule, pos and message" do - source = Source.new "a = 1\n b = 2 ", "source.cr" - subject.catch(source).should_not be_valid - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:7" - issue.end_location.to_s.should eq "source.cr:2:7" - issue.message.should eq "Trailing whitespace detected" - end end end diff --git a/spec/ameba/rule/lint/comparison_to_boolean_spec.cr b/spec/ameba/rule/lint/comparison_to_boolean_spec.cr index e72278d8..e99eba97 100644 --- a/spec/ameba/rule/lint/comparison_to_boolean_spec.cr +++ b/spec/ameba/rule/lint/comparison_to_boolean_spec.cr @@ -94,16 +94,6 @@ module Ameba::Rule::Lint a CRYSTAL end - - it "reports rule, pos and message" do - source = Source.new "a != true", "source.cr" - subject.catch(source) - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.message.should eq "Comparison to a boolean is pointless" - end end context "boolean on the left" do @@ -165,17 +155,6 @@ module Ameba::Rule::Lint a CRYSTAL end - - it "reports rule, pos and message" do - source = Source.new "true != a", "source.cr" - subject.catch(source).should_not be_valid - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:9" - issue.message.should eq "Comparison to a boolean is pointless" - end end end end diff --git a/spec/ameba/rule/lint/debug_calls_spec.cr b/spec/ameba/rule/lint/debug_calls_spec.cr index bd46aba9..ffdc90ea 100644 --- a/spec/ameba/rule/lint/debug_calls_spec.cr +++ b/spec/ameba/rule/lint/debug_calls_spec.cr @@ -28,16 +28,5 @@ module Ameba::Rule::Lint CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new "pp! :foo", "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:8" - issue.message.should eq "Possibly forgotten debug-related `pp!` call detected" - end end end diff --git a/spec/ameba/rule/lint/debugger_statement_spec.cr b/spec/ameba/rule/lint/debugger_statement_spec.cr index 2f67e351..ad6942df 100644 --- a/spec/ameba/rule/lint/debugger_statement_spec.cr +++ b/spec/ameba/rule/lint/debugger_statement_spec.cr @@ -31,16 +31,5 @@ module Ameba::Rule::Lint expect_no_corrections source end - - it "reports rule, pos and message" do - s = Source.new "debugger", "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:8" - issue.message.should eq "Possible forgotten debugger statement detected" - end end end diff --git a/spec/ameba/rule/lint/duplicated_require_spec.cr b/spec/ameba/rule/lint/duplicated_require_spec.cr index 14eee4d0..6967d764 100644 --- a/spec/ameba/rule/lint/duplicated_require_spec.cr +++ b/spec/ameba/rule/lint/duplicated_require_spec.cr @@ -22,28 +22,5 @@ module Ameba::Rule::Lint expect_no_corrections source end - - it "reports rule, pos and message" do - source = Source.new <<-CRYSTAL, "source.cr" - require "./thing" - require "./thing" - require "./another_thing" - require "./another_thing" - CRYSTAL - - subject.catch(source).should_not be_valid - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:1" - issue.end_location.to_s.should eq "" - issue.message.should eq "Duplicated require of `./thing`" - - issue = source.issues.last - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:4:1" - issue.end_location.to_s.should eq "" - issue.message.should eq "Duplicated require of `./another_thing`" - end end end diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index 7918d094..9e9bbee7 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -110,18 +110,5 @@ module Ameba ) subject.catch(s).should be_valid end - - it "reports rule, location and message" do - s = Source.new %( - if () - end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:4" - issue.end_location.to_s.should eq "source.cr:1:5" - issue.message.should eq "Avoid empty expressions" - end end end diff --git a/spec/ameba/rule/lint/empty_loop_spec.cr b/spec/ameba/rule/lint/empty_loop_spec.cr index d2a8c21e..3d0fc0c2 100644 --- a/spec/ameba/rule/lint/empty_loop_spec.cr +++ b/spec/ameba/rule/lint/empty_loop_spec.cr @@ -64,21 +64,5 @@ module Ameba::Rule::Lint end CRYSTAL end - - it "reports rule, message and location" do - s = Source.new %( - a = 1 - loop do - # comment goes here - end - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:1" - issue.end_location.to_s.should eq "source.cr:4:3" - issue.message.should eq EmptyLoop::MSG - end end end diff --git a/spec/ameba/rule/lint/hash_duplicated_key_spec.cr b/spec/ameba/rule/lint/hash_duplicated_key_spec.cr index def877f7..e0093f88 100644 --- a/spec/ameba/rule/lint/hash_duplicated_key_spec.cr +++ b/spec/ameba/rule/lint/hash_duplicated_key_spec.cr @@ -32,17 +32,5 @@ module Ameba::Rule::Lint # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "key1", "key2" CRYSTAL end - - it "reports rule, location and message" do - s = Source.new %q( - h = {"a" => 1, "a" => 2} - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:5" - issue.end_location.to_s.should eq "source.cr:1:24" - issue.message.should eq %(Duplicated keys in hash literal: "a") - end end end diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr index 6f6c5004..c0040d91 100644 --- a/spec/ameba/rule/lint/literals_comparison_spec.cr +++ b/spec/ameba/rule/lint/literals_comparison_spec.cr @@ -58,18 +58,5 @@ module Ameba::Rule::Lint CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - "foo" == "foo" - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:14" - issue.message.should eq "Comparison always evaluates to true" - end end end diff --git a/spec/ameba/rule/lint/not_nil_after_no_bang_spec.cr b/spec/ameba/rule/lint/not_nil_after_no_bang_spec.cr index 521b79ef..954d0d7a 100644 --- a/spec/ameba/rule/lint/not_nil_after_no_bang_spec.cr +++ b/spec/ameba/rule/lint/not_nil_after_no_bang_spec.cr @@ -58,18 +58,5 @@ module Ameba::Rule::Lint CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - (1..3).index(1).not_nil! - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:8" - issue.end_location.to_s.should eq "source.cr:1:24" - issue.message.should eq "Use `index! {...}` instead of `index {...}.not_nil!`" - end end end diff --git a/spec/ameba/rule/lint/not_nil_spec.cr b/spec/ameba/rule/lint/not_nil_spec.cr index 3359009e..0f5881b2 100644 --- a/spec/ameba/rule/lint/not_nil_spec.cr +++ b/spec/ameba/rule/lint/not_nil_spec.cr @@ -32,18 +32,5 @@ module Ameba::Rule::Lint CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - (1..3).first?.not_nil! - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:15" - issue.end_location.to_s.should eq "source.cr:1:22" - issue.message.should eq "Avoid using `not_nil!`" - end end end diff --git a/spec/ameba/rule/lint/rand_zero_spec.cr b/spec/ameba/rule/lint/rand_zero_spec.cr index cb5cc6d4..e30e1ff8 100644 --- a/spec/ameba/rule/lint/rand_zero_spec.cr +++ b/spec/ameba/rule/lint/rand_zero_spec.cr @@ -25,16 +25,5 @@ module Ameba::Rule::Lint # ^^^^^ error: rand(1) always returns 0 CRYSTAL end - - it "reports rule, location and a message" do - s = Source.new "rand(1)", "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:7" - issue.message.should eq "rand(1) always returns 0" - end end end diff --git a/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr b/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr index 72340ebb..ba44bc11 100644 --- a/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr +++ b/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr @@ -157,20 +157,6 @@ module Ameba::Rule::Lint CRYSTAL end - it "reports rule, location and message" do - source = Source.new %( - foo = 1 - 3.times { |foo| foo + 1 } - ), "source.cr" - subject.catch(source).should_not be_valid - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:12" - issue.end_location.should be_nil - issue.message.should eq "Shadowing outer local variable `foo`" - end - context "macro" do it "does not report shadowed vars in outer scope" do expect_no_issues subject, <<-CRYSTAL diff --git a/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr b/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr index 221845c2..3381c935 100644 --- a/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr +++ b/spec/ameba/rule/lint/shared_var_in_fiber_spec.cr @@ -194,24 +194,5 @@ module Ameba::Rule::Lint end CRYSTAL end - - it "reports rule, location and message" do - s = Source.new %( - i = 0 - while true - i += 1 - spawn { i } - end - ), "source.cr" - - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:4:11" - issue.end_location.to_s.should eq "source.cr:4:11" - issue.message.should eq "Shared variable `i` is used in fiber" - end end end diff --git a/spec/ameba/rule/lint/syntax_spec.cr b/spec/ameba/rule/lint/syntax_spec.cr index 107752ac..fbbe47c3 100644 --- a/spec/ameba/rule/lint/syntax_spec.cr +++ b/spec/ameba/rule/lint/syntax_spec.cr @@ -23,16 +23,6 @@ module Ameba::Rule::Lint CRYSTAL end - it "reports rule, location and message" do - s = Source.new "def hello end", "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.message.should match /unexpected token: "?end"? \(expected ["'];["'] or newline\)/ - end - it "has highest severity" do subject.severity.should eq Severity::Error end diff --git a/spec/ameba/rule/lint/useless_condition_in_when_spec.cr b/spec/ameba/rule/lint/useless_condition_in_when_spec.cr index 9fffc4a9..c31e3e31 100644 --- a/spec/ameba/rule/lint/useless_condition_in_when_spec.cr +++ b/spec/ameba/rule/lint/useless_condition_in_when_spec.cr @@ -24,22 +24,5 @@ module Ameba::Rule::Lint end CRYSTAL end - - it "reports rule, location and message" do - s = Source.new %( - case - when String - puts "hello" - when can_generate? - generate if can_generate? - end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:5:15" - issue.end_location.to_s.should eq "source.cr:5:27" - issue.message.should eq "Useless condition in when detected" - end end end diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index d99a6aa8..c02cb7e5 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/spec/ameba/rule/performance/any_after_filter_spec.cr @@ -66,14 +66,5 @@ module Ameba::Rule::Performance expect_no_corrections source end end - - it "reports rule, pos and message" do - source = expect_issue subject, <<-CRYSTAL - [1, 2, 3].reject { |e| e > 2 }.any? - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `reject {...}.any?` - CRYSTAL - - expect_no_corrections source - end end end diff --git a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr index 2b637a8f..1812c5dc 100644 --- a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr +++ b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr @@ -42,18 +42,5 @@ module Ameba::Rule::Performance CRYSTAL end end - - it "reports rule, pos and message" do - source = Source.new path: "source.cr", code: %( - [1, 2, 3].any? - ) - subject.catch(source).should_not be_valid - issue = source.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:14" - issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`" - end end end diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr index 123e36bf..eaef1a8f 100644 --- a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr +++ b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr @@ -54,22 +54,6 @@ module Ameba::Rule::Performance end end - it "reports rule, pos and message" do - source = Source.new path: "source.cr", code: <<-CODE - [1, 2, 3].select { |e| e > 1 }.reverse - CODE - - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:32" - issue.end_location.to_s.should eq "source.cr:1:38" - - issue.message.should eq "Use bang method variant `reverse!` after chained `select` call" - end - context "macro" do it "doesn't report in macro scope" do expect_no_issues subject, <<-CRYSTAL diff --git a/spec/ameba/rule/performance/compact_after_map_spec.cr b/spec/ameba/rule/performance/compact_after_map_spec.cr index 9c7a72e3..544e2905 100644 --- a/spec/ameba/rule/performance/compact_after_map_spec.cr +++ b/spec/ameba/rule/performance/compact_after_map_spec.cr @@ -36,18 +36,5 @@ module Ameba::Rule::Performance CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - (1..3).map(&.itself).compact - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:8" - issue.end_location.to_s.should eq "source.cr:1:29" - issue.message.should eq "Use `compact_map {...}` instead of `map {...}.compact`" - end end end diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index 4352c200..d3e290ee 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr @@ -72,21 +72,6 @@ module Ameba::Rule::Performance end end - it "reports rule, pos and message" do - s = Source.new %( - [1, 2, 3].select { |e| e > 2 }.first - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:37" - - issue.message.should eq "Use `find {...}` instead of `select {...}.first`" - end - context "macro" do it "doesn't report in macro scope" do expect_no_issues subject, <<-CRYSTAL diff --git a/spec/ameba/rule/performance/flatten_after_map_spec.cr b/spec/ameba/rule/performance/flatten_after_map_spec.cr index 54796380..a7f89193 100644 --- a/spec/ameba/rule/performance/flatten_after_map_spec.cr +++ b/spec/ameba/rule/performance/flatten_after_map_spec.cr @@ -30,18 +30,5 @@ module Ameba::Rule::Performance CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - %w[Alice Bob].map(&.chars).flatten - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:15" - issue.end_location.to_s.should eq "source.cr:1:35" - issue.message.should eq "Use `flat_map {...}` instead of `map {...}.flatten`" - end end end diff --git a/spec/ameba/rule/performance/map_instead_of_block_spec.cr b/spec/ameba/rule/performance/map_instead_of_block_spec.cr index 95cfd901..b154ce48 100644 --- a/spec/ameba/rule/performance/map_instead_of_block_spec.cr +++ b/spec/ameba/rule/performance/map_instead_of_block_spec.cr @@ -45,18 +45,5 @@ module Ameba::Rule::Performance CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - (1..3).map(&.to_u64).sum - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:8" - issue.end_location.to_s.should eq "source.cr:1:25" - issue.message.should eq "Use `sum {...}` instead of `map {...}.sum`" - end end end diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index fecedc5e..f00c7c8d 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/spec/ameba/rule/performance/size_after_filter_spec.cr @@ -61,18 +61,5 @@ module Ameba::Rule::Performance CRYSTAL end end - - it "reports rule, pos and message" do - s = Source.new %( - lines.split("\n").reject(&.empty?).size - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:4" - issue.end_location.to_s.should eq "source.cr:2:25" - issue.message.should eq "Use `count {...}` instead of `reject {...}.size`." - end end end diff --git a/spec/ameba/rule/style/constant_names_spec.cr b/spec/ameba/rule/style/constant_names_spec.cr index 00267289..23048382 100644 --- a/spec/ameba/rule/style/constant_names_spec.cr +++ b/spec/ameba/rule/style/constant_names_spec.cr @@ -37,19 +37,5 @@ module Ameba # it_reports_constant "MyBadConstant", "1", "MYBADCONSTANT" it_reports_constant "Wrong_NAME", "2", "WRONG_NAME" it_reports_constant "Wrong_Name", "3", "WRONG_NAME" - - it "reports rule, pos and message" do - s = Source.new %( - Const_Name = 1 - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:10" - issue.message.should eq( - "Constant name should be screaming-cased: CONST_NAME, not Const_Name" - ) - end end end diff --git a/spec/ameba/rule/style/is_a_filter_spec.cr b/spec/ameba/rule/style/is_a_filter_spec.cr index ed01d213..234c2600 100644 --- a/spec/ameba/rule/style/is_a_filter_spec.cr +++ b/spec/ameba/rule/style/is_a_filter_spec.cr @@ -59,20 +59,5 @@ module Ameba::Rule::Style CRYSTAL end end - - it "reports rule, pos and message" do - source = Source.new path: "source.cr", code: %( - [1, 2, nil].reject(&.nil?) - ) - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:13" - issue.end_location.to_s.should eq "source.cr:1:26" - - issue.message.should eq "Use `reject(Nil)` instead of `reject {...}`" - end end end diff --git a/spec/ameba/rule/style/is_a_nil_spec.cr b/spec/ameba/rule/style/is_a_nil_spec.cr index 44454cc9..8c2c7d42 100644 --- a/spec/ameba/rule/style/is_a_nil_spec.cr +++ b/spec/ameba/rule/style/is_a_nil_spec.cr @@ -34,19 +34,5 @@ module Ameba::Rule::Style a.nil? CRYSTAL end - - it "reports rule, location and message" do - s = Source.new %( - nil.is_a? Nil - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:13" - issue.message.should eq IsANil::MSG - end end end diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index 4682aae2..725ec158 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -117,18 +117,6 @@ module Ameba it_transforms "3.001234", "3.001_234" it_transforms "3.0012345", "3.001_234_5" - it "reports rule, pos and message" do - s = Source.new %q( - 1200000 - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:7" - issue.message.should match /1_200_000/ - end - context "properties" do it "#int_min_digits" do rule = Rule::Style::LargeNumbers.new diff --git a/spec/ameba/rule/style/method_names_spec.cr b/spec/ameba/rule/style/method_names_spec.cr index 94d2608d..c9729dc4 100644 --- a/spec/ameba/rule/style/method_names_spec.cr +++ b/spec/ameba/rule/style/method_names_spec.cr @@ -38,20 +38,5 @@ module Ameba it_reports_method_name "firstName", "first_name" it_reports_method_name "date_of_Birth", "date_of_birth" it_reports_method_name "homepageURL", "homepage_url" - - it "reports rule, pos and message" do - s = Source.new %( - def bad_Name(a) - end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:5" - issue.end_location.to_s.should eq "source.cr:1:12" - issue.message.should eq( - "Method name should be underscore-cased: bad_name, not bad_Name" - ) - end end end diff --git a/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr b/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr index 19251c54..60edb7cf 100644 --- a/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr +++ b/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr @@ -53,16 +53,5 @@ module Ameba::Rule::Style end CRYSTAL end - - it "reports rule, pos and message" do - s = Source.new ":nok unless !s.empty?", "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:21" - issue.message.should eq "Avoid negated conditions in unless blocks" - end end end diff --git a/spec/ameba/rule/style/predicate_name_spec.cr b/spec/ameba/rule/style/predicate_name_spec.cr index 29acd0aa..46e37fb6 100644 --- a/spec/ameba/rule/style/predicate_name_spec.cr +++ b/spec/ameba/rule/style/predicate_name_spec.cr @@ -27,24 +27,6 @@ module Ameba::Rule::Style CRYSTAL end - it "reports rule, pos and message" do - s = Source.new %q( - class Image - def is_valid?(x) - true - end - end - ), "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:3" - issue.end_location.to_s.should eq "source.cr:4:5" - issue.message.should eq( - "Favour method name 'valid?' over 'is_valid?'") - end - it "ignores if alternative name isn't valid syntax" do expect_no_issues subject, <<-CRYSTAL class Image diff --git a/spec/ameba/rule/style/redundant_begin_spec.cr b/spec/ameba/rule/style/redundant_begin_spec.cr index b9bb0617..acfa2adc 100644 --- a/spec/ameba/rule/style/redundant_begin_spec.cr +++ b/spec/ameba/rule/style/redundant_begin_spec.cr @@ -294,24 +294,5 @@ module Ameba::Rule::Style } CRYSTAL end - - it "reports rule, pos and message" do - s = Source.new %q( - def method - begin - open_connection - ensure - close_connection - end - end - ), "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:2:3" - issue.end_location.to_s.should eq "source.cr:2:7" - issue.message.should eq "Redundant `begin` block detected" - end end end diff --git a/spec/ameba/rule/style/type_names_spec.cr b/spec/ameba/rule/style/type_names_spec.cr index 31abc89b..31a84d39 100644 --- a/spec/ameba/rule/style/type_names_spec.cr +++ b/spec/ameba/rule/style/type_names_spec.cr @@ -49,20 +49,5 @@ module Ameba # ^{} error: Type name should be camelcased: NumericValue, but it was Numeric_value CRYSTAL end - - it "reports rule, pos and message" do - s = Source.new %( - class My_class - end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:2:3" - issue.message.should eq( - "Type name should be camelcased: MyClass, but it was My_class" - ) - end end end diff --git a/spec/ameba/rule/style/unless_else_spec.cr b/spec/ameba/rule/style/unless_else_spec.cr index cf1c065d..7d9bdb7c 100644 --- a/spec/ameba/rule/style/unless_else_spec.cr +++ b/spec/ameba/rule/style/unless_else_spec.cr @@ -22,23 +22,5 @@ module Ameba::Rule::Style end CRYSTAL end - - it "reports rule, pos and message" do - s = Source.new %( - unless something - :one - else - :two - end - ), "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.should_not be_nil - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:5:3" - issue.message.should eq "Favour if over unless with else" - end end end diff --git a/spec/ameba/rule/style/variable_names_spec.cr b/spec/ameba/rule/style/variable_names_spec.cr index 3443ddcb..7add818e 100644 --- a/spec/ameba/rule/style/variable_names_spec.cr +++ b/spec/ameba/rule/style/variable_names_spec.cr @@ -62,19 +62,5 @@ module Ameba end CRYSTAL end - - it "reports rule, pos and message" do - s = Source.new %( - badName = "Yeah" - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "source.cr:1:7" - issue.message.should eq( - "Var name should be underscore-cased: bad_name, not badName" - ) - end end end diff --git a/spec/ameba/rule/style/verbose_block_spec.cr b/spec/ameba/rule/style/verbose_block_spec.cr index b638a65c..d7851ba0 100644 --- a/spec/ameba/rule/style/verbose_block_spec.cr +++ b/spec/ameba/rule/style/verbose_block_spec.cr @@ -273,20 +273,5 @@ module Ameba::Rule::Style (1..3).join(separator: '.', &.to_s) CRYSTAL end - - it "reports rule, pos and message" do - source = Source.new path: "source.cr", code: <<-CRYSTAL - (1..3).any? { |i| i.odd? } - CRYSTAL - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:8" - issue.end_location.to_s.should eq "source.cr:1:26" - - issue.message.should eq "Use short block notation instead: `any?(&.odd?)`" - end end end From cfea0fda34b1b3f5a32cb67de5a140b176c2ecfe Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 21:14:29 +0100 Subject: [PATCH 14/32] Fix typo --- spec/ameba/rule/layout/trailing_whitespace_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ameba/rule/layout/trailing_whitespace_spec.cr b/spec/ameba/rule/layout/trailing_whitespace_spec.cr index 9885e14f..df376870 100644 --- a/spec/ameba/rule/layout/trailing_whitespace_spec.cr +++ b/spec/ameba/rule/layout/trailing_whitespace_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Layout describe TrailingWhitespace do it "passes if all lines do not have trailing whitespace" do - expect_no_issues subject, "no-whispace" + expect_no_issues subject, "no-whitespace" end it "fails if there is a line with trailing whitespace" do From 232c16d7437791fa2e25c49edef47931f964592b Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 21:15:17 +0100 Subject: [PATCH 15/32] Add spec for `10000` `Int128` literal in `Style/LargeNumbers` --- spec/ameba/rule/style/large_numbers_spec.cr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index 725ec158..37cdf231 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -97,10 +97,12 @@ module Ameba it_transforms "10000_i16", "10_000_i16" it_transforms "10000_i32", "10_000_i32" it_transforms "10000_i64", "10_000_i64" + it_transforms "10000_i128", "10_000_i128" it_transforms "10000_u16", "10_000_u16" it_transforms "10000_u32", "10_000_u32" it_transforms "10000_u64", "10_000_u64" + it_transforms "10000_u128", "10_000_u128" it_transforms "123456_f32", "123_456_f32" it_transforms "123456_f64", "123_456_f64" From ea4439d6e271cbec98d3fe48f4f7a0c6ea7b8261 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Thu, 22 Dec 2022 13:45:27 +0100 Subject: [PATCH 16/32] Fix regression in docs CI --- .github/workflows/docs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 8848dba4..f889f916 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -44,7 +44,7 @@ jobs: path: docs - name: Deploy docs 🚀 - uses: JamesIves/github-pages-deploy-action@4 + uses: JamesIves/github-pages-deploy-action@v4 with: branch: gh-pages folder: docs From f26cd7f82380504664b064e5868dffc85f48f845 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 19 Dec 2022 15:40:30 +0100 Subject: [PATCH 17/32] Extend `Lint/UnusedArgument` rule with corrections --- spec/ameba/rule/lint/unused_argument_spec.cr | 226 +++++++++---------- src/ameba/rule/lint/unused_argument.cr | 13 +- 2 files changed, 125 insertions(+), 114 deletions(-) diff --git a/spec/ameba/rule/lint/unused_argument_spec.cr b/spec/ameba/rule/lint/unused_argument_spec.cr index 47be8ce9..427ae569 100644 --- a/spec/ameba/rule/lint/unused_argument_spec.cr +++ b/spec/ameba/rule/lint/unused_argument_spec.cr @@ -6,7 +6,7 @@ module Ameba::Rule::Lint describe UnusedArgument do it "doesn't report if arguments are used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b, c) a + b + c end @@ -16,155 +16,158 @@ module Ameba::Rule::Lint end ->(i : Int32) { i + 1 } - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if method argument is unused" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def method(a, b, c) + # ^ error: Unused argument `c`. If it's necessary, use `_c` as an argument name to indicate that it won't be used. a + b end - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ - "as an argument name to indicate that it won't be used." + CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a, b, _c) + a + b + end + CRYSTAL end it "reports if block argument is unused" do - s = Source.new %( - [1,2].each_with_index do |a, i| + source = expect_issue subject, <<-CRYSTAL + [1, 2].each_with_index do |a, i| + # ^ error: Unused argument `i`. [...] a end - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq "Unused argument `i`. If it's necessary, use `_` " \ - "as an argument name to indicate that it won't be used." + CRYSTAL + + expect_correction source, <<-CRYSTAL + [1, 2].each_with_index do |a, _| + a + end + CRYSTAL end it "reports if proc argument is unused" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL -> (a : Int32, b : String) do + # ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used. a = a + 1 end - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ - "as an argument name to indicate that it won't be used." + CRYSTAL + + expect_correction source, <<-CRYSTAL + -> (a : Int32, _b : String) do + a = a + 1 + end + CRYSTAL end it "reports multiple unused args" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def method(a, b, c) + # ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used. + # ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used. + # ^ error: Unused argument `c`. If it's necessary, use `_c` as an argument name to indicate that it won't be used. nil end - ) - subject.catch(s).should_not be_valid - s.issues[0].message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ - "as an argument name to indicate that it won't be used." - s.issues[1].message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ - "as an argument name to indicate that it won't be used." - s.issues[2].message.should eq "Unused argument `c`. If it's necessary, use `_c` " \ - "as an argument name to indicate that it won't be used." + CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(_a, _b, _c) + nil + end + CRYSTAL end it "doesn't report if it is an instance var argument" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class A def method(@name) end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if a typed argument is used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(x : Int32) 3.times do puts x end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if an argument with default value is used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(x = 1) puts x end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if argument starts with a _" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(_x) end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if it is a block and used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(&block) block.call end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if block arg is not used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(&block) end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if unused and there is yield" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(&block) yield 1 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if it's an anonymous block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(&) yield 1 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if variable is referenced implicitly" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Bar < Foo def method(a, b) super end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if arg if referenced in case" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def foo(a) case a when /foo/ end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if enum in a record" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Class record Record do enum Enum @@ -172,59 +175,49 @@ module Ameba::Rule::Lint end end end - ) - subject.catch(s).should be_valid + CRYSTAL end context "super" do it "reports if variable is not referenced implicitly by super" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL class Bar < Foo def method(a, b) + # ^ error: Unused argument `b`. If it's necessary, use `_b` as an argument name to indicate that it won't be used. super a end end - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq "Unused argument `b`. If it's necessary, use `_b` " \ - "as an argument name to indicate that it won't be used." - end + CRYSTAL - it "reports rule, location and message" do - s = Source.new %( - def method(a) + expect_correction source, <<-CRYSTAL + class Bar < Foo + def method(a, _b) + super a + end end - ), "source.cr" - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.rule.should_not be_nil - issue.message.should eq "Unused argument `a`. If it's necessary, use `_a` " \ - "as an argument name to indicate that it won't be used." - issue.location.to_s.should eq "source.cr:1:12" + CRYSTAL end end context "macro" do it "doesn't report if it is a used macro argument" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL macro my_macro(arg) {% arg %} end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if it is a used macro block argument" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL macro my_macro(&block) {% block %} end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report used macro args with equal names in record" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL record X do macro foo(a, b) {{ a }} + {{ b }} @@ -234,12 +227,11 @@ module Ameba::Rule::Lint {{ a }} + {{ b }} + {{ c }} end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report used args in macro literals" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def print(f : Array(U)) forall U f.size.times do |i| {% if U == Float64 %} @@ -249,65 +241,73 @@ module Ameba::Rule::Lint {% end %} end end - ) - subject.catch(s).should be_valid + CRYSTAL end end context "properties" do describe "#ignore_defs" do it "lets the rule to ignore def scopes if true" do - subject.ignore_defs = true - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_defs = true + + expect_no_issues rule, <<-CRYSTAL def method(a) end - ) - subject.catch(s).should be_valid + CRYSTAL end it "lets the rule not to ignore def scopes if false" do - subject.ignore_defs = false - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_defs = false + + expect_issue rule, <<-CRYSTAL def method(a) + # ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used. end - ) - subject.catch(s).should_not be_valid + CRYSTAL end end context "#ignore_blocks" do it "lets the rule to ignore block scopes if true" do - subject.ignore_blocks = true - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_blocks = true + + expect_no_issues rule, <<-CRYSTAL 3.times { |i| puts "yo!" } - ) - subject.catch(s).should be_valid + CRYSTAL end it "lets the rule not to ignore block scopes if false" do - subject.ignore_blocks = false - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_blocks = false + + expect_issue rule, <<-CRYSTAL 3.times { |i| puts "yo!" } - ) - subject.catch(s).should_not be_valid + # ^ error: Unused argument `i`. If it's necessary, use `_` as an argument name to indicate that it won't be used. + CRYSTAL end end context "#ignore_procs" do it "lets the rule to ignore proc scopes if true" do - subject.ignore_procs = true - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_procs = true + + expect_no_issues rule, <<-CRYSTAL ->(a : Int32) {} - ) - subject.catch(s).should be_valid + CRYSTAL end it "lets the rule not to ignore proc scopes if false" do - subject.ignore_procs = false - s = Source.new %( + rule = UnusedArgument.new + rule.ignore_procs = false + + expect_issue rule, <<-CRYSTAL ->(a : Int32) {} - ) - subject.catch(s).should_not be_valid + # ^ error: Unused argument `a`. If it's necessary, use `_a` as an argument name to indicate that it won't be used. + CRYSTAL end end end diff --git a/src/ameba/rule/lint/unused_argument.cr b/src/ameba/rule/lint/unused_argument.cr index c42668a5..864b982f 100644 --- a/src/ameba/rule/lint/unused_argument.cr +++ b/src/ameba/rule/lint/unused_argument.cr @@ -63,7 +63,18 @@ module Ameba::Rule::Lint next if scope.references?(argument.variable) name_suggestion = scope.node.is_a?(Crystal::Block) ? '_' : "_#{argument.name}" - issue_for argument.node, MSG % {argument.name, name_suggestion} + message = MSG % {argument.name, name_suggestion} + + location = argument.node.location + end_location = location.try &.adjust(column_number: argument.name.size - 1) + + if location && end_location + issue_for argument.node, message do |corrector| + corrector.replace(location, end_location, name_suggestion) + end + else + issue_for argument.node, message + end end end end From 3586b4242fa0c8102db4d79fc2d8ac896d660b99 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 19 Dec 2022 15:40:59 +0100 Subject: [PATCH 18/32] Extend `Lint/UnusedBlockArgument` rule with corrections --- .../rule/lint/unused_block_argument_spec.cr | 79 +++++++++++-------- src/ameba/rule/lint/unused_block_argument.cr | 21 ++++- 2 files changed, 65 insertions(+), 35 deletions(-) diff --git a/spec/ameba/rule/lint/unused_block_argument_spec.cr b/spec/ameba/rule/lint/unused_block_argument_spec.cr index adcdc6a1..dbc95b8c 100644 --- a/spec/ameba/rule/lint/unused_block_argument_spec.cr +++ b/spec/ameba/rule/lint/unused_block_argument_spec.cr @@ -5,105 +5,118 @@ module Ameba::Rule::Lint describe UnusedBlockArgument do it "doesn't report if it is an instance var argument" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class A def initialize(&@callback) end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if anonymous" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b, c, &) end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if argument name starts with a `_`" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b, c, &_block) end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if it is a block and used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b, c, &block) block.call end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if block arg is not used" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def method(a, b, c, &block) + # ^ error: Unused block argument `block`. [...] end - ) - subject.catch(s).should_not be_valid + CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a, b, c, &_block) + end + CRYSTAL end it "reports if unused and there is yield" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def method(a, b, c, &block) + # ^ error: Use `&` as an argument name to indicate that it won't be referenced. 3.times do |i| i.try do yield i end end end - ) - subject.catch(s).should_not be_valid + CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a, b, c, &) + 3.times do |i| + i.try do + yield i + end + end + end + CRYSTAL end it "doesn't report if anonymous and there is yield" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b, c, &) yield 1 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if variable is referenced implicitly" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Bar < Foo def method(a, b, c, &block) super end end - ) - subject.catch(s).should be_valid + CRYSTAL end context "super" do it "reports if variable is not referenced implicitly by super" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL class Bar < Foo def method(a, b, c, &block) + # ^ error: Unused block argument `block`. [...] super a, b, c end end - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq "Unused block argument `block`. If it's necessary, " \ - "use `_block` as an argument name to indicate " \ - "that it won't be used." + CRYSTAL + + expect_correction source, <<-CRYSTAL + class Bar < Foo + def method(a, b, c, &_block) + super a, b, c + end + end + CRYSTAL end end context "macro" do it "doesn't report if it is a used macro block argument" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL macro my_macro(&block) {% block %} end - ) - subject.catch(s).should be_valid + CRYSTAL end end end diff --git a/src/ameba/rule/lint/unused_block_argument.cr b/src/ameba/rule/lint/unused_block_argument.cr index 077b9b23..0af82811 100644 --- a/src/ameba/rule/lint/unused_block_argument.cr +++ b/src/ameba/rule/lint/unused_block_argument.cr @@ -31,6 +31,8 @@ module Ameba::Rule::Lint # Enabled: true # ``` class UnusedBlockArgument < Base + include AST::Util + properties do description "Disallows unused block arguments" end @@ -51,11 +53,26 @@ module Ameba::Rule::Lint return if block_arg.anonymous? return if scope.references?(block_arg.variable) + location = block_arg.node.location + end_location = location.try &.adjust(column_number: block_arg.name.size - 1) + if scope.yields? - issue_for block_arg.node, MSG_YIELDED + if location && end_location + issue_for block_arg.node, MSG_YIELDED do |corrector| + corrector.remove(location, end_location) + end + else + issue_for block_arg.node, MSG_YIELDED + end else return if block_arg.ignored? - issue_for block_arg.node, MSG_UNUSED % block_arg.name + if location && end_location + issue_for block_arg.node, MSG_UNUSED % block_arg.name do |corrector| + corrector.insert_before(location, '_') + end + else + issue_for block_arg.node, MSG_UNUSED % block_arg.name + end end end end From 2cedd72b0961feaf83eecbb29caacbb5dc437cb2 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Thu, 22 Dec 2022 19:18:27 +0100 Subject: [PATCH 19/32] Apply review sugggestions --- spec/ameba/source_spec.cr | 51 ++++++++++++++++------------- src/ameba/rule/style/unless_else.cr | 14 +++++--- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index e50b499b..8c8b1a1b 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -4,63 +4,70 @@ module Ameba describe Source do describe ".new" do it "allows to create a source by code and path" do - s = Source.new("code", "path") - s.path.should eq "path" - s.code.should eq "code" - s.lines.should eq ["code"] + source = Source.new "code", "path" + source.path.should eq "path" + source.code.should eq "code" + source.lines.should eq ["code"] end end describe "#fullpath" do it "returns a relative path of the source" do - s = Source.new "", "./source_spec.cr" - s.fullpath.should contain "source_spec.cr" + source = Source.new "", "./source_spec.cr" + source.fullpath.should contain "source_spec.cr" end it "returns fullpath if path is blank" do - s = Source.new "", "" - s.fullpath.should_not be_nil + 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 - s = Source.new "", "./source_spec.cr" - s.spec?.should be_true + source = Source.new "", "./source_spec.cr" + source.spec?.should be_true end it "returns false if the source is not a spec file" do - s = Source.new "", "./source.cr" - s.spec?.should be_false + source = Source.new "", "./source.cr" + source.spec?.should be_false end end describe "#matches_path?" do it "returns true if source's path is matched" do - s = Source.new "", "source.cr" - s.matches_path?("source.cr").should be_true + source = Source.new "", "source.cr" + source.matches_path?("source.cr").should be_true end it "returns false if source's path is not matched" do - s = Source.new "", "source.cr" - s.matches_path?("new_source.cr").should be_false + source = Source.new "", "source.cr" + source.matches_path?("new_source.cr").should be_false end end describe "#pos" do it "works" do - s = Source.new <<-EOS + source = Source.new <<-CRYSTAL foo bar fizz buzz - EOS - loc = Crystal::Location.new("", 2, 1) - end_loc = Crystal::Location.new("", 3, 4) - s.code[s.pos(loc)...s.pos(end_loc, end: true)].should eq <<-EOS + CRYSTAL + + location = Crystal::Location.new("", 2, 1) + end_location = Crystal::Location.new("", 3, 4) + + range = Range.new( + source.pos(location), + source.pos(end_location, end: true), + exclusive: true + ) + source.code[range].should eq <<-CRYSTAL bar fizz - EOS + CRYSTAL end end end diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index 20fa0e08..42f9cfb9 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -51,10 +51,16 @@ module Ameba::Rule::Style def test(source, node : Crystal::Unless) return if node.else.nop? - return unless location = node.location - return unless cond_end_location = node.cond.end_location - return unless else_location = node.else_location - return unless end_location = node.end_location + + location = node.location + cond_end_location = node.cond.end_location + else_location = node.else_location + end_location = node.end_location + + unless location && cond_end_location && else_location && end_location + issue_for node, MSG + return + end issue_for node, MSG do |corrector| keyword_begin_pos = source.pos(location) From bbbfdfc5a2e7b9234c66487a59464915d45d2b66 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 23 Dec 2022 14:46:23 +0100 Subject: [PATCH 20/32] Tweak reported location for `Lint/UnusedBlockArgument` --- spec/ameba/rule/lint/unused_block_argument_spec.cr | 6 +++--- src/ameba/rule/lint/unused_block_argument.cr | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/ameba/rule/lint/unused_block_argument_spec.cr b/spec/ameba/rule/lint/unused_block_argument_spec.cr index dbc95b8c..d5ad799c 100644 --- a/spec/ameba/rule/lint/unused_block_argument_spec.cr +++ b/spec/ameba/rule/lint/unused_block_argument_spec.cr @@ -38,7 +38,7 @@ module Ameba::Rule::Lint it "reports if block arg is not used" do source = expect_issue subject, <<-CRYSTAL def method(a, b, c, &block) - # ^ error: Unused block argument `block`. [...] + # ^^^^^ error: Unused block argument `block`. [...] end CRYSTAL @@ -51,7 +51,7 @@ module Ameba::Rule::Lint it "reports if unused and there is yield" do source = expect_issue subject, <<-CRYSTAL def method(a, b, c, &block) - # ^ error: Use `&` as an argument name to indicate that it won't be referenced. + # ^^^^^ error: Use `&` as an argument name to indicate that it won't be referenced. 3.times do |i| i.try do yield i @@ -94,7 +94,7 @@ module Ameba::Rule::Lint source = expect_issue subject, <<-CRYSTAL class Bar < Foo def method(a, b, c, &block) - # ^ error: Unused block argument `block`. [...] + # ^^^^^ error: Unused block argument `block`. [...] super a, b, c end end diff --git a/src/ameba/rule/lint/unused_block_argument.cr b/src/ameba/rule/lint/unused_block_argument.cr index 0af82811..7fdb0f67 100644 --- a/src/ameba/rule/lint/unused_block_argument.cr +++ b/src/ameba/rule/lint/unused_block_argument.cr @@ -58,7 +58,7 @@ module Ameba::Rule::Lint if scope.yields? if location && end_location - issue_for block_arg.node, MSG_YIELDED do |corrector| + issue_for location, end_location, MSG_YIELDED do |corrector| corrector.remove(location, end_location) end else @@ -67,7 +67,7 @@ module Ameba::Rule::Lint else return if block_arg.ignored? if location && end_location - issue_for block_arg.node, MSG_UNUSED % block_arg.name do |corrector| + issue_for location, end_location, MSG_UNUSED % block_arg.name do |corrector| corrector.insert_before(location, '_') end else From caaf803ecdd3f22314a99e42c2eda4e50dbb5241 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Fri, 23 Dec 2022 14:46:45 +0100 Subject: [PATCH 21/32] Tweak reported location for `Style/UnlessElse` --- src/ameba/rule/style/unless_else.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ameba/rule/style/unless_else.cr b/src/ameba/rule/style/unless_else.cr index 42f9cfb9..ba9c380b 100644 --- a/src/ameba/rule/style/unless_else.cr +++ b/src/ameba/rule/style/unless_else.cr @@ -62,7 +62,7 @@ module Ameba::Rule::Style return end - issue_for node, MSG do |corrector| + issue_for location, cond_end_location, MSG do |corrector| keyword_begin_pos = source.pos(location) keyword_end_pos = keyword_begin_pos + {{ "unless".size }} keyword_range = keyword_begin_pos...keyword_end_pos From 34620a986de28874e9b2d341b513f17e42874652 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 2 Jan 2023 17:35:24 +0100 Subject: [PATCH 22/32] Add GitHub Actions workflow to build and push Docker images to GHCR --- .github/workflows/cd.yml | 74 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 .github/workflows/cd.yml diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml new file mode 100644 index 00000000..a4597deb --- /dev/null +++ b/.github/workflows/cd.yml @@ -0,0 +1,74 @@ +name: Docker image build and deploy + +on: + push: + branches: [master] + release: + types: [published] + +env: + # Use docker.io for Docker Hub if empty + REGISTRY: ghcr.io + # github.repository as / + IMAGE_NAME: ${{ github.repository }} + +jobs: + build: + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + # This is used to complete the identity challenge + # with sigstore/fulcio when running outside of PRs. + id-token: write + + steps: + - name: Checkout repository + uses: actions/checkout@v3 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v2 + + - name: Setup Docker Buildx + uses: docker/setup-buildx-action@v2 + + # Login against a Docker registry except on PR + # https://github.com/docker/login-action + - name: Log into ${{ env.REGISTRY }} registry + uses: docker/login-action@v2 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # Extract metadata (tags, labels) for Docker + # https://github.com/docker/metadata-action + - name: Extract Docker metadata + id: meta + uses: docker/metadata-action@v4 + with: + images: | + ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + tags: | + type=ref,event=tag + type=ref,event=branch + type=ref,event=pr + type=sha,format=long + type=semver,pattern={{version}} + type=semver,pattern={{major}}.{{minor}} + + # Build and push Docker image with Buildx + # https://github.com/docker/build-push-action + - name: Build and push Docker image + id: build-and-push + uses: docker/build-push-action@v3 + with: + context: . + push: true + cache-from: type=gha + cache-to: type=gha,mode=max + platforms: | + linux/amd64 + linux/arm64 + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} From 1a60cc3c18b5996f27f27107d6a2805d28eae2a9 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 2 Jan 2023 18:06:23 +0100 Subject: [PATCH 23/32] Update `README.md` in re: to the GHCR --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b068799e..48c48766 100644 --- a/README.md +++ b/README.md @@ -162,16 +162,16 @@ $ brew install ameba Build the image: ```sh -$ docker build -t crystal-ameba/ameba . +$ docker build -t ghcr.io/crystal-ameba/ameba . ``` To use the resulting image on a local source folder, mount the current (or target) directory into `/src`: ```sh -$ docker run -v $(pwd):/src crystal-ameba/ameba +$ docker run -v $(pwd):/src ghcr.io/crystal-ameba/ameba ``` -Also available on DockerHub: https://hub.docker.com/r/veelenga/ameba +Also available on GitHub: https://github.com/crystal-ameba/ameba/pkgs/container/ameba ### From sources From 01ab89b348a22473f421c962885c97fb4b7b5dad Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 2 Jan 2023 18:41:46 +0100 Subject: [PATCH 24/32] Tweak CI settings --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd3f3546..f5359c21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,9 @@ name: CI on: push: + branches: [master] pull_request: + types: [opened, synchronize, reopened] schedule: - cron: "0 3 * * 1" # Every monday at 3 AM From e9226c05d5133bfeb380c5de08d521142f7ac566 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 2 Jan 2023 19:06:33 +0100 Subject: [PATCH 25/32] Cleanup `Makefile` --- Makefile | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 84144a86..8b7772a4 100644 --- a/Makefile +++ b/Makefile @@ -4,19 +4,35 @@ PREFIX ?= /usr/local SHARD_BIN ?= ../../bin CRFLAGS ?= -Dpreview_mt -build: bin/ameba -bin/ameba: +.PHONY: build +build: $(SHARDS_BIN) build $(CRFLAGS) + +.PHONY: lint +lint: build + ./bin/ameba --all + +.PHONY: spec +spec: + $(CRYSTAL_BIN) spec + +.PHONY: clean clean: rm -f ./bin/ameba ./bin/ameba.dwarf + +.PHONY: install install: build mkdir -p $(PREFIX)/bin cp ./bin/ameba $(PREFIX)/bin + +.PHONY: bin bin: build mkdir -p $(SHARD_BIN) cp ./bin/ameba $(SHARD_BIN) + +.PHONY: run_file run_file: cp -n ./bin/ameba.cr $(SHARD_BIN) || true -test: build - $(CRYSTAL_BIN) spec - ./bin/ameba --all + +.PHONY: test +test: spec lint From 1ebc1a68c6500d451f3eab771ed157c43f2926b9 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Mon, 2 Jan 2023 19:06:56 +0100 Subject: [PATCH 26/32] Refactor CI workflow to clearly show failed steps --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5359c21..86049975 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,7 +30,10 @@ jobs: run: shards install - name: Run specs - run: make test + run: crystal spec - - name: Check formatting - run: crystal tool format --check + - name: Build ameba binary + run: shards build -Dpreview_mt + + - name: Run ameba linter + run: bin/ameba --all From 853f84d954347c17eed733529ed140045eee612f Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 3 Jan 2023 16:36:33 +0100 Subject: [PATCH 27/32] Fix docs CI workflow --- .github/workflows/docs.yml | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f889f916..ec938262 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -4,8 +4,12 @@ on: push: branches: [master] +permissions: + contents: write + jobs: - build: + build-and-deploy: + concurrency: ci-${{ github.ref }} runs-on: ubuntu-latest steps: - name: Inject slug/short variables @@ -16,8 +20,6 @@ jobs: - name: Download source uses: actions/checkout@v3 - with: - persist-credentials: false - name: Install dependencies run: shards install @@ -27,22 +29,6 @@ jobs: sed -i -e 's:<.*>::g' README.md crystal docs --project-version="${{ env.GITHUB_REF_SLUG }}" --source-refname="${{ env.GITHUB_SHA_SHORT }}" - - name: Upload artifacts - uses: actions/upload-artifact@v3 - with: - name: docs - path: docs - - deploy: - needs: build - runs-on: ubuntu-latest - steps: - - name: Download artifacts - uses: actions/download-artifact@v3 - with: - name: docs - path: docs - - name: Deploy docs 🚀 uses: JamesIves/github-pages-deploy-action@v4 with: From 1514fc53ca737ea76b7123a43953d0ab8cd373c4 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 3 Jan 2023 17:04:37 +0100 Subject: [PATCH 28/32] =?UTF-8?q?There=E2=80=99s=20no=20need=20to=20strip?= =?UTF-8?q?=20the=20HTML=20from=20`.md`=20docs=20anymoar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/docs.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index ec938262..2bf0f59a 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -25,9 +25,7 @@ jobs: run: shards install - name: Build docs - run: | - sed -i -e 's:<.*>::g' README.md - crystal docs --project-version="${{ env.GITHUB_REF_SLUG }}" --source-refname="${{ env.GITHUB_SHA_SHORT }}" + run: crystal docs --project-version="${{ env.GITHUB_REF_SLUG }}" --source-refname="${{ env.GITHUB_SHA_SHORT }}" - name: Deploy docs 🚀 uses: JamesIves/github-pages-deploy-action@v4 From 6f0b6ffcd0776efcf397f6162a850f897bcdd92c Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 4 Jan 2023 02:40:07 +0100 Subject: [PATCH 29/32] Remove buggy auto-correction from `Performance/AnyInsteadOfEmpty` rule --- .../rule/performance/any_instead_of_empty_spec.cr | 12 ++---------- src/ameba/rule/performance/any_instead_of_empty.cr | 6 +----- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr index 1812c5dc..7d4827b9 100644 --- a/spec/ameba/rule/performance/any_instead_of_empty_spec.cr +++ b/spec/ameba/rule/performance/any_instead_of_empty_spec.cr @@ -14,14 +14,10 @@ module Ameba::Rule::Performance end it "reports if there is any? call without a block nor argument" do - source = expect_issue subject, <<-CRYSTAL + expect_issue subject, <<-CRYSTAL [1, 2, 3].any? # ^^^^ error: Use `!{...}.empty?` instead of `{...}.any?` CRYSTAL - - expect_correction source, <<-CRYSTAL - ![1, 2, 3].empty? - CRYSTAL end it "does not report if source is a spec" do @@ -32,14 +28,10 @@ module Ameba::Rule::Performance context "macro" do it "reports in macro scope" do - source = expect_issue subject, <<-CRYSTAL + expect_issue subject, <<-CRYSTAL {{ [1, 2, 3].any? }} # ^^^^ error: Use `!{...}.empty?` instead of `{...}.any?` CRYSTAL - - expect_correction source, <<-CRYSTAL - {{ ![1, 2, 3].empty? }} - CRYSTAL end end end diff --git a/src/ameba/rule/performance/any_instead_of_empty.cr b/src/ameba/rule/performance/any_instead_of_empty.cr index 9e8d41ee..cee76ef6 100644 --- a/src/ameba/rule/performance/any_instead_of_empty.cr +++ b/src/ameba/rule/performance/any_instead_of_empty.cr @@ -42,14 +42,10 @@ module Ameba::Rule::Performance return unless node.block.nil? && node.args.empty? return unless node.obj - return unless location = node.location return unless name_location = node.name_location return unless end_location = name_end_location(node) - issue_for name_location, end_location, MSG do |corrector| - corrector.insert_before(location, '!') - corrector.replace(name_location, end_location, "empty?") - end + issue_for name_location, end_location, MSG end end end From d31e41b8a75f691df78f6f5753d6c45dc0879d40 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 16:23:36 +0100 Subject: [PATCH 30/32] Fix specs on Crystal nightly (soon to be v1.7.0) --- spec/ameba/rule/lint/duplicated_require_spec.cr | 2 +- spec/ameba/rule/lint/redundant_with_index_spec.cr | 4 ++-- spec/ameba/rule/lint/redundant_with_object_spec.cr | 2 +- spec/ameba/rule/performance/any_after_filter_spec.cr | 6 +++--- spec/ameba/rule/performance/compact_after_map_spec.cr | 2 +- .../rule/performance/first_last_after_filter_spec.cr | 8 ++++---- spec/ameba/rule/performance/flatten_after_map_spec.cr | 2 +- spec/ameba/rule/performance/map_instead_of_block_spec.cr | 6 +++--- spec/ameba/rule/performance/size_after_filter_spec.cr | 6 +++--- spec/ameba/rule/style/query_bool_methods_spec.cr | 4 ++-- 10 files changed, 21 insertions(+), 21 deletions(-) diff --git a/spec/ameba/rule/lint/duplicated_require_spec.cr b/spec/ameba/rule/lint/duplicated_require_spec.cr index 6967d764..031fd195 100644 --- a/spec/ameba/rule/lint/duplicated_require_spec.cr +++ b/spec/ameba/rule/lint/duplicated_require_spec.cr @@ -17,7 +17,7 @@ module Ameba::Rule::Lint require "big" require "math" require "big" - # ^{} error: Duplicated require of `big` + # ^^^^^^^^^^^ error: Duplicated require of `big` CRYSTAL expect_no_corrections source diff --git a/spec/ameba/rule/lint/redundant_with_index_spec.cr b/spec/ameba/rule/lint/redundant_with_index_spec.cr index 494ea4ef..6f356c0c 100644 --- a/spec/ameba/rule/lint/redundant_with_index_spec.cr +++ b/spec/ameba/rule/lint/redundant_with_index_spec.cr @@ -77,7 +77,7 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil issue.location.to_s.should eq "source.cr:2:19" - issue.end_location.to_s.should eq "source.cr:2:29" + issue.end_location.to_s.should eq "source.cr:2:28" issue.message.should eq "Remove redundant with_index" end end @@ -155,7 +155,7 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil issue.location.to_s.should eq "source.cr:2:14" - issue.end_location.to_s.should eq "source.cr:2:29" + issue.end_location.to_s.should eq "source.cr:2:28" issue.message.should eq "Use each instead of each_with_index" end end diff --git a/spec/ameba/rule/lint/redundant_with_object_spec.cr b/spec/ameba/rule/lint/redundant_with_object_spec.cr index bc3a9baf..f7fbc6d8 100644 --- a/spec/ameba/rule/lint/redundant_with_object_spec.cr +++ b/spec/ameba/rule/lint/redundant_with_object_spec.cr @@ -76,7 +76,7 @@ module Ameba::Rule::Lint issue = s.issues.first issue.rule.should_not be_nil issue.location.to_s.should eq "source.cr:2:14" - issue.end_location.to_s.should eq "source.cr:2:30" + issue.end_location.to_s.should eq "source.cr:2:29" issue.message.should eq "Use `each` instead of `each_with_object`" end end diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index c02cb7e5..f902ae0b 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/spec/ameba/rule/performance/any_after_filter_spec.cr @@ -17,7 +17,7 @@ module Ameba::Rule::Performance it "reports if there is select followed by any? without a block" do source = expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.any? - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `select {...}.any?` + # ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `select {...}.any?` CRYSTAL expect_no_corrections source @@ -32,7 +32,7 @@ module Ameba::Rule::Performance it "reports if there is reject followed by any? without a block" do source = expect_issue subject, <<-CRYSTAL [1, 2, 3].reject { |e| e > 2 }.any? - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `reject {...}.any?` + # ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `reject {...}.any?` CRYSTAL expect_no_corrections source @@ -60,7 +60,7 @@ module Ameba::Rule::Performance it "reports in macro scope" do source = expect_issue subject, <<-CRYSTAL {{ [1, 2, 3].reject { |e| e > 2 }.any? }} - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `reject {...}.any?` + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `any? {...}` instead of `reject {...}.any?` CRYSTAL expect_no_corrections source diff --git a/spec/ameba/rule/performance/compact_after_map_spec.cr b/spec/ameba/rule/performance/compact_after_map_spec.cr index 544e2905..b09e4d97 100644 --- a/spec/ameba/rule/performance/compact_after_map_spec.cr +++ b/spec/ameba/rule/performance/compact_after_map_spec.cr @@ -19,7 +19,7 @@ module Ameba::Rule::Performance it "reports if there is map followed by compact call" do expect_issue subject, <<-CRYSTAL (1..3).map(&.itself).compact - # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `compact_map {...}` instead of `map {...}.compact` + # ^^^^^^^^^^^^^^^^^^^^^ error: Use `compact_map {...}` instead of `map {...}.compact` CRYSTAL end diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index d3e290ee..7d0f320f 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr @@ -17,7 +17,7 @@ module Ameba::Rule::Performance it "reports if there is select followed by last" do expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.last - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last` + # ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last` CRYSTAL end @@ -30,14 +30,14 @@ module Ameba::Rule::Performance it "reports if there is select followed by last?" do expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.last? - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last?` + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last?` CRYSTAL end it "reports if there is select followed by first" do expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.first - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first` + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first` CRYSTAL end @@ -50,7 +50,7 @@ module Ameba::Rule::Performance it "reports if there is select followed by first?" do expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.first? - # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first?` + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first?` CRYSTAL end diff --git a/spec/ameba/rule/performance/flatten_after_map_spec.cr b/spec/ameba/rule/performance/flatten_after_map_spec.cr index a7f89193..3d498fdd 100644 --- a/spec/ameba/rule/performance/flatten_after_map_spec.cr +++ b/spec/ameba/rule/performance/flatten_after_map_spec.cr @@ -13,7 +13,7 @@ module Ameba::Rule::Performance it "reports if there is map followed by flatten call" do expect_issue subject, <<-CRYSTAL %w[Alice Bob].map(&.chars).flatten - # ^^^^^^^^^^^^^^^^^^^^^ error: Use `flat_map {...}` instead of `map {...}.flatten` + # ^^^^^^^^^^^^^^^^^^^^ error: Use `flat_map {...}` instead of `map {...}.flatten` CRYSTAL end diff --git a/spec/ameba/rule/performance/map_instead_of_block_spec.cr b/spec/ameba/rule/performance/map_instead_of_block_spec.cr index b154ce48..878e22bc 100644 --- a/spec/ameba/rule/performance/map_instead_of_block_spec.cr +++ b/spec/ameba/rule/performance/map_instead_of_block_spec.cr @@ -14,7 +14,7 @@ module Ameba::Rule::Performance it "reports if there is map followed by sum without a block" do expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum - # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + # ^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` CRYSTAL end @@ -27,14 +27,14 @@ module Ameba::Rule::Performance it "reports if there is map followed by sum without a block (with argument)" do expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum(0) - # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + # ^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` CRYSTAL end it "reports if there is map followed by sum with a block" do expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum(&.itself) - # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + # ^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` CRYSTAL end diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index f00c7c8d..1b081738 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/spec/ameba/rule/performance/size_after_filter_spec.cr @@ -19,7 +19,7 @@ module Ameba::Rule::Performance it "reports if there is a select followed by size" do expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.size - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `select {...}.size`. + # ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `select {...}.size`. CRYSTAL end @@ -32,14 +32,14 @@ module Ameba::Rule::Performance it "reports if there is a reject followed by size" do expect_issue subject, <<-CRYSTAL [1, 2, 3].reject { |e| e < 2 }.size - # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. + # ^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. CRYSTAL end it "reports if a block shorthand used" do expect_issue subject, <<-CRYSTAL [1, 2, 3].reject(&.empty?).size - # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. + # ^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. CRYSTAL end diff --git a/spec/ameba/rule/style/query_bool_methods_spec.cr b/spec/ameba/rule/style/query_bool_methods_spec.cr index e5716000..5c71ec51 100644 --- a/spec/ameba/rule/style/query_bool_methods_spec.cr +++ b/spec/ameba/rule/style/query_bool_methods_spec.cr @@ -51,7 +51,7 @@ module Ameba::Rule::Style expect_issue subject, <<-CRYSTAL, call: {{ call }} class Foo %{call} foo : Bool = true - _{call} # ^ error: Consider using '%{call}?' for 'foo' + _{call} # ^^^ error: Consider using '%{call}?' for 'foo' end CRYSTAL end @@ -60,7 +60,7 @@ module Ameba::Rule::Style expect_issue subject, <<-CRYSTAL, call: {{ call }} class Foo %{call} foo : Bool - _{call} # ^ error: Consider using '%{call}?' for 'foo' + _{call} # ^^^ error: Consider using '%{call}?' for 'foo' def initialize(@foo = true) end From 8e86374d08689da2240a3d8334c01eec9c6b9386 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 17:44:46 +0100 Subject: [PATCH 31/32] Remove outdated spec --- spec/ameba/rule/lint/empty_expression_spec.cr | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/ameba/rule/lint/empty_expression_spec.cr b/spec/ameba/rule/lint/empty_expression_spec.cr index 9e9bbee7..d45e4626 100644 --- a/spec/ameba/rule/lint/empty_expression_spec.cr +++ b/spec/ameba/rule/lint/empty_expression_spec.cr @@ -84,11 +84,6 @@ module Ameba it_detects_empty_expression %( begin; end ) - it_detects_empty_expression %( - begin - nil - end - ) it_detects_empty_expression %( begin () From 3b79392ef2011bc7b570c17d88b792825cb7380b Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 21 Dec 2022 17:54:41 +0100 Subject: [PATCH 32/32] Simplify implementation of `Lint/EmptyExpression` rule --- src/ameba/rule/lint/empty_expression.cr | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/ameba/rule/lint/empty_expression.cr b/src/ameba/rule/lint/empty_expression.cr index 8a8f89ff..9ceb2147 100644 --- a/src/ameba/rule/lint/empty_expression.cr +++ b/src/ameba/rule/lint/empty_expression.cr @@ -34,19 +34,11 @@ module Ameba::Rule::Lint description "Disallows empty expressions" end - MSG = "Avoid empty expression %s" - MSG_EXRS = "Avoid empty expressions" - - def test(source, node : Crystal::NilLiteral) - exp = node_source(node, source.lines) - return if exp.in?(nil, "nil") - - issue_for node, MSG % exp - end + MSG = "Avoid empty expressions" def test(source, node : Crystal::Expressions) return unless node.expressions.size == 1 && node.expressions.first.nop? - issue_for node, MSG_EXRS + issue_for node, MSG end end end