From 3470a3cf223f56a75188d2e143a82669e2eb76b9 Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Wed, 19 Jun 2024 13:34:31 -0400 Subject: [PATCH 1/2] Define `Node#*_type?` methods using macro This allows us to document the methods, without the verbosity of statically defining each one individually. Specs are dynamically generated for these methods, so there is no danger of omitting any. --- lib/rubocop/ast/node.rb | 161 +++++++++++++++++++++++++++++++--- spec/rubocop/ast/node_spec.rb | 10 +++ 2 files changed, 161 insertions(+), 10 deletions(-) diff --git a/lib/rubocop/ast/node.rb b/lib/rubocop/ast/node.rb index 98e175465..e9adc1e0d 100644 --- a/lib/rubocop/ast/node.rb +++ b/lib/rubocop/ast/node.rb @@ -137,6 +137,15 @@ def #{recursive_kind} # def recursive_litera RUBY end + # Define a +_type?+ predicate method for the given node type. + private_class_method def self.def_node_type_predicate(name, type = name) + class_eval <<~RUBY, __FILE__, __LINE__ + 1 + def #{name}_type? # def block_type? + @type == :#{type} # @type == :block + end # end + RUBY + end + # @see https://www.rubydoc.info/gems/ast/AST/Node:initialize def initialize(type, children = EMPTY_CHILDREN, properties = EMPTY_PROPERTIES) @mutable_attributes = {} @@ -164,21 +173,153 @@ def type?(*types) !group_type.nil? && types.include?(group_type) end - (Parser::Meta::NODE_TYPES - [:send]).each do |node_type| - method_name = "#{node_type.to_s.gsub(/\W/, '')}_type?" - class_eval <<~RUBY, __FILE__, __LINE__ + 1 - def #{method_name} # def block_type? - @type == :#{node_type} # @type == :block - end # end - RUBY - end + # @!group Type Predicates + + # @!macro [attach] def_node_type_predicate + # @!method $1_type? + # @return [Boolean] + def_node_type_predicate :true + def_node_type_predicate :false + def_node_type_predicate :nil + def_node_type_predicate :int + def_node_type_predicate :float + def_node_type_predicate :str + def_node_type_predicate :dstr + def_node_type_predicate :sym + def_node_type_predicate :dsym + def_node_type_predicate :xstr + def_node_type_predicate :regopt + def_node_type_predicate :regexp + def_node_type_predicate :array + def_node_type_predicate :splat + def_node_type_predicate :pair + def_node_type_predicate :kwsplat + def_node_type_predicate :hash + def_node_type_predicate :irange + def_node_type_predicate :erange + def_node_type_predicate :self + def_node_type_predicate :lvar + def_node_type_predicate :ivar + def_node_type_predicate :cvar + def_node_type_predicate :gvar + def_node_type_predicate :const + def_node_type_predicate :defined, :defined? + def_node_type_predicate :lvasgn + def_node_type_predicate :ivasgn + def_node_type_predicate :cvasgn + def_node_type_predicate :gvasgn + def_node_type_predicate :casgn + def_node_type_predicate :mlhs + def_node_type_predicate :masgn + def_node_type_predicate :op_asgn + def_node_type_predicate :and_asgn + def_node_type_predicate :ensure + def_node_type_predicate :rescue + def_node_type_predicate :arg_expr + def_node_type_predicate :or_asgn + def_node_type_predicate :back_ref + def_node_type_predicate :nth_ref + def_node_type_predicate :match_with_lvasgn + def_node_type_predicate :match_current_line + def_node_type_predicate :module + def_node_type_predicate :class + def_node_type_predicate :sclass + def_node_type_predicate :def + def_node_type_predicate :defs + def_node_type_predicate :undef + def_node_type_predicate :alias + def_node_type_predicate :args + def_node_type_predicate :cbase + def_node_type_predicate :arg + def_node_type_predicate :optarg + def_node_type_predicate :restarg + def_node_type_predicate :blockarg + def_node_type_predicate :block_pass + def_node_type_predicate :kwarg + def_node_type_predicate :kwoptarg + def_node_type_predicate :kwrestarg + def_node_type_predicate :kwnilarg + def_node_type_predicate :csend + def_node_type_predicate :super + def_node_type_predicate :zsuper + def_node_type_predicate :yield + def_node_type_predicate :block + def_node_type_predicate :and + def_node_type_predicate :not + def_node_type_predicate :or + def_node_type_predicate :if + def_node_type_predicate :when + def_node_type_predicate :case + def_node_type_predicate :while + def_node_type_predicate :until + def_node_type_predicate :while_post + def_node_type_predicate :until_post + def_node_type_predicate :for + def_node_type_predicate :break + def_node_type_predicate :next + def_node_type_predicate :redo + def_node_type_predicate :return + def_node_type_predicate :resbody + def_node_type_predicate :kwbegin + def_node_type_predicate :begin + def_node_type_predicate :retry + def_node_type_predicate :preexe + def_node_type_predicate :postexe + def_node_type_predicate :iflipflop + def_node_type_predicate :eflipflop + def_node_type_predicate :shadowarg + def_node_type_predicate :complex + def_node_type_predicate :rational + def_node_type_predicate :__FILE__ + def_node_type_predicate :__LINE__ + def_node_type_predicate :__ENCODING__ + def_node_type_predicate :ident + def_node_type_predicate :lambda + def_node_type_predicate :indexasgn + def_node_type_predicate :index + def_node_type_predicate :procarg0 + def_node_type_predicate :restarg_expr + def_node_type_predicate :blockarg_expr + def_node_type_predicate :objc_kwarg + def_node_type_predicate :objc_restarg + def_node_type_predicate :objc_varargs + def_node_type_predicate :numargs + def_node_type_predicate :numblock + def_node_type_predicate :forward_args + def_node_type_predicate :forwarded_args + def_node_type_predicate :forward_arg + def_node_type_predicate :case_match + def_node_type_predicate :in_match + def_node_type_predicate :in_pattern + def_node_type_predicate :match_var + def_node_type_predicate :pin + def_node_type_predicate :match_alt + def_node_type_predicate :match_as + def_node_type_predicate :match_rest + def_node_type_predicate :array_pattern + def_node_type_predicate :match_with_trailing_comma + def_node_type_predicate :array_pattern_with_tail + def_node_type_predicate :hash_pattern + def_node_type_predicate :const_pattern + def_node_type_predicate :if_guard + def_node_type_predicate :unless_guard + def_node_type_predicate :match_nil_pattern + def_node_type_predicate :empty_else + def_node_type_predicate :find_pattern + def_node_type_predicate :kwargs + def_node_type_predicate :match_pattern_p + def_node_type_predicate :match_pattern + def_node_type_predicate :forwarded_restarg + def_node_type_predicate :forwarded_kwrestarg - # Most nodes are of 'send' type, so this method is defined - # separately to make this check as fast as possible. def send_type? + # Most nodes are of 'send' type, so this method is defined + # separately to make this check as fast as possible. false end + # @!endgroup + # Returns the parent node, or `nil` if the receiver is a root node. # # @return [Node, nil] the parent node or `nil` diff --git a/spec/rubocop/ast/node_spec.rb b/spec/rubocop/ast/node_spec.rb index 96a795078..d161e7f6d 100644 --- a/spec/rubocop/ast/node_spec.rb +++ b/spec/rubocop/ast/node_spec.rb @@ -1109,4 +1109,14 @@ class << expr end end end + + describe '*_type? methods on Node' do + Parser::Meta::NODE_TYPES.each do |node_type| + method_name = "#{node_type.to_s.gsub(/\W/, '')}_type?" + + it "is not of #{method_name}" do + expect(described_class.allocate.public_send(method_name)).to be(false) + end + end + end end From 78c7935f1d52394dae91a905c669be758b381fc4 Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Fri, 8 Nov 2024 15:02:39 -0500 Subject: [PATCH 2/2] Fallback to dynamically defining node type predicates While we have tests which enforce that we're calling `def_node_type_predicate` for all known `Parser::Meta::NODE_TYPES`, it is possible for a host application to use a newer version of `parser`, which might support additional nodes, and for the application to attempt to access those nodes in custom cops. To preserve the previous forward compatibility, we fallback to generating any missing methods. They won't be documented, but at least they'll work. The tests will enforce that if rubocop-ast bumps its Parser version, all node type predicates are generated via `dev_node_type_predicate`. --- lib/rubocop/ast/node.rb | 11 ++++++++++ spec/rubocop/ast/node_spec.rb | 38 +++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/rubocop/ast/node.rb b/lib/rubocop/ast/node.rb index e9adc1e0d..1b6fa44cd 100644 --- a/lib/rubocop/ast/node.rb +++ b/lib/rubocop/ast/node.rb @@ -139,12 +139,15 @@ def #{recursive_kind} # def recursive_litera # Define a +_type?+ predicate method for the given node type. private_class_method def self.def_node_type_predicate(name, type = name) + @node_types_with_documented_predicate_method << type + class_eval <<~RUBY, __FILE__, __LINE__ + 1 def #{name}_type? # def block_type? @type == :#{type} # @type == :block end # end RUBY end + @node_types_with_documented_predicate_method = [] # @see https://www.rubydoc.info/gems/ast/AST/Node:initialize def initialize(type, children = EMPTY_CHILDREN, properties = EMPTY_PROPERTIES) @@ -317,6 +320,14 @@ def send_type? # separately to make this check as fast as possible. false end + @node_types_with_documented_predicate_method << :send + + # Ensure forward compatibility with new node types by defining methods for unknown node types. + # Note these won't get auto-generated documentation, which is why we prefer defining them above. + (Parser::Meta::NODE_TYPES - @node_types_with_documented_predicate_method).each do |node_type| + method_name = :"#{node_type.to_s.gsub(/\W/, '')}_type?" + define_method(method_name) { false } + end # @!endgroup diff --git a/spec/rubocop/ast/node_spec.rb b/spec/rubocop/ast/node_spec.rb index d161e7f6d..02958d52d 100644 --- a/spec/rubocop/ast/node_spec.rb +++ b/spec/rubocop/ast/node_spec.rb @@ -1110,13 +1110,43 @@ class << expr end end - describe '*_type? methods on Node' do - Parser::Meta::NODE_TYPES.each do |node_type| - method_name = "#{node_type.to_s.gsub(/\W/, '')}_type?" + Parser::Meta::NODE_TYPES.each do |node_type| + node_name = node_type.to_s.gsub(/\W/, '') + method_name = :"#{node_name}_type?" - it "is not of #{method_name}" do + describe "##{method_name}" do + it 'is false' do expect(described_class.allocate.public_send(method_name)).to be(false) end + + it 'is documented' do + expect(node_type_predicate_is_documented?(node_type)).to( + be(true), + missing_documentation_failure_message(method_name, node_name, node_type) + ) + end + + private + + def node_type_predicate_is_documented?(node_type) + described_class + .instance_variable_get(:@node_types_with_documented_predicate_method) + .include?(node_type) + end + + def missing_documentation_failure_message(method_name, node_name, node_type) + name_matches_type = node_type.to_s == node_name + + <<~MSG + #{described_class.name}##{method_name} is not documented as it was generated automatically as a fallback. + + To fix this, define it using the following macro instead: + + class #{described_class.name} < #{described_class.superclass.name} + # ... + def_node_type_predicate :#{node_name}#{", :#{node_type}" unless name_matches_type} + MSG + end end end end