Skip to content

Commit

Permalink
renaming folder_visibility to folder_privacy
Browse files Browse the repository at this point in the history
  • Loading branch information
perryqh committed Jun 27, 2024
1 parent 5d22e47 commit eb8b5ee
Show file tree
Hide file tree
Showing 15 changed files with 105 additions and 38 deletions.
16 changes: 8 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Currently, it ships the following checkers to help improve the boundaries between packages. These checkers are:
- A `privacy` checker that ensures other packages are using your package's public API
- A `visibility` checker that allows packages to be private except to an explicit group of other packages.
- A `folder_visibility` checker that allows packages to their sibling packs and parent pack (to be used in an application that uses folder packs)
- A `folder_privacy` checker that allows packages to their sibling packs and parent pack (to be used in an application that uses folder packs)
- A `layer` (formerly `architecture`) checker that allows packages to specify their "layer" and requires that each layer only communicate with layers below it.

## Installation
Expand All @@ -25,7 +25,7 @@ Alternatively, you can require individual checkers:
require:
- packwerk/privacy/checker
- packwerk/visibility/checker
- packwerk/folder_visibility/checker
- packwerk/folder_privacy/checker
- packwerk/layer/checker
```
Expand All @@ -43,7 +43,7 @@ enforce_privacy: true

Setting `enforce_privacy` to `true` will make all references to private constants in your package a violation.

Setting `enforce_privacy` to `strict` will forbid all references to private constants in your package. **This includes violations that have been added to other packages' `package_todo.yml` files.**
Setting `enforce_privacy` to `strict` will forbid all references to private constants in your package. **This includes violations that have been added to other packages' `package_todo.yml` files.**

Note: You will need to remove all existing privacy violations before setting `enforce_privacy` to `strict`.

Expand Down Expand Up @@ -171,16 +171,16 @@ visible_to:
```

## Folder-Visibility Checker
The folder visibility checker can be used to allow a package to be private to their sibling packs and parent packs and will create todos if used by any other package.
The folder privacy checker can be used to allow a package to be private to their sibling packs and parent packs and will create todos if used by any other package.

To enforce visibility for your package, set `enforce_folder_visibility` to `true` on your pack.
To enforce folder privacy for your package, set `enforce_folder_privacy` to `true` on your pack.

```yaml
# components/merchandising/package.yml
enforce_folder_visibility: true
enforce_folder_privacy: true
```

Here is an example of paths and whether their use of `packs/b/packs/e` is OK or not, assuming that protects itself via `enforce_folder_visibility`
Here is an example of paths and whether their use of `packs/b/packs/e` is OK or not, assuming that protects itself via `enforce_folder_privacy`

