Add struct_constructor?, class_definition? and module_definition? matchers

[tejasbubane]

Closes #28

[marcandre]

Getting closer...

[marcandre]

Getting closer, but a few things to fix

[marcandre]

Also, I think class_constructor? has an issue with ::Class.new.
I'll reiterate that I think a top_level_constant would probably be useful, it seems an easy error to make

[bbatsov]

@tejasbubane ping :-)

[tejasbubane]

@marcandre I have pushed some corrections & additional tests. I gave a shot at top_level_constant but could not wrap my head around different argument sizes. Here's what I tried so far:

def_node_matcher :top_level_constant?, '(const {nil? (cbase)} {%1 %2})'

(send top_level_constant?(:Class, :Module) :new ...) # matches
(send top_level_constant?(:Struct) :new ...) # does not match

def_node_matcher :top_level_constant?, '(const {nil? (cbase)} %1)'

(send top_level_constant?({:Class :Module}) :new ...) # does not compile

Am I missing something?

[marcandre]

Right, it is a bit tricky for multiple possibilities, but I think this could work:

def_node_matcher :top_level_constant?, '(const {nil? (cbase)} %1)'

def_node_matcher :foo, "(send top_level_constant?(%modules) :new ...)", modules: Set[:Class, :Module]

But you raise a good point, it would be nice if that worked out of the box. I'll see if I can make #function({literal literal literal}) and same for predicate?({...}) work

[marcandre]

Update: Looks like it should be pretty simple actually to allow any pattern as argument. Implementation is done, I need tests now 😄, but time for 🛌

[tejasbubane]

Update: Looks like it should be pretty simple actually to allow any pattern as argument. Implementation is done, I need tests now 😄, but time for 🛌

@marcandre Did you implement this?

[marcandre]

Update: Looks like it should be pretty simple actually to allow any pattern as argument. Implementation is done, I need tests now 😄, but time for 🛌

@marcandre Did you implement this?

Yes! 🎉

[marcandre]

Really clean with #global_const 👍

There are two more small changes.

Also, maybe it would be useful to capture the definition body, what do you think?

[marcandre]

Maybe best to simply check for (block ..., no? It could be var = Module.new {}, prepend Module.new, ...

Same for class_definition?

[tejasbubane]

@marcandre Changed to capture body. The CI failures seem to be related to rubocop-rspec: rubocop/rubocop-rspec#974

[marcandre]

All green now 🎉, merging. Great work! 💪