Folder visibility checker added (#38)

* Add nested visibility checker

* FolderVisibility is the new name

* Checker code clarified

Author: shageman

Gemfile.lock

@@ -17,7 +17,7 @@ GIT
 PATH
   remote: .
   specs:
-    packwerk-extensions (0.1.7)
+    packwerk-extensions (0.1.8)
       packwerk (>= 2.2.1)
       railties (>= 6.0.0)
       sorbet-runtime

README.md

@@ -5,7 +5,8 @@
 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.
-- An experimental `architecture` checker that allows packages to specify their "layer" and requires that each layer only communicate with layers below it.
+- A `folder_visibility` checker that allows packages to their sibling packs and parent pack (to be used in an application that uses folder packs)
+- An `architecture` checker that allows packages to specify their "layer" and requires that each layer only communicate with layers below it.
 
 ## Installation
 
@@ -24,6 +25,7 @@ Alternatively, you can require individual checkers:
 require:
   - packwerk/privacy/checker
   - packwerk/visibility/checker
+  - packwerk/folder_visibility/checker
   - packwerk/architecture/checker
 ```
 
@@ -87,6 +89,30 @@ visible_to:
   - components/other_package
 ```
 
+## 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.
+
+To enforce visibility for your package, set `enforce_folder_visibility` to `true` on your pack.
+
+```yaml
+# components/merchandising/package.yml
+enforce_folder_visibility: 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`
+
+```
+.                         OK (parent of parent)
+packs/a                   VIOLATION
+packs/b                   OK (parent)
+packs/b/packs/d           OK (sibling)
+packs/b/packs/e           ENFORCE_NESTED_VISIBILITY: TRUE
+packs/b/packs/e/packs/f   VIOLATION
+packs/b/packs/e/packs/g   VIOLATION
+packs/b/packs/h           OK (sibling)
+packs/c                   VIOLATION
+```
+
 ## Architecture Checker
 The architecture checker can be used to enforce constraints on what can depend on what.
 
@@ -105,3 +131,19 @@ layer: utility
 ```
 
 Now this pack can only depend on other utility packages.
+
+
+## Contributing
+
+Got another checker you would like to add? Add it to this repo!
+
+Please ensure these commands pass for you locally:
+
+```
+bundle
+srb tc
+bin/rubocop
+bin/rake test
+```
+
+Then, submit a PR!

lib/packwerk-extensions.rb

@@ -6,6 +6,7 @@
 
 require 'packwerk/privacy/checker'
 require 'packwerk/visibility/checker'
+require 'packwerk/folder_visibility/checker'
 require 'packwerk/architecture/checker'
 
 module Packwerk

lib/packwerk/folder_visibility/checker.rb

@@ -0,0 +1,93 @@
+# typed: strict
+# frozen_string_literal: true
+
+require 'packwerk/folder_visibility/package'
+require 'packwerk/folder_visibility/validator'
+
+module Packwerk
+  module FolderVisibility
+    class Checker
+      extend T::Sig
+      include Packwerk::Checker
+
+      VIOLATION_TYPE = T.let('folder_visibility', String)
+
+      sig { override.returns(String) }
+      def violation_type
+        VIOLATION_TYPE
+      end
+
+      sig do
+        override
+          .params(reference: Packwerk::Reference)
+          .returns(T::Boolean)
+      end
+      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)
+
+        # the root pack is parent folder of all packs, so we short-circuit this here
+        referencing_package_is_root_pack = referencing_package.name == '.'
+        return false if referencing_package_is_root_pack
+
+        packages_are_sibling_folders = Pathname.new(referenced_package.name).dirname == Pathname.new(referencing_package.name).dirname
+        return false if packages_are_sibling_folders
+
+        referencing_package_is_parent_folder = Pathname.new(referenced_package.name).to_s.start_with?(referencing_package.name)
+        return false if referencing_package_is_parent_folder
+
+        true
+      end
+
+      sig do
+        override
+          .params(listed_offense: Packwerk::ReferenceOffense)
+          .returns(T::Boolean)
+      end
+      def strict_mode_violation?(listed_offense)
+        publishing_package = listed_offense.reference.constant.package
+        publishing_package.config['enforce_folder_visibility'] == 'strict'
+      end
+
+      sig do
+        override
+          .params(reference: Packwerk::Reference)
+          .returns(String)
+      end
+      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.
+          Is there a different package to use instead, or should '#{reference.constant.package}' also be visible to #{source_desc}?
+
+          #{standard_help_message(reference)}
+        MESSAGE
+
+        message.chomp
+      end
+
+      private
+
+      sig do
+        params(visibility_option: T.nilable(T.any(T::Boolean, String)))
+          .returns(T::Boolean)
+      end
+      def enforcement_disabled?(visibility_option)
+        [false, nil].include?(visibility_option)
+      end
+
+      sig { params(reference: Reference).returns(String) }
+      def standard_help_message(reference)
+        standard_message = <<~MESSAGE.chomp
+          Inference details: this is a reference to #{reference.constant.name} which seems to be defined in #{reference.constant.location}.
+          To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations
+        MESSAGE
+
+        standard_message.chomp
+      end
+    end
+  end
+end

lib/packwerk/folder_visibility/package.rb

@@ -0,0 +1,23 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Packwerk
+  module FolderVisibility
+    class Package < T::Struct
+      extend T::Sig
+
+      const :enforce_folder_visibility, 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']
+          )
+        end
+      end
+    end
+  end
+end

lib/packwerk/folder_visibility/validator.rb

@@ -0,0 +1,36 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Packwerk
+  module FolderVisibility
+    class Validator
+      extend T::Sig
+      include Packwerk::Validator
+
+      Result = Packwerk::Validator::Result
+
+      sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Result) }
+      def call(package_set, configuration)
+        results = T.let([], T::Array[Result])
+
+        package_manifests_settings_for(configuration, 'enforce_folder_visibility').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}"
+          )
+        end
+
+        merge_results(results, separator: "\n---\n")
+      end
+
+      sig { override.returns(T::Array[String]) }
+      def permitted_keys
+        %w[enforce_folder_visibility]
+      end
+    end
+  end
+end

packwerk-extensions.gemspec

@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = 'packwerk-extensions'
-  spec.version       = '0.1.7'
+  spec.version       = '0.1.8'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
 

test/integration/extension_test.rb

@@ -21,7 +21,7 @@ class ExtensionTest < Minitest::Test
       test 'extension is properly loaded' do
         use_template(:extended)
         Packwerk::Checker.all
-        assert_equal(Packwerk::Checker.all.count, 4)
+        assert_equal(Packwerk::Checker.all.count, 5)
         found_checker = Packwerk::Checker.all.any? do |checker|
           T.unsafe(checker).is_a?(Packwerk::Privacy::Checker)
         end

test/unit/folder_visibility/checker_test.rb

@@ -0,0 +1,70 @@
+# typed: false
+# frozen_string_literal: true
+
+require 'test_helper'
+
+module Packwerk
+  module FolderVisibility
+    class CheckerTest < Minitest::Test
+      extend T::Sig
+      include FactoryHelper
+      include RailsApplicationFixtureHelper
+
+      setup do
+        setup_application_fixture
+      end
+
+      teardown do
+        teardown_application_fixture
+      end
+
+      true_ = true
+
+      [
+        # enforce, pack,             referencing pack  invalid?   note
+        [false, 'packs/a',         'packs/b/c/d/e/f', false, 'turned off'],
+        [true_, 'packs/a',         'packs/b',         false, 'siblings are ok'],
+        [true_, 'packs/a',         'packs/c',         false, 'siblings are ok'],
+        [true_, 'packs/a/packs/1', 'packs/a/packs/2', false, 'siblings are ok'],
+        [true_, 'packs/a/packs/1', 'packs/a/packs',   false, 'access from parent is ok'],
+        [true_, 'packs/a/packs/1', 'packs/a',         false, 'access from parent of parent is ok'],
+        [true_, 'packs/a/packs/1', 'packs',           false, 'access from parent of parent is ok'],
+        [true_, 'packs/a/packs/1', '.',               false, 'access from root pack is ok'],
+
+        [true_, 'packs/a',         'packs/b/c',       true_, 'not siblings or child'],
+        [true_, 'packs/a',         'packs/b/packs/c', true_, 'not siblings or child'],
+        [true_, 'packs/a/packs/1', 'packs/b/packs/1', true_, 'not siblings or child'],
+        [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
+          source_package = Packwerk::Package.new(name: test[2])
+          destination_package = Packwerk::Package.new(name: test[1], config: { 'enforce_folder_visibility' => test[0] })
+          reference = build_reference(
+            source_package: source_package,
+            destination_package: destination_package
+          )
+
+          assert_equal test[3], folder_visibility_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.
+          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.
+          To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations
+        MSG
+      end
+
+      private
+
+      sig { returns(Checker) }
+      def folder_visibility_checker
+        FolderVisibility::Checker.new
+      end
+    end
+  end
+end

test/unit/folder_visibility/validator_test.rb

@@ -0,0 +1,49 @@
+# typed: true
+# frozen_string_literal: true
+
+require 'test_helper'
+
+# make sure PrivateThing.constantize succeeds to pass the privacy validity check
+require 'fixtures/skeleton/components/timeline/app/models/private_thing'
+
+module Packwerk
+  module FolderVisibility
+    class ValidatorTest < Minitest::Test
+      extend T::Sig
+      include ApplicationFixtureHelper
+      include RailsApplicationFixtureHelper
+
+      setup do
+        setup_application_fixture
+      end
+
+      teardown do
+        teardown_application_fixture
+      end
+
+      # 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.' })
+
+      #   result = validator.call(package_set, config)
+
+      #   refute result.ok?
+      #   assert_match(/Invalid 'enforce_folder_visibility' option/, result.error_value)
+      # end
+
+      test 'call returns success when enforce_folder_visibility is set to strict' do
+        use_template(:minimal)
+        merge_into_app_yaml_file('package.yml', { 'enforce_folder_visibility' => 'strict' })
+
+        result = validator.call(package_set, config)
+
+        assert result.ok?
+      end
+
+      sig { returns(Packwerk::FolderVisibility::Validator) }
+      def validator
+        @validator ||= Packwerk::FolderVisibility::Validator.new
+      end
+    end
+  end
+end