From 27b83eb6e066cdb9734ea0f8f5645c247741ec4e Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Mon, 28 Oct 2024 17:50:11 -0400 Subject: [PATCH 1/2] Refactor `CasgnNode` and `ConstNode` to extract common functionality to `ConstantNode` mixin. --- lib/rubocop/ast.rb | 1 + lib/rubocop/ast/node/casgn_node.rb | 14 +--- lib/rubocop/ast/node/const_node.rb | 53 +-------------- lib/rubocop/ast/node/mixin/constant_node.rb | 62 ++++++++++++++++++ spec/rubocop/ast/casgn_node_spec.rb | 71 +++++++++++++++++++++ 5 files changed, 137 insertions(+), 64 deletions(-) create mode 100644 lib/rubocop/ast/node/mixin/constant_node.rb diff --git a/lib/rubocop/ast.rb b/lib/rubocop/ast.rb index c473150f3..1d0912160 100644 --- a/lib/rubocop/ast.rb +++ b/lib/rubocop/ast.rb @@ -27,6 +27,7 @@ require_relative 'ast/node/mixin/binary_operator_node' require_relative 'ast/node/mixin/collection_node' require_relative 'ast/node/mixin/conditional_node' +require_relative 'ast/node/mixin/constant_node' require_relative 'ast/node/mixin/hash_element_node' require_relative 'ast/node/mixin/method_dispatch_node' require_relative 'ast/node/mixin/modifier_node' diff --git a/lib/rubocop/ast/node/casgn_node.rb b/lib/rubocop/ast/node/casgn_node.rb index 50831ce48..617c8c70c 100644 --- a/lib/rubocop/ast/node/casgn_node.rb +++ b/lib/rubocop/ast/node/casgn_node.rb @@ -6,19 +6,9 @@ module AST # This will be used in place of a plain node when the builder constructs # the AST, making its methods available to all assignment nodes within RuboCop. class CasgnNode < Node - # The namespace of the constant being assigned. - # - # @return [Node, nil] the node associated with the scope (e.g. cbase, const, ...) - def namespace - node_parts[0] - end + include ConstantNode - # The name of the variable being assigned as a symbol. - # - # @return [Symbol] the name of the variable being assigned - def name - node_parts[1] - end + alias name short_name # The expression being assigned to the variable. # diff --git a/lib/rubocop/ast/node/const_node.rb b/lib/rubocop/ast/node/const_node.rb index f3dac0875..730478256 100644 --- a/lib/rubocop/ast/node/const_node.rb +++ b/lib/rubocop/ast/node/const_node.rb @@ -4,58 +4,7 @@ module RuboCop module AST # A node extension for `const` nodes. class ConstNode < Node - # @return [Node, nil] the node associated with the scope (e.g. cbase, const, ...) - def namespace - children[0] - end - - # @return [Symbol] the demodulized name of the constant: "::Foo::Bar" => :Bar - def short_name - children[1] - end - - # @return [Boolean] if the constant is a Module / Class, according to the standard convention. - # Note: some classes might have uppercase in which case this method - # returns false - def module_name? - short_name.match?(/[[:lower:]]/) - end - alias class_name? module_name? - - # @return [Boolean] if the constant starts with `::` (aka s(:cbase)) - def absolute? - return false unless namespace - - each_path.first.cbase_type? - end - - # @return [Boolean] if the constant does not start with `::` (aka s(:cbase)) - def relative? - !absolute? - end - - # Yield nodes for the namespace - # - # For `::Foo::Bar::BAZ` => yields: - # s(:cbase), then - # s(:const, :Foo), then - # s(:const, s(:const, :Foo), :Bar) - def each_path(&block) - return to_enum(__method__) unless block - - descendants = [] - last = self - loop do - last = last.children.first - break if last.nil? - - descendants << last - break unless last.const_type? - end - descendants.reverse_each(&block) - - self - end + include ConstantNode end end end diff --git a/lib/rubocop/ast/node/mixin/constant_node.rb b/lib/rubocop/ast/node/mixin/constant_node.rb new file mode 100644 index 000000000..3579a9a23 --- /dev/null +++ b/lib/rubocop/ast/node/mixin/constant_node.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module RuboCop + module AST + # Common functionality for nodes that deal with constants: + # `const`, `casgn`. + module ConstantNode + # @return [Node, nil] the node associated with the scope (e.g. cbase, const, ...) + def namespace + children[0] + end + + # @return [Symbol] the demodulized name of the constant: "::Foo::Bar" => :Bar + def short_name + children[1] + end + + # @return [Boolean] if the constant is a Module / Class, according to the standard convention. + # Note: some classes might have uppercase in which case this method + # returns false + def module_name? + short_name.match?(/[[:lower:]]/) + end + alias class_name? module_name? + + # @return [Boolean] if the constant starts with `::` (aka s(:cbase)) + def absolute? + return false unless namespace + + each_path.first.cbase_type? + end + + # @return [Boolean] if the constant does not start with `::` (aka s(:cbase)) + def relative? + !absolute? + end + + # Yield nodes for the namespace + # + # For `::Foo::Bar::BAZ` => yields: + # s(:cbase), then + # s(:const, :Foo), then + # s(:const, s(:const, :Foo), :Bar) + def each_path(&block) + return to_enum(__method__) unless block + + descendants = [] + last = self + loop do + last = last.children.first + break if last.nil? + + descendants << last + break unless last.const_type? + end + descendants.reverse_each(&block) + + self + end + end + end +end diff --git a/spec/rubocop/ast/casgn_node_spec.rb b/spec/rubocop/ast/casgn_node_spec.rb index 39fb2a019..9d7838abd 100644 --- a/spec/rubocop/ast/casgn_node_spec.rb +++ b/spec/rubocop/ast/casgn_node_spec.rb @@ -43,6 +43,14 @@ it { is_expected.to eq(:VAR) } end + describe '#short_name' do + subject { casgn_node.short_name } + + let(:source) { 'VAR = value' } + + it { is_expected.to eq(:VAR) } + end + describe '#expression' do include AST::Sexp @@ -52,4 +60,67 @@ it { is_expected.to eq(s(:send, nil, :value)) } end + + describe '#module_name?' do + context 'with a constant with only uppercase letters' do + let(:source) { 'VAR = value' } + + it { expect(casgn_node).not_to be_module_name } + end + + context 'with a constant with a lowercase letter' do + let(:source) { '::Foo::Bar = value' } + + it { expect(casgn_node).to be_module_name } + end + end + + describe '#absolute?' do + context 'with a constant starting with ::' do + let(:source) { '::VAR' } + + it { expect(casgn_node).to be_absolute } + end + + context 'with a constant not starting with ::' do + let(:source) { 'Foo::Bar::BAZ' } + + it { expect(casgn_node).not_to be_absolute } + end + + context 'with a non-namespaced constant' do + let(:source) { 'Foo' } + + it { expect(casgn_node).not_to be_absolute } + end + end + + describe '#relative?' do + context 'with a constant starting with ::' do + let(:source) { '::VAR' } + + it { expect(casgn_node).not_to be_relative } + end + + context 'with a constant not starting with ::' do + let(:source) { 'Foo::Bar::BAZ' } + + it { expect(casgn_node).to be_relative } + end + + context 'with a non-namespaced constant' do + let(:source) { 'Foo' } + + it { expect(casgn_node).to be_relative } + end + end + + describe '#each_path' do + let(:source) { '::Foo::Bar::BAZ = value' } + + it 'yields all parts of the namespace' do + expect(casgn_node.each_path.map(&:type)).to eq %i[cbase const const] + expect(casgn_node.each_path.to_a.last(2).map(&:short_name)).to eq %i[Foo Bar] + end + end end From f1b378ab45186b2ff9bb7603ec1b780c340973c2 Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Fri, 13 Aug 2021 14:58:24 -0400 Subject: [PATCH 2/2] Add `VarNode` class for `lvar`, `ivar`, `cvar` and `gvar` node types. --- ...dd_varnode_class_for_lvar_ivar_cvar_and.md | 1 + docs/modules/ROOT/pages/node_types.adoc | 8 +-- lib/rubocop/ast.rb | 1 + lib/rubocop/ast/builder.rb | 4 ++ lib/rubocop/ast/node/var_node.rb | 15 +++++ spec/rubocop/ast/var_node_spec.rb | 59 +++++++++++++++++++ 6 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 changelog/new_add_varnode_class_for_lvar_ivar_cvar_and.md create mode 100644 lib/rubocop/ast/node/var_node.rb create mode 100644 spec/rubocop/ast/var_node_spec.rb diff --git a/changelog/new_add_varnode_class_for_lvar_ivar_cvar_and.md b/changelog/new_add_varnode_class_for_lvar_ivar_cvar_and.md new file mode 100644 index 000000000..08c3e3676 --- /dev/null +++ b/changelog/new_add_varnode_class_for_lvar_ivar_cvar_and.md @@ -0,0 +1 @@ +* [#204](https://github.com/rubocop-hq/rubocop-ast/pull/204): Add `VarNode` class for `lvar`, `ivar`, `cvar` and `gvar` node types. ([@dvandersluis][]) diff --git a/docs/modules/ROOT/pages/node_types.adoc b/docs/modules/ROOT/pages/node_types.adoc index 8b1051379..9007663a8 100644 --- a/docs/modules/ROOT/pages/node_types.adoc +++ b/docs/modules/ROOT/pages/node_types.adoc @@ -88,7 +88,7 @@ The following fields are given when relevant to nodes in the source code: |csend|Null-safe method invocation, i.e. using `&.`|First child is the receiver node (e.g. `self`), second child is the method name (e.g. `:foo=`) and the remaining children (if any) are nodes representing arguments.|foo&.bar|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/SendNode[SendNode] -|cvar|Class variable access|One child, the variable name `:@@cfoo`|@@cfoo|N/A +|cvar|Class variable access|One child, the variable name `:@@cfoo`|@@cfoo|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/VarNode[VarNode] |cvasgn|Class variable assignment|Two children: the variable name `:@@foo` and the expression being assigned|@@foo = 5|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/AsgnNode[AsgnNode] @@ -124,7 +124,7 @@ The following fields are given when relevant to nodes in the source code: |forwarded_kwrestarg|Forwarding keyword arguments into a method call|None|foo(**)|N/A -|gvar|Global variable access|One child, the variable name as a symbol `:$foo`|$foo|N/A +|gvar|Global variable access|One child, the variable name as a symbol `:$foo`|$foo|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/VarNode[VarNode] |gvasgn|Global variable assignment|Two children, the variable name `:$foo` and the expression being assigned|$foo = 5|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/AsgnNode[AsgnNode] @@ -134,7 +134,7 @@ The following fields are given when relevant to nodes in the source code: |int|Integer literal|1, the integer value|-123|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/IntNode[IntNode] -|ivar|Instance variable access|One child, the variable name `:@foo`|@foo|N/A +|ivar|Instance variable access|One child, the variable name `:@foo`|@foo|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/VarNode[VarNode] |ivasgn|Instance variable assignment|Two children, the variable name `:@foo` and the expression being assigned|@foo = 5|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/AsgnNode[AsgnNode] @@ -152,7 +152,7 @@ The following fields are given when relevant to nodes in the source code: |kwrestargs|Double splat used for keyword arguments inside a function definition (as opposed to a function call). Must come inside an `args`.|One child - a symbol, representing the argument name, if a name is given. If no name given, it has no children..|def foo(**kwargs)|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/ArgNode[ArgNode] -|lvar|Local variable access|One child, the variable name|foo|N/A +|lvar|Local variable access|One child, the variable name|foo|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/VarNode[VarNode] |lvasgn|Local variable assignment|Two children: The variable name (symbol) and the expression.|a = some_thing|https://rubydoc.info/github/rubocop/rubocop-ast/RuboCop/AST/AsgnNode[AsgnNode] diff --git a/lib/rubocop/ast.rb b/lib/rubocop/ast.rb index 1d0912160..5ac1a6581 100644 --- a/lib/rubocop/ast.rb +++ b/lib/rubocop/ast.rb @@ -84,6 +84,7 @@ require_relative 'ast/node/super_node' require_relative 'ast/node/symbol_node' require_relative 'ast/node/until_node' +require_relative 'ast/node/var_node' require_relative 'ast/node/when_node' require_relative 'ast/node/while_node' require_relative 'ast/node/yield_node' diff --git a/lib/rubocop/ast/builder.rb b/lib/rubocop/ast/builder.rb index 106b3cc82..0ffa82933 100644 --- a/lib/rubocop/ast/builder.rb +++ b/lib/rubocop/ast/builder.rb @@ -87,6 +87,10 @@ class Builder < Parser::Builders::Default sym: SymbolNode, until: UntilNode, until_post: UntilNode, + lvar: VarNode, + ivar: VarNode, + cvar: VarNode, + gvar: VarNode, when: WhenNode, while: WhileNode, while_post: WhileNode, diff --git a/lib/rubocop/ast/node/var_node.rb b/lib/rubocop/ast/node/var_node.rb new file mode 100644 index 000000000..be1540754 --- /dev/null +++ b/lib/rubocop/ast/node/var_node.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module RuboCop + module AST + # A node extension for `lvar`, `ivar`, `cvar` and `gvar` nodes. + # This will be used in place of a plain node when the builder constructs + # the AST, making its methods available to all assignment nodes within RuboCop. + class VarNode < Node + # @return [Symbol] The name of the variable. + def name + node_parts[0] + end + end + end +end diff --git a/spec/rubocop/ast/var_node_spec.rb b/spec/rubocop/ast/var_node_spec.rb new file mode 100644 index 000000000..c1d94bb5a --- /dev/null +++ b/spec/rubocop/ast/var_node_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::AST::VarNode do + let(:node) { parse_source(source).node } + + describe '.new' do + context 'with a `lvar` node' do + let(:source) { 'x = 1; >>x<<' } + + it { expect(node).to be_a(described_class) } + end + + context 'with an `ivar` node' do + let(:source) { '@x' } + + it { expect(node).to be_a(described_class) } + end + + context 'with an `cvar` node' do + let(:source) { '@@x' } + + it { expect(node).to be_a(described_class) } + end + + context 'with an `gvar` node' do + let(:source) { '$x' } + + it { expect(node).to be_a(described_class) } + end + end + + describe '#name' do + subject { node.name } + + context 'with a `lvar` node' do + let(:source) { 'x = 1; >>x<<' } + + it { is_expected.to eq(:x) } + end + + context 'with an `ivar` node' do + let(:source) { '@x' } + + it { is_expected.to eq(:@x) } + end + + context 'with an `cvar` node' do + let(:source) { '@@x' } + + it { is_expected.to eq(:@@x) } + end + + context 'with an `gvar` node' do + let(:source) { '$x' } + + it { is_expected.to eq(:$x) } + end + end +end