alexevanczuk / packs Goto Github PK

View Code? Open in Web Editor NEW

60.0 60.0 7.0 947 KB

A pure Rust implementation of packwerk, a gradual modularization tool for Ruby

License: MIT License

Rust 99.60% Shell 0.40%

packs's People

Contributors

Stargazers

Watchers

Forkers

oleg-vinted szabgab andrewswerlick perryqh mclark stevegeek ethankhall

packs's Issues

Generated todo files reference packwerk instead of pks

Running pks update currently generates package_todo.yml files with a header that references the legacy implementation of packwerk. See:

packs/src/packs/package_todo.rs

Line 222 in 59a13d0

# bin/packwerk update-todo

This line should probably be changed to reference pks update or packs update.

no method named `take_boxed_value`

I wasn't experiencing this directly, but a coworker of mine is seeing it:

error[E0599]: no method named `take_boxed_value` found for struct `PoolValue<Token>` in the current scope
     --> /Users/[redacted]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lib-ruby-parser-4.0.5+ruby-3.1.2/src/parser/parse.rs:15521:37
      |
15521 |             let boxed_token = token.take_boxed_value();
      |                                     ^^^^^^^^^^^^^^^^ help: there is a method with a similar name: `take_value`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `lib-ruby-parser` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: failed to compile `pks v0.1.75`, intermediate artifacts can be found at `/var/folders/pt/bg69lq2j29z8qgc___k_c3sr0000gq/T/cargo-installU7Eu3Z`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

Let me know if this is actually a https://github.com/lib-ruby-parser/lib-ruby-parser issue. Thanks in advance for any time you can spend

`pks check` does not fail on architecture violations

Here's an example:
https://github.com/professor/packs-architecture-violation

pks update successfully generates this package_todo.yml:

packs/domain_pack:
  "::Domain":
    violations:
    - architecture
    - privacy
    files:
    - packs/utility_pack/app/models/utility.rb

However, pks check reports No violations detected! when enforce_architecture: strict

bin/packwerk check and bin/packwerk validate both correctly fail.

`list_packs` lists the wrong packs when using default `package_paths`

To reproduce, rm packwerk.yml, packs list_packs.

LSP Ideas

Make pack names in package.yml files a link
Highlight violations
Status bar packs (like Sorbet)
- Run update
- Jump to package.yml
- Jump to pack public API
add violations for individual constant reference violations
One-click to solve violation should, for example, detect for cycles in advance before recommending adding a dependency. It can say “if you were to add this as a dependency to packs/foo/package.yml, you’ll create cycles in the dependency graph, resulting in X strongly connected components.
See dev/notes.md

Version number is off by one

➜  ~ wget https://github.com/alexevanczuk/packs/releases/download/v0.1.66/packs
[...]
➜  ~ chmod u+x packs
➜  ~ ./packs -V
pks 0.1.67

Should packs have an 'init' command?

Thanks for your work on this, amazing speed difference checking a large project!

Just was creating a new rails app as a reproduction for a separate issue I am having with packs. I thought I could just use packs without packwerk installed, but I dont see an init command to setup configuration for me (eg root pack yml). It would be nice to be able to init the root pack and packwerk.yml directly using packs cause I guess then you wouldn't need packwerk at all?

#<NameError: uninitialized constant Gem::BundlerVersionFinder

We are upgrading our Ruby version from 3.0.6 to 3.1.4 and are seeing a #<NameError: uninitialized constant Gem::BundlerVersionFinder error when running any packs command (see errors below).

Our current packs version is 0.1.75

$ uname -a
Linux f650d5f5cdb4 6.6.12-linuxkit #1 SMP Fri Jan 19 08:53:17 UTC 2024 aarch64 GNU/Linux
$ ruby -v
ruby 3.1.4p223 (2023-03-30 revision 957bb7cb81) [aarch64-linux]
$ bundler version
Bundler version 2.5.5 (2024-01-18 commit 2efa8cec93)
$ packs -e validate
[27, #<Thread:0x0000ffffae143c60 run>, #<NameError: uninitialized constant Gem::BundlerVersionFinder

    Gem::BundlerVersionFinder.prioritize!(matches) if prioritizes_bundler?
       ^^^^^^^^^^^^^^^^^^^^^^>, ["/usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:280:in `matching_specs'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2613:in `block (2 levels) in traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2613:in `block (2 levels) in traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1067:in `block in find_in_unresolved_tree'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1066:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1066:in `find_in_unresolved_tree'", "<internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:114:in `require'", "/usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:280:in `matching_specs'", "/usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:303:in `to_specs'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1451:in `block in activate_dependencies'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1457:in `block in activate_dependencies'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'", "/usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'", "/usr/local/lib/ruby/3.1.0/rubygems.rb:286:in `block in activate_bin_path'", "/usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `synchronize'", "/usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `activate_bin_path'", "/usr/local/bundle/bin/packs:25:in `<main>'"]]
<internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:167:in `ensure in require': CRITICAL: RUBYGEMS_ACTIVATION_MONITOR.owned?: before false -> after true (RuntimeError)
	from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:167:in `require'
	from /usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:280:in `matching_specs'
	from /usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:303:in `to_specs'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1451:in `block in activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1457:in `block in activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:286:in `block in activate_bin_path'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `synchronize'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `activate_bin_path'
	from /usr/local/bundle/bin/packs:25:in `<main>'
/usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:280:in `matching_specs': uninitialized constant Gem::BundlerVersionFinder (NameError)

    Gem::BundlerVersionFinder.prioritize!(matches) if prioritizes_bundler?
       ^^^^^^^^^^^^^^^^^^^^^^
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2613:in `block (2 levels) in traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2613:in `block (2 levels) in traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2602:in `block in traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:2600:in `traverse'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1067:in `block in find_in_unresolved_tree'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1066:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1066:in `find_in_unresolved_tree'
	from <internal:/usr/local/lib/ruby/3.1.0/rubygems/core_ext/kernel_require.rb>:114:in `require'
	from /usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:280:in `matching_specs'
	from /usr/local/lib/ruby/3.1.0/rubygems/dependency.rb:303:in `to_specs'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1451:in `block in activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1457:in `block in activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `each'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1439:in `activate_dependencies'
	from /usr/local/lib/ruby/3.1.0/rubygems/specification.rb:1421:in `activate'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:286:in `block in activate_bin_path'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `synchronize'
	from /usr/local/lib/ruby/3.1.0/rubygems.rb:285:in `activate_bin_path'
	from /usr/local/bundle/bin/packs:25:in `<main>'

stale violation detection not supported

`packs update` adds todos for `strict` violations

"Bug"

Running pks update will add meaningless "todo" entries for strict violations. pks check behaves as expected in that it detects/reports strict errors regardless of what is in todo files.

Proposed Change:

pks update to no longer update todos with strict violations
pks update outputs skipped strict violations so that the user can expect a pks check error.
pks check remains unchanged

How?

Add a new strict_mode bool to the ViolationIdentifier struct. It will be populated in the "checkers" and be used to ignore violations when writing out todos and detecting strict_mode violations.

Instead of a strict_mode bool we could also consider an enum. I'm not sure if the complexity is warranted at this time.

Packwerk

As an aside, packwerk will not update todo files with strict violations and will also not report packwerk check strict violations if they have a corresponding todo entry. Supporting pks check ignoring strict todo violations is outside the scope of this issue.

Feedback

Thoughts? Better approaches?

Todo files not being deleted when no more violations exist

We're finding that when we:

have an existing package_todo.yml for a given pack
solve all of the violations such that no more exist for that pack
run pks update

the todo file is not being deleted as we would expect. Violations do disappear from the todo file in between runs of pks update as long as they are being addressed, but the file continues to exist even after all violations are gone.

We are specifically running with the options pks -e --no-cache update, if that makes a difference. This has been seen for both regular packs and the package_todo.yml in the root directory.

`pack validate` panics when an item of `dependencies` doesn't exist

Consider a repo with a package.yml that has the a typo in a dependency:

❯ pks validate
thread 'main' panicked at src/packs/checker/dependency.rs:67:41:
/Users/josh.nichols/workspace/someapp/package.yml has 'pack/non-existent in its dependencies, but that pack cannot be found. Try `packs list-packs` to debug.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This seems like it is a condition that validate will run in, so probably should be caught and displayed instead of an error with debug info.

`strict` enforcement levels on checkers not yet supported

pks seems to hallucinate when used with experimental parser and constants are defined outside of the application

Examples:

Dependency violation: ::String belongs to ., but packs/domain/internal_users/package.yml does not specify a dependency on .

Architecture violation: ::Rails belongs to . (whose layer is application) cannot be accessed from packs/domain/geography (whose layer is domain)

Is this expected behavior? I mean, it is called "experimental".

Architecture violations without references

If a pack's package.yml contains a dependency to a pack in a higher architecture layer and there are no actual references to pack in the higher architecture error, pks check will not report a violation.

An example

the utility layer is belowdomain
the packs/utility_pack package.yml lists packs/domain_pack as a dependency
there are no actual references to packs/domain_pack in packs/utility_pack
pks check returns “No violations detected!”

bin/packwerk validate reports the architecture violation even if the pack isn’t referenced. pks check does not report a violation.

The source code indicates that “checkers” only deal with references and not declared package dependencies. Is this intended behavior?

pks check-unnecessary-dependencies does show that the dependency is unnecessary. Are we to run pks [validate, check, check-unnecessary-dependencies] as a replacement for packwerk [validate, check]?

VSCode Extension does not print all errors in a file

When using the vscode extension https://github.com/rubyatscale/packwerk-vscode and setting pks check to be the executable, I'm noticing that it's only highlighting one of the errors in this file.

Not sure what changed, but it looks like the output of pks check is different than bin/packwerk and the regex matching in the extension no longer matches.

@perryqh Are you seeing this on your side too? This would be nice to fix since pks check is a lot snappier in vscode than bin/packwerk check.

something strange with load paths and active admin

trying to run pks on the app I'm working on right now, it complains about path collisions between packs/*/app/models/something.rb and packs/*/app/admin/something.rb. But app/admin isn't actually autoloaded. This is apparently the default pattern for ActiveAdmin.

So it seems that pks is somehow guessing the autoload paths incorrectly?