Skip to content

Commit

Permalink
strict_for_new mode: fail check only for violations unlisted in packa…
Browse files Browse the repository at this point in the history
…ge_todo.yml
  • Loading branch information
alxckn committed Aug 8, 2023
1 parent e850241 commit 4bd03b2
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 13 deletions.
4 changes: 2 additions & 2 deletions lib/packwerk/checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/packwerk/offense_collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
18 changes: 14 additions & 4 deletions lib/packwerk/reference_checking/checkers/dependency_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/validators/dependency_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
45 changes: 45 additions & 0 deletions test/unit/packwerk/dependency_checker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/unit/packwerk/dependency_validator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4bd03b2

Please sign in to comment.