diff --git a/lib/packwerk/checker.rb b/lib/packwerk/checker.rb index 9d94da608..ed5412724 100644 --- a/lib/packwerk/checker.rb +++ b/lib/packwerk/checker.rb @@ -51,8 +51,8 @@ def checker_by_violation_type(name) sig { abstract.returns(String) } def violation_type; end - sig { abstract.params(listed_offense: ReferenceOffense).returns(T::Boolean) } - def strict_mode_violation?(listed_offense); end + sig { abstract.params(offense: ReferenceOffense, already_listed: T::Boolean).returns(T::Boolean) } + def strict_mode_violation?(offense, already_listed:); end sig { abstract.params(reference: Reference).returns(T::Boolean) } def invalid_reference?(reference); end diff --git a/lib/packwerk/offense_collection.rb b/lib/packwerk/offense_collection.rb index 1e6d63823..647eeb736 100644 --- a/lib/packwerk/offense_collection.rb +++ b/lib/packwerk/offense_collection.rb @@ -59,7 +59,7 @@ def add_offense(offense) new_violations << offense unless already_listed - if strict_mode_violation?(offense) + if strict_mode_violation?(offense, already_listed) add_to_package_todo(offense) if already_listed strict_mode_violations << offense else @@ -104,10 +104,10 @@ def add_to_package_todo(offense) offense.violation_type) end - sig { params(offense: ReferenceOffense).returns(T::Boolean) } - def strict_mode_violation?(offense) + sig { params(offense: ReferenceOffense, already_listed: T::Boolean).returns(T::Boolean) } + def strict_mode_violation?(offense, already_listed) checker = Checker.find(offense.violation_type) - checker.strict_mode_violation?(offense) + checker.strict_mode_violation?(offense, already_listed: already_listed) end sig { params(package_set: Packwerk::PackageSet).void } diff --git a/lib/packwerk/package.rb b/lib/packwerk/package.rb index 95988aa57..f29fba9aa 100644 --- a/lib/packwerk/package.rb +++ b/lib/packwerk/package.rb @@ -30,7 +30,7 @@ def initialize(name:, config: nil) sig { returns(T::Boolean) } def enforce_dependencies? - [true, "strict"].include?(@config["enforce_dependencies"]) + [true, "strict", "strict_for_new"].include?(@config["enforce_dependencies"]) end sig { params(package: Package).returns(T::Boolean) } diff --git a/lib/packwerk/reference_checking/checkers/dependency_checker.rb b/lib/packwerk/reference_checking/checkers/dependency_checker.rb index f57c25e35..ba1015768 100644 --- a/lib/packwerk/reference_checking/checkers/dependency_checker.rb +++ b/lib/packwerk/reference_checking/checkers/dependency_checker.rb @@ -47,10 +47,20 @@ def message(reference) EOS end - sig { override.params(listed_offense: ReferenceOffense).returns(T::Boolean) } - def strict_mode_violation?(listed_offense) - referencing_package = listed_offense.reference.package - referencing_package.config["enforce_dependencies"] == "strict" + sig { override.params(offense: ReferenceOffense, already_listed: T::Boolean).returns(T::Boolean) } + def strict_mode_violation?(offense, already_listed:) + referencing_package = offense.reference.package + + case referencing_package.config["enforce_dependencies"] + when nil, false, true + false + when "strict" + true + when "strict_for_new" + !already_listed + else + raise "Unknown value for 'enforce_dependencies': #{referencing_package.config["enforce_dependencies"]}" + end end private diff --git a/lib/packwerk/validators/dependency_validator.rb b/lib/packwerk/validators/dependency_validator.rb index 5430f67bc..65f8397d1 100644 --- a/lib/packwerk/validators/dependency_validator.rb +++ b/lib/packwerk/validators/dependency_validator.rb @@ -34,7 +34,7 @@ def permitted_keys def check_package_manifest_syntax(configuration) errors = [] - valid_settings = [true, false, "strict"] + valid_settings = [true, false, "strict", "strict_for_new"] package_manifests_settings_for(configuration, "enforce_dependencies").each do |config, setting| next if setting.nil? diff --git a/test/unit/packwerk/dependency_checker_test.rb b/test/unit/packwerk/dependency_checker_test.rb index 37080a922..309c40306 100644 --- a/test/unit/packwerk/dependency_checker_test.rb +++ b/test/unit/packwerk/dependency_checker_test.rb @@ -34,6 +34,51 @@ class DependencyCheckerTest < Minitest::Test refute checker.invalid_reference?(reference) end + test "does not report any violation as strict in non strict mode" do + source_package = Package.new(name: "components/sales", config: { "enforce_dependencies" => true }) + checker = dependency_checker + + offense = Packwerk::ReferenceOffense.new( + reference: build_reference(source_package: source_package), + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE, + message: "some message" + ) + + assert checker.invalid_reference?(offense.reference) + refute checker.strict_mode_violation?(offense, already_listed: true) + refute checker.strict_mode_violation?(offense, already_listed: false) + end + + test "reports any violation as strict in strict mode" do + source_package = Package.new(name: "components/sales", config: { "enforce_dependencies" => "strict" }) + checker = dependency_checker + + offense = Packwerk::ReferenceOffense.new( + reference: build_reference(source_package: source_package), + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE, + message: "some message" + ) + + assert checker.invalid_reference?(offense.reference) + assert checker.strict_mode_violation?(offense, already_listed: true) + assert checker.strict_mode_violation?(offense, already_listed: false) + end + + test "only reports unlisted violations as strict in strict_for_new mode" do + source_package = Package.new(name: "components/sales", config: { "enforce_dependencies" => "strict_for_new" }) + checker = dependency_checker + + offense = Packwerk::ReferenceOffense.new( + reference: build_reference(source_package: source_package), + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE, + message: "some message" + ) + + assert checker.invalid_reference?(offense.reference) + refute checker.strict_mode_violation?(offense, already_listed: true) + assert checker.strict_mode_violation?(offense, already_listed: false) + end + private def dependency_checker diff --git a/test/unit/packwerk/dependency_validator_test.rb b/test/unit/packwerk/dependency_validator_test.rb index 07d000813..f3306f73a 100644 --- a/test/unit/packwerk/dependency_validator_test.rb +++ b/test/unit/packwerk/dependency_validator_test.rb @@ -67,6 +67,14 @@ class DependencyValidatorTest < Minitest::Test assert result.ok? end + test "returns success when enforce_dependencies is set to strict_for_new in the package.yml file" do + use_template(:minimal) + merge_into_app_yaml_file("components/sales/package.yml", { "enforce_dependencies" => "strict_for_new" }) + + result = dependency_validator.call(package_set, config) + assert result.ok? + end + private def dependency_validator diff --git a/test/unit/packwerk/reference_checking/reference_checker_test.rb b/test/unit/packwerk/reference_checking/reference_checker_test.rb index da0102880..2bc7417d0 100644 --- a/test/unit/packwerk/reference_checking/reference_checker_test.rb +++ b/test/unit/packwerk/reference_checking/reference_checker_test.rb @@ -29,7 +29,7 @@ def message(_reference) @message end - def strict_mode_violation?(_listed_offense) + def strict_mode_violation?(_offense, already_listed:) false end end