```
. OK (parent of parent)
Expand Down Expand Up @@ -226,7 +226,7 @@ The "Layer Checker" was formerly named "Architecture Checker". The associated ke

# replace 'architecture_layers' with 'layers' in packwerk.yml
sed -i '' 's/architecture_layers/layers/g' ./packwerk.yml

# replace 'enforce_architecture' with 'enforce_layers' in package.yml files
`rg -l 'enforce_architecture' -g 'package.yml' | xargs sed -i '' 's,enforce_architecture,enforce_layers,g'`

Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk-extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

require 'packwerk/privacy/checker'
require 'packwerk/visibility/checker'
require 'packwerk/folder_visibility/checker'
require 'packwerk/folder_privacy/checker'
require 'packwerk/layer/checker'

module Packwerk
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
# typed: strict
# frozen_string_literal: true

require 'packwerk/folder_visibility/package'
require 'packwerk/folder_visibility/validator'
require 'packwerk/folder_privacy/package'
require 'packwerk/folder_privacy/validator'

module Packwerk
module FolderVisibility
module FolderPrivacy
class Checker
extend T::Sig
include Packwerk::Checker

VIOLATION_TYPE = T.let('folder_visibility', String)
VIOLATION_TYPE = T.let('folder_privacy', String)

sig { override.returns(String) }
def violation_type
Expand All @@ -26,7 +26,7 @@ def invalid_reference?(reference)
referencing_package = reference.package
referenced_package = reference.constant.package

return false if enforcement_disabled?(Package.from(referenced_package).enforce_folder_visibility)
return false if enforcement_disabled?(Package.from(referenced_package).enforce_folder_privacy)

# the root pack is parent folder of all packs, so we short-circuit this here
referencing_package_is_root_pack = referencing_package.name == '.'
Expand All @@ -48,7 +48,7 @@ def invalid_reference?(reference)
end
def strict_mode_violation?(listed_offense)
publishing_package = listed_offense.reference.constant.package
publishing_package.config['enforce_folder_visibility'] == 'strict'
publishing_package.config['enforce_folder_privacy'] == 'strict'
end

sig do
Expand All @@ -60,7 +60,7 @@ def message(reference)
source_desc = "'#{reference.package}'"

message = <<~MESSAGE
Folder Visibility violation: '#{reference.constant.name}' belongs to '#{reference.constant.package}', which is not visible to #{source_desc} as it is not a sibling pack or parent pack.
Folder Privacy violation: '#{reference.constant.name}' belongs to '#{reference.constant.package}', which is private to #{source_desc} as it is not a sibling pack or parent pack.
Is there a different package to use instead, or should '#{reference.constant.package}' also be visible to #{source_desc}?
#{standard_help_message(reference)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@
# frozen_string_literal: true

module Packwerk
module FolderVisibility
module FolderPrivacy
class Package < T::Struct
extend T::Sig

const :enforce_folder_visibility, T.nilable(T.any(T::Boolean, String))
const :enforce_folder_privacy, T.nilable(T.any(T::Boolean, String))

class << self
extend T::Sig

sig { params(package: ::Packwerk::Package).returns(Package) }
def from(package)
Package.new(
enforce_folder_visibility: package.config['enforce_folder_visibility']
enforce_folder_privacy: package.config['enforce_folder_privacy']
)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# frozen_string_literal: true

module Packwerk
module FolderVisibility
module FolderPrivacy
class Validator
extend T::Sig
include Packwerk::Validator
Expand All @@ -13,14 +13,14 @@ class Validator
def call(package_set, configuration)
results = T.let([], T::Array[Result])

package_manifests_settings_for(configuration, 'enforce_folder_visibility').each do |config, setting|
package_manifests_settings_for(configuration, 'enforce_folder_privacy').each do |config, setting|
next if setting.nil?

next if [TrueClass, FalseClass].include?(setting.class) || setting == 'strict'

results << Result.new(
ok: false,
error_value: "\tInvalid 'enforce_folder_visibility' option: #{setting.inspect} in #{config.inspect}"
error_value: "\tInvalid 'enforce_folder_privacy' option: #{setting.inspect} in #{config.inspect}"
)
end

Expand All @@ -29,7 +29,7 @@ def call(package_set, configuration)

sig { override.returns(T::Array[String]) }
def permitted_keys
%w[enforce_folder_visibility]
%w[enforce_folder_privacy]
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# typed: ignore
# frozen_string_literal: true

class Order
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# typed: ignore
# frozen_string_literal: true

module Sales
class Order
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# typed: ignore
# frozen_string_literal: true

module Sales
module RecordNewOrder
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
metadata:
stewards:
- "@Shopify/sales"
slack_channels:
- "#sales"
8 changes: 8 additions & 0 deletions test/fixtures/with_unrecognized_config/package.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
enforce_folder_privacy: true

enforcement_globs_ignore:
- enforcements:
- privacy
ignores:
- "**/*"
- "!packs/product_services/serv1/**/*"
2 changes: 2 additions & 0 deletions test/fixtures/with_unrecognized_config/packwerk.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
include:
- "**/*.rb"
Empty file.
32 changes: 32 additions & 0 deletions test/integration/extension_with_unrecognized_config_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# typed: true
# frozen_string_literal: true

require 'test_helper'

module Packwerk
module Privacy
class ExtensionWithUnrecognizedConfigTest < Minitest::Test
extend T::Sig

include ApplicationFixtureHelper

setup do
setup_application_fixture
end

teardown do
teardown_application_fixture
end

test 'extension is properly loaded' do
use_template(:with_unrecognized_config)
Packwerk::Checker.all
assert_equal(Packwerk::Checker.all.count, 5)
found_checker = Packwerk::Checker.all.any? do |checker|
T.unsafe(checker).is_a?(Packwerk::Privacy::Checker)
end
assert found_checker
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'test_helper'

module Packwerk
module FolderVisibility
module FolderPrivacy
class CheckerTest < Minitest::Test
extend T::Sig
include FactoryHelper
Expand Down Expand Up @@ -37,21 +37,21 @@ class CheckerTest < Minitest::Test
[true_, 'packs/a', 'packs/a/packs/1', true_, 'access to parent not ok'],
[true_, 'packs/b', 'packs/a/packs/1', true_, 'not siblings or child']
].each do |test|
test "if #{test[1]} has enforce_folder_visibility: #{test[0]} than a reference from #{test[2]} is #{test[3] ? 'A VIOLATION' : 'OK'}" do
test "if #{test[1]} has enforce_folder_privacy: #{test[0]} than a reference from #{test[2]} is #{test[3] ? 'A VIOLATION' : 'OK'}" do
source_package = Packwerk::Package.new(name: test[2])
destination_package = Packwerk::Package.new(name: test[1], config: { 'enforce_folder_visibility' => test[0] })
destination_package = Packwerk::Package.new(name: test[1], config: { 'enforce_folder_privacy' => test[0] })
reference = build_reference(
source_package: source_package,
destination_package: destination_package
)

assert_equal test[3], folder_visibility_checker.invalid_reference?(reference)
assert_equal test[3], folder_privacy_checker.invalid_reference?(reference)
end
end

test 'provides a useful message' do
assert_equal folder_visibility_checker.message(build_reference), <<~MSG.chomp
Folder Visibility violation: '::SomeName' belongs to 'components/destination', which is not visible to 'components/source' as it is not a sibling pack or parent pack.
assert_equal folder_privacy_checker.message(build_reference), <<~MSG.chomp
Folder Privacy violation: '::SomeName' belongs to 'components/destination', which is private to 'components/source' as it is not a sibling pack or parent pack.
Is there a different package to use instead, or should 'components/destination' also be visible to 'components/source'?
Inference details: this is a reference to ::SomeName which seems to be defined in some/location.rb.
Expand All @@ -62,8 +62,8 @@ class CheckerTest < Minitest::Test
private

sig { returns(Checker) }
def folder_visibility_checker
FolderVisibility::Checker.new
def folder_privacy_checker
FolderPrivacy::Checker.new
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
require 'fixtures/skeleton/components/timeline/app/models/private_thing'

module Packwerk
module FolderVisibility
module FolderPrivacy
class ValidatorTest < Minitest::Test
extend T::Sig
include ApplicationFixtureHelper
Expand All @@ -23,26 +23,26 @@ class ValidatorTest < Minitest::Test

# test 'call returns an error for invalid enforce_visibility value' do
# use_template(:minimal)
# merge_into_app_yaml_file('package.yml', { 'enforce_folder_visibility' => 'yes, please.' })
# merge_into_app_yaml_file('package.yml', { 'enforce_folder_privacy' => 'yes, please.' })

# result = validator.call(package_set, config)

# refute result.ok?
# assert_match(/Invalid 'enforce_folder_visibility' option/, result.error_value)
# assert_match(/Invalid 'enforce_folder_privacy' option/, result.error_value)
# end

test 'call returns success when enforce_folder_visibility is set to strict' do
test 'call returns success when enforce_folder_privacy is set to strict' do
use_template(:minimal)
merge_into_app_yaml_file('package.yml', { 'enforce_folder_visibility' => 'strict' })
merge_into_app_yaml_file('package.yml', { 'enforce_folder_privacy' => 'strict' })

result = validator.call(package_set, config)

assert result.ok?
end

sig { returns(Packwerk::FolderVisibility::Validator) }
sig { returns(Packwerk::FolderPrivacy::Validator) }
def validator
@validator ||= Packwerk::FolderVisibility::Validator.new
@validator ||= Packwerk::FolderPrivacy::Validator.new
end
end
end
Expand Down

0 comments on commit eb8b5ee

Please sign in to comment.