search

rubocop / rubocop-ast

RuboCop's AST extensions and NodePattern functionality
https://docs.rubocop.org/rubocop-ast
MIT License
112 stars 53 forks source link

Define `Node#*_type?` methods using macro #297

Open sambostock opened 5 months ago

sambostock commented 5 months ago

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.

I don't think we need a changelog entry, as no functionality has changed, but I'm happy to add one if desired.

To view the generated docs locally, run yard server -r in your terminal and visit http://localhost:8808/docs/RuboCop/AST/Node in your browser.

image

...

image

...

marcandre commented 5 months ago

Very nice PR, thank you.

I have one concern though which is that the current code is future proof, in that it will work fine with newer versions of parser, even if these future versions add a new node type. Maybe we should add call def_node_type_predicate on the list of node types - the list of node types we know of, in case that list isn't empty?

bbatsov commented 3 weeks ago

@sambostock ping :-)

sambostock commented 3 weeks ago

@marcandre I've tentatively pushed a commit which adds the fallback. However, the only scenario I can think of in which it would be relevant would be if the host application bumps their parser version and gains support for a new Ruby version's syntax (or some other change that introduces a node type), and has custom cops which expect to use a predicate method for the new node type.

This seems unlikely, and I'm not entirely sure if it's possible (wouldn't they need to use a version of RuboCop supporting their TargetRubyVersion?). I made sure to add a test which fails if any new Parser::Meta::NODE_TYPES are introduced. Do you think that's sufficient, or is there a scenario I'm not thinking of?