From a503e0520899ba68c939ecf78de1da4a55128ff5 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Wed, 19 Aug 2026 11:19:49 -0700 Subject: [PATCH] Remove stray dbg! from the privacy checker `privacy.rs` carried a live `dbg!` on the `private_constants` branch, so any pack configured with `private_constants` emitted two lines to stderr per checked reference, in released builds as well as debug. It also leaked internal file and line numbers into user-facing output. No fixture configured `private_constants`, so nothing exercised the branch. Add `tests/fixtures/privacy_violations_with_private_constants`, covering all three combinations of the two flags the `dbg!` was printing: ::Bar listed in private_constants -> violation ::Bar::Inner inside the private namespace -> violation ::SomeConcern absent from the list -> no violation The last case is the one that had no coverage: a non-empty `private_constants` list narrows privacy to that list, leaving everything else in the pack public. `test_check_with_private_constants` asserts those outcomes and that stderr is empty, so the `dbg!` cannot come back unnoticed. Fixes #49 --- src/packs/checker/privacy.rs | 1 - tests/check_test.rs | 33 +++++++++++++++++++ .../package.yml | 0 .../packs/bar/app/services/bar.rb | 4 +++ .../packs/bar/app/services/bar/inner.rb | 6 ++++ .../packs/bar/app/services/some_concern.rb | 4 +++ .../packs/bar/package.yml | 3 ++ .../packs/foo/app/services/foo.rb | 13 ++++++++ .../packs/foo/package.yml | 0 .../packwerk.yml | 2 ++ 10 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/privacy_violations_with_private_constants/package.yml create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar.rb create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar/inner.rb create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/some_concern.rb create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/bar/package.yml create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/foo/app/services/foo.rb create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packs/foo/package.yml create mode 100644 tests/fixtures/privacy_violations_with_private_constants/packwerk.yml diff --git a/src/packs/checker/privacy.rs b/src/packs/checker/privacy.rs index b749f1b..c9eaf2d 100644 --- a/src/packs/checker/privacy.rs +++ b/src/packs/checker/privacy.rs @@ -61,7 +61,6 @@ impl CheckerInterface for Checker { &format!("{}::", private_constant); reference.constant_name.starts_with(namespaced_constant) }); - dbg!(constant_is_private, constant_is_in_private_namespace); if !constant_is_private && !constant_is_in_private_namespace { return Ok(None); } diff --git a/tests/check_test.rs b/tests/check_test.rs index 4f0d043..66ad17e 100644 --- a/tests/check_test.rs +++ b/tests/check_test.rs @@ -48,6 +48,39 @@ fn test_check_with_privacy_dependency_error_template_overrides( common::teardown(); Ok(()) } +#[test] +fn test_check_with_private_constants() -> Result<(), Box> { + let output = cargo_bin_cmd!("pks") + .arg("--project-root") + .arg("tests/fixtures/privacy_violations_with_private_constants") + .arg("check") + .assert() + .code(1) + .get_output() + .clone(); + + let stripped_stdout = stripped_output(output.stdout); + + // A non-empty `private_constants` list narrows privacy to that list: the listed + // constant and its namespace are private, everything else in the pack is public. + assert!(stripped_stdout.contains("2 violation(s) detected:")); + assert!(stripped_stdout.contains("packs/foo/app/services/foo.rb:3:4\nPrivacy violation: `::Bar` is private to `packs/bar`, but referenced from `packs/foo`")); + assert!(stripped_stdout.contains("packs/foo/app/services/foo.rb:7:4\nPrivacy violation: `::Bar::Inner` is private to `packs/bar`, but referenced from `packs/foo`")); + assert!(!stripped_stdout.contains("::SomeConcern")); + + // Regression guard: this branch once carried a live `dbg!`, which wrote a pair of + // lines to stderr for every reference checked against `private_constants`. + let stripped_stderr = stripped_output(output.stderr); + assert_eq!( + stripped_stderr, "", + "expected no stderr output, got:\n{}", + stripped_stderr + ); + + common::teardown(); + Ok(()) +} + #[test] fn test_check() -> Result<(), Box> { let output = cargo_bin_cmd!("pks") diff --git a/tests/fixtures/privacy_violations_with_private_constants/package.yml b/tests/fixtures/privacy_violations_with_private_constants/package.yml new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar.rb b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar.rb new file mode 100644 index 0000000..842e51b --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar.rb @@ -0,0 +1,4 @@ +# ::Bar is listed in private_constants, so referencing it is a violation. +module Bar + def bar; end +end diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar/inner.rb b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar/inner.rb new file mode 100644 index 0000000..27d07b2 --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/bar/inner.rb @@ -0,0 +1,6 @@ +# ::Bar::Inner is not listed itself, but sits inside the private ::Bar namespace, +# so referencing it is also a violation. +module Bar + module Inner + end +end diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/some_concern.rb b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/some_concern.rb new file mode 100644 index 0000000..484637a --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/app/services/some_concern.rb @@ -0,0 +1,4 @@ +# ::SomeConcern is defined in packs/bar but is absent from private_constants. +# A non-empty private_constants list makes everything outside it public, so +# referencing this is NOT a violation. +module SomeConcern; end diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/bar/package.yml b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/package.yml new file mode 100644 index 0000000..77e963a --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packs/bar/package.yml @@ -0,0 +1,3 @@ +enforce_privacy: true +private_constants: +- "::Bar" diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/foo/app/services/foo.rb b/tests/fixtures/privacy_violations_with_private_constants/packs/foo/app/services/foo.rb new file mode 100644 index 0000000..b801323 --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packs/foo/app/services/foo.rb @@ -0,0 +1,13 @@ +module Foo + def references_a_private_constant + ::Bar + end + + def references_a_constant_in_the_private_namespace + ::Bar::Inner + end + + def references_a_constant_left_public_by_omission + ::SomeConcern + end +end diff --git a/tests/fixtures/privacy_violations_with_private_constants/packs/foo/package.yml b/tests/fixtures/privacy_violations_with_private_constants/packs/foo/package.yml new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/privacy_violations_with_private_constants/packwerk.yml b/tests/fixtures/privacy_violations_with_private_constants/packwerk.yml new file mode 100644 index 0000000..7a85b9b --- /dev/null +++ b/tests/fixtures/privacy_violations_with_private_constants/packwerk.yml @@ -0,0 +1,2 @@ +# Whether or not you want the cache enabled (disabled by default) +cache: false