From 6b56c87e78d2cadb5d1a9fad41bda32cbef2aade Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Mon, 11 Nov 2019 15:39:49 +0200 Subject: [PATCH] Shadowing via short instance variable syntax closes #122 --- .../lint/shadowing_local_outer_var_spec.cr | 10 ++++++++ src/ameba/ast/scope.cr | 25 +++++++++++++++++++ src/ameba/ast/variabling/ivariable.cr | 13 ++++++++++ src/ameba/ast/visitors/scope_visitor.cr | 5 ++++ .../rule/lint/shadowing_local_outer_var.cr | 3 ++- 5 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/ameba/ast/variabling/ivariable.cr diff --git a/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr b/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr index 5682b58f..b97d1221 100644 --- a/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr +++ b/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr @@ -151,6 +151,16 @@ module Ameba::Rule::Lint subject.catch(source).should be_valid end + it "does not report if argument shadows an ivar assignment" do + s = Source.new %( + def bar(@foo) + @foo.try do |foo| + end + end + ) + subject.catch(s).should be_valid + end + it "reports rule, location and message" do source = Source.new %( foo = 1 diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr index 729176a9..132e58a5 100644 --- a/src/ameba/ast/scope.cr +++ b/src/ameba/ast/scope.cr @@ -10,6 +10,9 @@ module Ameba::AST # Link to the arguments in current scope getter arguments = [] of Argument + # Link to the instance variables used in current scope + getter ivariables = [] of InstanceVariable + # Link to the outer scope getter outer_scope : Scope? @@ -44,11 +47,27 @@ module Ameba::AST variables << Variable.new(node, self) end + # Creates a new argument in the current scope. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.add_argument(arg_node) + # ``` def add_argument(node) add_variable Crystal::Var.new(node.name).at(node) arguments << Argument.new(node, variables.last) end + # Adds a new instance variable to the current scope. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.add_ivariable(ivar_node) + # ``` + def add_ivariable(node) + ivariables << InstanceVariable.new(node) + end + # Returns variable by its name or nil if it does not exist. # # ``` @@ -75,6 +94,12 @@ module Ameba::AST node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral) end + # Returns true instance variable assinged in this scope. + def assigns_ivar?(name) + arguments.find { |arg| arg.name == name } && + ivariables.find { |var| var.name == "@#{name}" } + end + # Returns true if and only if current scope represents some # type definition, for example a class. def type_definition? diff --git a/src/ameba/ast/variabling/ivariable.cr b/src/ameba/ast/variabling/ivariable.cr new file mode 100644 index 00000000..836ad345 --- /dev/null +++ b/src/ameba/ast/variabling/ivariable.cr @@ -0,0 +1,13 @@ +module Ameba::AST + class InstanceVariable + getter node : Crystal::InstanceVar + + delegate location, to: @node + delegate end_location, to: @node + delegate name, to: @node + delegate to_s, to: @node + + def initialize(@node) + end + end +end diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index 04e35071..973568bc 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -135,6 +135,11 @@ module Ameba::AST @current_scope.add_argument node end + # :nodoc: + def visit(node : Crystal::InstanceVar) + @current_scope.add_ivariable(node) + end + # :nodoc: def visit(node : Crystal::Var) variable = @current_scope.find_variable node.name diff --git a/src/ameba/rule/lint/shadowing_local_outer_var.cr b/src/ameba/rule/lint/shadowing_local_outer_var.cr index 36f1c86a..c088e945 100644 --- a/src/ameba/rule/lint/shadowing_local_outer_var.cr +++ b/src/ameba/rule/lint/shadowing_local_outer_var.cr @@ -54,7 +54,8 @@ module Ameba::Rule::Lint private def find_shadowing(source, scope) scope.arguments.each do |arg| next if arg.ignored? - if scope.outer_scope.try &.find_variable(arg.name) + outer_scope = scope.outer_scope + if outer_scope && outer_scope.find_variable(arg.name) && !outer_scope.assigns_ivar?(arg.name) issue_for arg.node, MSG % arg.name end end