Dependency violation not detected when using public folders for privacy boundary

[Enceradeira]

Description
When I enforce privacy for specific constants, a dependency violation to a public constant in that package is correctly detected. As soon as I reconfigure the package and set enforce_privacy: true and move the public constant to the package's public folder the dependency violation is not detected anymore.

I expect that the way how I declare public/private constants doesn't influence the dependency check/violations.

To Reproduce

1. Example 1 reports correctly a dependency violation (on branch working).

git clone  git@github.com:Enceradeira/rails_problem_packwerk.git
bundle install
git checkout working
packwerk check

app/models/accounting/accounts.rb:6:8
Dependency violation: ::Debtors::DebtorsService belongs to 'app/models/debtors', but 'app/models/accounting' does not specify a dependency on 'app/models/debtors'.
Are we missing an abstraction?
Is the code making the reference, and the referenced constant, in the right packages?

Inference details: this is a reference to ::Debtors::DebtorsService which seems to be defined in app/models/debtors/debtors_service.rb.
To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations


1 offense detected

No stale violations detected

2. Example 2 reports wrongly no violation (on branch not_working)

git checkout not_working
packwerk check

No offenses detected
No stale violations detected

3. The difference between Example 1 and Example 2 is only the way how I declare the privacy boundary in app/models/debtors/package.yml

git diff working not_working | cat    

diff --git a/app/models/debtors/package.yml b/app/models/debtors/package.yml
index 619e57e..958af2c 100644
--- a/app/models/debtors/package.yml
+++ b/app/models/debtors/package.yml
@@ -1,5 +1,5 @@
-enforce_privacy: 
-  - "::Debtors::Enquiries"
+enforce_privacy: true
+
 enforce_dependencies: true
 dependencies:
   - app/models/accounting
\ No newline at end of file
diff --git a/app/models/debtors/debtors_service.rb b/app/models/debtors/public/debtors_service.rb
similarity index 100%
rename from app/models/debtors/debtors_service.rb
rename to app/models/debtors/public/debtors_service.rb

Expected Behaviour
Example 2 should produce the same result as example 1. The way how I define the privacy boundary should not influence which dependency violations are detected.

Version Information

• Packwerk: 2.1.1
• Ruby 2.7.6p203
• Zeitwerk: 2.5.4
• Rails: 7.0.2.3

[rafaelfranca]

That seems like a bug to me. Are you able to investigate? Thanks.

[Enceradeira]

Thanks for confirming it as a bug. Yes I will investigate it.

[exterm]

Took a quick look - I'm pretty sure your not_working example doesn't work at all. If the file path is app/models/debtors/public/debtors_service.rb, both zeitwerk and packwerk expect it to be namespaced under Public, so Debtors::Public::DebtorsService.

That code should raise on your reference to Debtors::DebtorsService.

From a packwerk perspective, the reference is not a dependency violation because the referenced constant can't be found.

If you want to do something like this, you have a couple of options:

• use the public namespace, as shown above
• define app/models/debtors/public as a nested load path (can be a little confusing, but nested load paths are not crazy, see app/models/concerns which is usually a nested load path)
• restructure your application to use an engine-like layout (maybe use engines):

components/ # or packages, or another name that makes sense to you
  debtors/
    app/
      public/
      models/
      controllers/
      ...
  accounting/
    app/
      public/
      models/
      controllers/
      ...

That would make public a load path, so that the word doesn't need to appear in the namespace path.

We use both the second and third option; top level components, then packages with their own load paths nested inside of them. Load paths are set up with Rails Engines. However, the simplest solution is to just use Public as a namespace.

[exterm]

Not a bug.

[Enceradeira]

Thank's for the thorough analysis. My not_working example is correctly loaded by zeitwerk but it's using the following configuration for zeitwerk:

# config/initializers/zeitwerk.rb
Rails.autoloaders.main.collapse("#{Rails.root}/app/**/public")

That is why the constants in the public folder are loaded correctly even they are not in the Public namespace.

@exterm: Is the above configuration of zeitwerk therefore incompatible with packwerk? If yes, is it a missing feature or just not the way how packwerk is indented to work?

[exterm]

collapse is indeed not currently supported, AFAIR. We should probably document this somewhere. However, maybe #186 would implicitly add support for collapse too?

[exterm]

collapse didn't exist when we originally developed packwerk, and we never added support for it - there's no specific reason other than that we didn't need it, and this is the first time it's come up as far as I know.

[Enceradeira]

collapse public folders is useful for legacy apps. With it you are able to define a folder as a package, make everything in it private and then just move the public constants into the public folder without changing the code where they are referenced. I will see if configuring privacy for specific constants is therefore a better option for now.

[exterm]

Can you check whether #186 would fix your problem? It looks like there's a chance it will land in packwerk.