diff --git a/spec/ameba/ast/variabling/type_def_variable_spec.cr b/spec/ameba/ast/variabling/type_def_variable_spec.cr new file mode 100644 index 00000000..217e8ada --- /dev/null +++ b/spec/ameba/ast/variabling/type_def_variable_spec.cr @@ -0,0 +1,31 @@ +require "../../../spec_helper" + +module Ameba::AST + describe TypeDecVariable do + var = Crystal::Var.new("foo") + declared_type = Crystal::Path.new("String") + type_dec = Crystal::TypeDeclaration.new(var, declared_type) + + describe "#initialize" do + it "creates a new type dec variable" do + variable = TypeDecVariable.new(type_dec) + variable.node.should_not be_nil + end + end + + describe "#name" do + it "returns var name" do + variable = TypeDecVariable.new(type_dec) + variable.name.should eq var.name + end + + it "raises if type declaration is incorrect" do + type_dec = Crystal::TypeDeclaration.new(declared_type, declared_type) + + expect_raises(Exception, "Unsupported var node type: Crystal::Path") do + TypeDecVariable.new(type_dec).name + end + end + 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 ba44bc11..a321f08a 100644 --- a/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr +++ b/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr @@ -136,6 +136,19 @@ module Ameba::Rule::Lint CRYSTAL end + it "doesn't report if it shadows type declaration" do + expect_no_issues subject, <<-CRYSTAL + class FooBar + getter index : String + + def bar + 3.times do |index| + end + end + end + CRYSTAL + end + it "doesn't report if it shadows throwaway arguments" do expect_no_issues subject, <<-CRYSTAL data = [{1, "a"}, {2, "b"}, {3, "c"}] diff --git a/spec/ameba/rule/lint/useless_assign_spec.cr b/spec/ameba/rule/lint/useless_assign_spec.cr index 2f67281c..a556d680 100644 --- a/spec/ameba/rule/lint/useless_assign_spec.cr +++ b/spec/ameba/rule/lint/useless_assign_spec.cr @@ -475,6 +475,35 @@ module Ameba::Rule::Lint issue.location.to_s.should eq "source.cr:1:1" issue.message.should eq "Useless assignment to variable `foo`" end + + it "doesn't report if type declaration assigned inside module and referenced" do + s = Source.new %( + module A + foo : String? = "foo" + + bar do + foo = "bar" + end + + p foo + end + + ) + subject.catch(s).should be_valid + end + + it "doesn't report if type declaration assigned inside class" do + s = Source.new %( + class A + foo : String? = "foo" + + def method + foo = "bar" + end + end + ) + subject.catch(s).should be_valid + end end context "branching" do diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr index ba0459c3..b74f8b62 100644 --- a/src/ameba/ast/scope.cr +++ b/src/ameba/ast/scope.cr @@ -19,6 +19,9 @@ module Ameba::AST # Link to the instance variables used in current scope getter ivariables = [] of InstanceVariable + # Link to the type declaration variables used in current scope + getter type_dec_variables = [] of TypeDecVariable + # Link to the outer scope getter outer_scope : Scope? @@ -74,6 +77,16 @@ module Ameba::AST ivariables << InstanceVariable.new(node) end + # Adds a new type declaration variable to the current scope. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.add_type_dec_variable(node) + # ``` + def add_type_dec_variable(node) + type_dec_variables << TypeDecVariable.new(node) + end + # Returns variable by its name or `nil` if it does not exist. # # ``` @@ -122,8 +135,13 @@ module Ameba::AST # Returns `true` if instance variable is assigned in this scope. def assigns_ivar?(name) - arguments.find(&.name.== name) && - ivariables.find(&.name.== "@#{name}") + arguments.any?(&.name.== name) && + ivariables.any?(&.name.== "@#{name}") + end + + # Returns `true` if type declaration variable is assigned in this scope. + def assigns_type_dec?(name) + type_dec_variables.any?(&.name.== name) || !!outer_scope.try(&.assigns_type_dec?(name)) end # Returns `true` if and only if current scope represents some @@ -161,7 +179,7 @@ module Ameba::AST # Returns `true` if this scope is a top level scope, `false` otherwise. def top_level? - outer_scope.nil? + outer_scope.nil? || type_definition? end # Returns `true` if var is an argument in current scope, `false` otherwise. diff --git a/src/ameba/ast/variabling/type_def_variable.cr b/src/ameba/ast/variabling/type_def_variable.cr new file mode 100644 index 00000000..3376b37f --- /dev/null +++ b/src/ameba/ast/variabling/type_def_variable.cr @@ -0,0 +1,21 @@ +module Ameba::AST + class TypeDecVariable + getter node : Crystal::TypeDeclaration + + delegate location, to: @node + delegate end_location, to: @node + delegate to_s, to: @node + + def initialize(@node) + end + + def name + case var = @node.var + when Crystal::Var, Crystal::InstanceVar, Crystal::ClassVar, Crystal::Global + var.name + else + raise "Unsupported var node type: #{var.class}" + end + end + end +end diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index 1dec9af8..a5c578ae 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -102,10 +102,19 @@ module Ameba::AST # :nodoc: def visit(node : Crystal::TypeDeclaration) - return if @current_scope.type_definition? - return if !(var = node.var).is_a?(Crystal::Var) + return unless (var = node.var).is_a?(Crystal::Var) @current_scope.add_variable(var) + @current_scope.add_type_dec_variable(node) + @current_assign = node.value unless node.value.nil? + end + + def end_visit(node : Crystal::TypeDeclaration) + return unless (var = node.var).is_a?(Crystal::Var) + + on_assign_end(var, node) + @current_assign = nil + on_scope_end(node) if @current_scope.eql?(node) end # :nodoc: diff --git a/src/ameba/rule/lint/shadowing_outer_local_var.cr b/src/ameba/rule/lint/shadowing_outer_local_var.cr index d61221ec..9a1f8c84 100644 --- a/src/ameba/rule/lint/shadowing_outer_local_var.cr +++ b/src/ameba/rule/lint/shadowing_outer_local_var.cr @@ -57,6 +57,7 @@ module Ameba::Rule::Lint next if variable.nil? || !variable.declared_before?(arg) next if outer_scope.assigns_ivar?(arg.name) + next if outer_scope.assigns_type_dec?(arg.name) issue_for arg.node, MSG % arg.name end diff --git a/src/ameba/rule/lint/useless_assign.cr b/src/ameba/rule/lint/useless_assign.cr index e09df45e..f0319329 100644 --- a/src/ameba/rule/lint/useless_assign.cr +++ b/src/ameba/rule/lint/useless_assign.cr @@ -39,6 +39,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) var.assignments.each do |assign| next if assign.referenced? || assign.transformed?