Skip to content

Conversation

@martinemde
Copy link
Contributor

@martinemde martinemde commented Jan 7, 2026

Summary

Replace ruby packwerk with pks. Uses the new json output from pks check to show packwerk offenses.

Why

  • pks is much faster
  • we rely on pks everywhere instead of packwerk, making the results more consistent on PRs
  • the packwerk integration required monkey patching, making the implementation brittle

Create a T::Struct that represents violations from pks JSON output with
fields for violation_type, file, line, column, constant_name, and pack
names. Includes methods for JSON deserialization and compatibility with
the BasicReferenceOffense interface used by existing formatters.
Shells out to `pks check --output-format json <files>`, parses the
response, and converts violations to PksOffense objects. Handles pks
binary not found gracefully by raising PksBinaryNotFoundError.
Create JSON fixture files representing pks check output for various
violation scenarios (privacy, dependency, multiple, strict mode).
Extend pks_offense_spec with fixture-based tests to verify parsing.
Add pks_wrapper_spec with test patterns ready for PksWrapper impl.

All 90 existing tests continue to pass.
Replace PackwerkWrapper with PksWrapper to shell out to the pks binary
for packwerk violation detection. This enables JSON-based violation
parsing and prepares for removing the packwerk gem dependency.

Changes:
- Add PksWrapper to shell out to `pks check --output-format json`
- Add Packwerk::ReferenceOffense adapter methods to PksOffense
- Update DangerPackwerk#check to use PksWrapper.get_offenses_for_files
- Update test mocks to stub PksWrapper instead of PackwerkWrapper
Replace all sorbet_double(Packwerk::ReferenceOffense, ...) mocks with
real PksOffense objects in the test suite. This aligns the tests with
the actual behavior of the PksWrapper, which returns PksOffense objects.

Key changes:
- Add build_pks_offense helper to spec_helper.rb
- Replace Packwerk mocks in 'using inputted formatter' tests
- Replace Packwerk mocks in 'using default formatter' tests
- Update expectations where constant.location now points to the
  violation file instead of the constant definition file

This is a known behavioral change: pks output doesn't include the
constant definition location, so URLs and file paths in formatted
messages now reference the violation file.
- Replace T.untyped with PackageAdapter in ConstantAdapter struct
- Reorder adapter classes so PackageAdapter is defined before use
- Convert multi-line if to modifier if in PksWrapper
- Replace semicolons and $? with $CHILD_STATUS in specs
Document the pks (Rust packwerk) integration including:
- Overview and benefits (10-100x faster than Ruby packwerk)
- Installation options (dotslash and cargo install)
- Migration notes (transparent, no code changes required)
- JSON output format specification
- Error handling for missing binary
- Updated Advanced Usage to reference pks instead of bin/packwerk
Verify PksBinaryNotFoundError propagates correctly from PksWrapper
through DangerPackwerk#check with a helpful error message directing
users to install the pks binary.
martinemde and others added 2 commits January 8, 2026 19:09
Remove the adapter classes (PackageAdapter, ConstantAdapter,
ReferenceAdapter, LocationAdapter) from PksOffense that existed solely
to make it compatible with Packwerk::ReferenceOffense interface.

Instead, use PksOffense's direct properties throughout:
- offense.constant_name instead of offense.reference.constant.name
- offense.file instead of offense.reference.relative_path
- offense.line instead of offense.location.line
- offense.defining_pack_name instead of offense.reference.constant.package.name

Update Check::OffensesFormatter interface to accept PksOffense directly
instead of Packwerk::ReferenceOffense, and update DefaultFormatter to
resolve constant locations via Private.constant_resolver.

Closes dp-5q5
@martinemde martinemde changed the title Polish pks integration pks integration Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants