search

Shopify / constant_resolver

Resolve a partially qualified Ruby constant reference to the fully qualified name and the path of the file defining it.
MIT License
26 stars 10 forks source link

Default namespace support #27

Closed mclark closed 2 years ago

mclark commented 2 years ago

As described in https://github.com/Shopify/packwerk/issues/186, Zeitwerk supports default modules for root directories. This pull request adds support for this capability to ConstantResolver.

I opted to keep this as simple as possible and just deal with Strings for the constant names. The resolver appears to inherently only be concerned with Strings anyway so it seemed to be consistent to me versus holding references to Module instances. If the provided load paths hash points to modules, as it does in the hash returned by Zeitwerk::Loader#root_dirs, then we just call to_s on them anyway.

For detecting whether the load paths are a hash or not, I duck check to see if the argument responds to :transform_keys. I'm not sure if this is ideal, maybe others have ideas?

I've also tested this change in concert with the Packwerk changes on this branch against the GitHub monolith to ensure things behave as expected. The GitHub monolith is old and has a few root directories which have default namespaces other than Object so this is a decent test case. I'm pleased to report the expected violations were detected resulting much more accurate reports.

cc @exterm @rafaelfranca from our discussions on https://github.com/Shopify/packwerk/issues/186

mclark commented 2 years ago

Oops! Bad etiquette, I should not have force pushed after a review ☝️ All that was doing was fixing a style linting error, adding a comma to the last element of an array.

exterm commented 2 years ago

Let's see if we can get a second review within a few days, but if not, I'm OK shipping this as is