diff --git a/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr b/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr index 5053ff67..3e69729b 100644 --- a/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr +++ b/spec/ameba/ast/visitors/flow_expression_visitor_spec.cr @@ -1,8 +1,6 @@ require "../../../spec_helper" module Ameba::AST - source = Source.new "" - describe FlowExpressionVisitor do it "creates an expression for return" do rule = FlowExpressionRule.new diff --git a/spec/ameba/rule/lint/useless_assign_spec.cr b/spec/ameba/rule/lint/useless_assign_spec.cr index d764a7da..7c7310de 100644 --- a/spec/ameba/rule/lint/useless_assign_spec.cr +++ b/spec/ameba/rule/lint/useless_assign_spec.cr @@ -3,124 +3,107 @@ require "../../../spec_helper" module Ameba::Rule::Lint describe UselessAssign do subject = UselessAssign.new + .tap(&.exclude_type_declarations = false) it "does not report used assignments" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 2 a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports a useless assignment in a method" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a = 2 + # ^ error: Useless assignment to variable `a` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports a useless assignment in a proc" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL ->() { a = 2 + # ^ error: Useless assignment to variable `a` } - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports a useless assignment in a block" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method 3.times do a = 1 + # ^ error: Useless assignment to variable `a` end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports a useless assignment in a proc inside def" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method ->() { a = 2 + # ^ error: Useless assignment to variable `a` } end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "does not report ignored assignments" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL payload, _header = decode puts payload - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports a useless assignment in a proc inside a block" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method 3.times do ->() { a = 2 + # ^ error: Useless assignment to variable `a` } end end - ) - subject.catch(s).should_not be_valid - end - - it "reports rule, position and a message" do - s = Source.new %( - def method - a = 2 - 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.message.should eq "Useless assignment to variable `a`" + CRYSTAL end it "does not report useless assignment of instance var" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Cls def initialize(@name) end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if assignment used in the inner block scope" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method var = true 3.times { var = false } end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assigned is not referenced in the inner block scope" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method var = true + # ^^^ error: Useless assignment to variable `var` 3.times {} end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if assignment in referenced in inner block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method two = true @@ -132,184 +115,172 @@ module Ameba::Rule::Lint two.should be_true end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if first assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method var = true + # ^^^ error: Useless assignment to variable `var` var = false var end - ) - subject.catch(s).should_not be_valid - s.issues.first.location.to_s.should eq ":2:3" + CRYSTAL end it "reports if variable reassigned and not used" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method var = true + # ^^^ error: Useless assignment to variable `var` var = false + # ^^^ error: Useless assignment to variable `var` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "does not report if variable used in a condition" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 1 if a nil end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports second assignment as useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a = 1 a = a + 1 + # ^ error: Useless assignment to variable `a` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "does not report if variable is referenced in other assignment" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method if f = get_something @f = f end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if variable is referenced in a setter" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method foo = 2 table[foo] ||= "bar" end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if variable is reassigned but not referenced" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method foo = 1 puts foo foo = 2 + # ^^^ error: Useless assignment to variable `foo` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "does not report if variable is referenced in a call" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method if f = FORMATTER @formatter = f.new end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if a setter is invoked with operator assignment" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method obj = {} of Symbol => Int32 obj[:name] = 3 end - ) - subject.catch(s).should be_valid + CRYSTAL end context "block unpacking" do it "does not report if the first arg is transformed and not used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL collection.each do |(a, b)| puts b end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if the second arg is transformed and not used" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL collection.each do |(a, b)| puts a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if all transformed args are not used in a block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL collection.each do |(foo, bar), (baz, _qux), index, object| end - ) - subject.catch(s).should be_valid + CRYSTAL end end it "does not report if assignment is referenced in a proc" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method called = false ->() { called = true } called end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if variable is shadowed in inner scope" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method i = 1 + # ^ error: Useless assignment to variable `i` 3.times do |i| i + 1 end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "does not report if parameter is referenced after the branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(param) 3.times do param = 3 end param end - ) - subject.catch(s).should be_valid + CRYSTAL end context "op assigns" do it "does not report if variable is referenced below the op assign" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 1 a += 1 a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if variable is referenced in op assign few times" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 1 a += 1 @@ -317,103 +288,76 @@ module Ameba::Rule::Lint a = a + 1 a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if variable is not referenced below the op assign" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a = 1 a += 1 + # ^ error: Useless assignment to variable `a` end - ) - subject.catch(s).should_not be_valid - end - - it "reports rule, location and a message" do - s = Source.new %( - def method - b = 2 - a = 3 - a += 1 - end - ), "source.cr" - subject.catch(s).should_not be_valid - - issue = s.issues.last - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:4:3" - issue.message.should eq "Useless assignment to variable `a`" + CRYSTAL end end context "multi assigns" do it "does not report if all assigns are referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a, b = {1, 2} a + b end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if one assign is not referenced" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a, b = {1, 2} + # ^ error: Useless assignment to variable `b` a end - ) - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.location.to_s.should eq ":2:6" - issue.message.should eq "Useless assignment to variable `b`" + CRYSTAL end it "reports if both assigns are reassigned and useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a, b = {1, 2} + # ^ error: Useless assignment to variable `a` + # ^ error: Useless assignment to variable `b` a, b = {3, 4} + # ^ error: Useless assignment to variable `a` + # ^ error: Useless assignment to variable `b` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports if both assigns are not referenced" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a, b = {1, 2} + # ^ error: Useless assignment to variable `a` + # ^ error: Useless assignment to variable `b` end - ) - subject.catch(s).should_not be_valid - - issue = s.issues.first - issue.location.to_s.should eq ":2:3" - issue.message.should eq "Useless assignment to variable `a`" - - issue = s.issues.last - issue.location.to_s.should eq ":2:6" - issue.message.should eq "Useless assignment to variable `b`" + CRYSTAL end end context "top level" do it "reports if assignment is not referenced" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL a = 1 + # ^{} error: Useless assignment to variable `a` a = 2 - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":1:1" - s.issues.last.location.to_s.should eq ":2:1" + # ^{} error: Useless assignment to variable `a` + CRYSTAL end it "doesn't report if assignments are referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 a += 1 a @@ -421,63 +365,70 @@ module Ameba::Rule::Lint b, c = {1, 2} b c - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is captured by block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 3.times do a = 2 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment initialized and captured by block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a : String? = nil 1.times do a = "Fotis" end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if this is a record declaration" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL record Foo, foo = "foo" - ) - subject.catch(s).should be_valid + CRYSTAL + end + + it "doesn't report if this is an accessor declaration" do + accessor_macros = %w[setter class_setter] + %w[getter class_getter property class_property].each do |name| + accessor_macros << name + accessor_macros << "#{name}?" + accessor_macros << "#{name}!" + end + accessor_macros.each do |accessor| + expect_no_issues subject, <<-CRYSTAL + class Foo + #{accessor} foo : String? + #{accessor} bar = "bar" + end + CRYSTAL + end end it "does not report if assignment is referenced after the record declaration" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 2 record Bar, foo = 3 # foo = 3 is not parsed as assignment puts foo - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is not referenced after the record declaration" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL foo = 2 + # ^ error: Useless assignment to variable `foo` record Bar, foo = 3 - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.location.to_s.should eq "source.cr:1:1" - issue.message.should eq "Useless assignment to variable `foo`" + CRYSTAL end it "doesn't report if type declaration assigned inside module and referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL module A foo : String? = "foo" @@ -487,29 +438,28 @@ module Ameba::Rule::Lint p foo end - - ) - subject.catch(s).should be_valid + CRYSTAL end - it "doesn't report if type declaration assigned inside class" do - s = Source.new %( + it "reports if type declaration assigned inside class" do + expect_issue subject, <<-CRYSTAL class A foo : String? = "foo" + # ^^^^^^^^^^^^^^^^^^^^^ error: Useless assignment to variable `foo` def method foo = "bar" + # ^^^ error: Useless assignment to variable `foo` end end - ) - subject.catch(s).should be_valid + CRYSTAL end end context "branching" do context "if-then-else" do it "doesn't report if assignment is consumed by branches" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 0 if something @@ -519,12 +469,11 @@ module Ameba::Rule::Lint end a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is in one branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 0 if something @@ -534,65 +483,59 @@ module Ameba::Rule::Lint end a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is in one line branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 0 a = 1 if something a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless in the branch" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) if a a = 2 + # ^ error: Useless assignment to variable `a` end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports if only last assignment is referenced in a branch" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) a = 1 if a a = 2 + # ^ error: Useless assignment to variable `a` a = 3 end a end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":4:5" + CRYSTAL end it "does not report of assignments are referenced in all branches" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method if matches matches = owner.lookup_matches signature else matches = owner.lookup_matches signature end - matches end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report referenced assignments in inner branches" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method has_newline = false @@ -606,14 +549,13 @@ module Ameba::Rule::Lint has_newline end - ) - subject.catch(s).should be_valid + CRYSTAL end end context "unless-then-else" do it "doesn't report if assignment is consumed by branches" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 0 unless something @@ -623,32 +565,29 @@ module Ameba::Rule::Lint end a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is a useless assignment in a branch" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method a = 0 unless something a = 1 + # ^ error: Useless assignment to variable `a` a = 2 else a = 2 end a end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":4:5" + CRYSTAL end end context "case" do it "does not report if assignment is referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) case a when /foo/ @@ -658,92 +597,82 @@ module Ameba::Rule::Lint end puts a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) case a when /foo/ a = 1 + # ^ error: Useless assignment to variable `a` when /bar/ a = 2 + # ^ error: Useless assignment to variable `a` end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":4:5" - s.issues.last.location.to_s.should eq ":6:5" + CRYSTAL end it "doesn't report if assignment is referenced in cond" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 2 case a when /foo/ end end - ) - subject.catch(s).should be_valid + CRYSTAL end end context "binary operator" do it "does not report if assignment is referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) (a = 1) && (b = 1) a + b end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) (a = 1) || (b = 1) + # ^ error: Useless assignment to variable `b` a end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:15" + CRYSTAL end end context "while" do it "does not report if assignment is referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) while a < 10 a = a + 1 end a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) while a < 10 b = a + # ^ error: Useless assignment to variable `b` end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":3:5" + CRYSTAL end it "does not report if assignment is referenced in a loop" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 3 result = 0 @@ -754,12 +683,11 @@ module Ameba::Rule::Lint end result end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if assignment is referenced as param in a loop" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) result = 0 @@ -769,12 +697,11 @@ module Ameba::Rule::Lint end result end - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if assignment is referenced in loop and inner branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) result = 0 @@ -788,12 +715,11 @@ module Ameba::Rule::Lint end result end - ) - subject.catch(s).should be_valid + CRYSTAL end it "works properly if there is branch with blank node" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def visit count = 0 while true @@ -806,108 +732,94 @@ module Ameba::Rule::Lint count += 1 end end - ) - subject.catch(s).should be_valid + CRYSTAL end end context "until" do it "does not report if assignment is referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) until a > 10 a = a + 1 end a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) until a > 10 b = a + 1 + # ^ error: Useless assignment to variable `b` end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":3:5" + CRYSTAL end end context "exception handler" do it "does not report if assignment is referenced in body" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) a = 2 rescue a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in ensure" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) a = 2 ensure a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in else" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) a = 2 rescue else a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if assignment is useless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def method(a) rescue a = 2 + # ^ error: Useless assignment to variable `a` end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":3:3" + CRYSTAL end end end context "typeof" do it "reports useless assignments in typeof" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL typeof(begin foo = 1 + # ^^^ error: Useless assignment to variable `foo` bar = 2 + # ^^^ error: Useless assignment to variable `bar` end) - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":2:3" - s.issues.first.message.should eq "Useless assignment to variable `foo`" - - s.issues.last.location.to_s.should eq ":3:3" - s.issues.last.message.should eq "Useless assignment to variable `bar`" + CRYSTAL end end context "macro" do it "doesn't report if assignment is referenced in macro" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 2 {% if flag?(:bits64) %} @@ -916,12 +828,11 @@ module Ameba::Rule::Lint a {% end %} end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report referenced assignments in macro literal" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method a = 2 {% if flag?(:bits64) %} @@ -931,12 +842,11 @@ module Ameba::Rule::Lint {% end %} puts a end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in macro def" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL macro macro_call puts x end @@ -945,12 +855,11 @@ module Ameba::Rule::Lint x = 1 macro_call end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in a macro below" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Foo def foo a = 1 @@ -961,73 +870,69 @@ module Ameba::Rule::Lint puts a end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in a macro expression as string" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 1 puts {{ "foo".id }} - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in for macro in exp" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 22 {% for x in %w[foo] %} add({{ x.id }}) {% end %} - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in for macro in body" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 22 {% for x in %w[bar] %} puts {{ "foo".id }} {% end %} - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in if macro in cond" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 22 + {% if "foo".id %} {% end %} - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in if macro in then" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 22 + {% if true %} puts {{ "foo".id }} {% end %} - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if assignment is referenced in if macro in else" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL foo = 22 + {% if true %} {% else %} puts {{ "foo".id }} {% end %} - ) - subject.catch(s).should be_valid + CRYSTAL end end it "does not report if variable is referenced and there is a deep level scope" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL response = JSON.build do |json| json.object do json.object do @@ -1064,35 +969,90 @@ module Ameba::Rule::Lint response = JSON.parse(response) response - ) - subject.catch(s).should be_valid + CRYSTAL + end + + context "type declaration" do + it "reports if it's not referenced at a top level" do + expect_issue subject, <<-CRYSTAL + a : String? + # ^^^^^^^^^ error: Useless assignment to variable `a` + CRYSTAL + end + + it "reports if it's not referenced in a method" do + expect_issue subject, <<-CRYSTAL + def foo + a : String? + # ^^^^^^^^^^^ error: Useless assignment to variable `a` + end + CRYSTAL + end + + it "reports if it's not referenced in a class" do + expect_issue subject, <<-CRYSTAL + class Foo + a : String? + # ^^^^^^^^^^^ error: Useless assignment to variable `a` + end + CRYSTAL + end + + it "doesn't report if it's referenced" do + expect_no_issues subject, <<-CRYSTAL + def foo + a : String? + a + end + CRYSTAL + end + end + + context "#properties" do + context "#exclude_type_declarations" do + it "doesn't report type declarations if enabled" do + rule = UselessAssign.new + rule.exclude_type_declarations = true + + expect_no_issues rule, <<-CRYSTAL + a : String? + + class Foo + b : String? + end + + def foo + c : String? + end + CRYSTAL + end + end end context "uninitialized" do it "reports if uninitialized assignment is not referenced at a top level" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL a = uninitialized U - ) - subject.catch(s).should_not be_valid + # ^{} error: Useless assignment to variable `a` + CRYSTAL end it "reports if uninitialized assignment is not referenced in a method" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def foo a = uninitialized U + # ^ error: Useless assignment to variable `a` end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if uninitialized assignment is referenced" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def foo a = uninitialized U a end - ) - subject.catch(s).should be_valid + CRYSTAL end end end diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr index 08b42931..97b71483 100644 --- a/src/ameba/ast/scope.cr +++ b/src/ameba/ast/scope.cr @@ -186,9 +186,19 @@ module Ameba::AST node.is_a?(Crystal::Def) end + # Returns `true` if current scope is a class, `false` otherwise. + def class_def? + node.is_a?(Crystal::ClassDef) + end + + # Returns `true` if current scope is a module, `false` otherwise. + def module_def? + node.is_a?(Crystal::ModuleDef) + end + # Returns `true` if this scope is a top level scope, `false` otherwise. def top_level? - outer_scope.nil? || type_definition? + outer_scope.nil? end # Returns `true` if var is an argument in current scope, `false` otherwise. diff --git a/src/ameba/ast/variabling/assignment.cr b/src/ameba/ast/variabling/assignment.cr index 3ab1ba7f..7168f5cb 100644 --- a/src/ameba/ast/variabling/assignment.cr +++ b/src/ameba/ast/variabling/assignment.cr @@ -32,9 +32,7 @@ module Ameba::AST return unless scope = @variable.scope @branch = Branch.of(@node, scope) - @referenced = true if @variable.special? || - @variable.scope.type_definition? || - referenced_in_loop? + @referenced = true if @variable.special? || referenced_in_loop? end def referenced_in_loop? diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index 73aa89bc..0b93cdb9 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -170,22 +170,29 @@ module Ameba::AST # :nodoc: def visit(node : Crystal::Call) - case - when @current_scope.def? - if node.name.in?(SPECIAL_NODE_NAMES) && node.args.empty? - @current_scope.arguments.each do |arg| - variable = arg.variable + scope = @current_scope - ref = variable.reference(variable.node, @current_scope) - ref.explicit = false - end + case + when scope.top_level? && record_macro?(node) then return false + when scope.type_definition? && record_macro?(node) then return false + when scope.type_definition? && accessor_macro?(node) then return false + when scope.def? && special_node?(node) + scope.arguments.each do |arg| + variable = arg.variable + + ref = variable.reference(variable.node, scope) + ref.explicit = false end - true - when @current_scope.top_level? && record_macro?(node) - false - else - true end + true + end + + private def special_node?(node) + node.name.in?(SPECIAL_NODE_NAMES) && node.args.empty? + end + + private def accessor_macro?(node) + node.name.matches? /^(class_)?(getter[?!]?|setter|property[?!]?)$/ end private def record_macro?(node) diff --git a/src/ameba/rule/lint/useless_assign.cr b/src/ameba/rule/lint/useless_assign.cr index 85ac424f..ed7d536d 100644 --- a/src/ameba/rule/lint/useless_assign.cr +++ b/src/ameba/rule/lint/useless_assign.cr @@ -24,10 +24,12 @@ module Ameba::Rule::Lint # ``` # Lint/UselessAssign: # Enabled: true + # ExcludeTypeDeclarations: false # ``` class UselessAssign < Base properties do description "Disallows useless variable assignments" + exclude_type_declarations false end MSG = "Useless assignment to variable `%s`" @@ -39,7 +41,7 @@ module Ameba::Rule::Lint def test(source, node, scope : AST::Scope) scope.variables.each do |var| next if var.ignored? || var.used_in_macro? || var.captured_by_block? - next if scope.assigns_type_dec?(var.name) + next if exclude_type_declarations? && scope.assigns_type_dec?(var.name) var.assignments.each do |assign| next if assign.referenced?