Skip to content

Commit

Permalink
added feature to detect stale violations in packwerk via command 'pac…
Browse files Browse the repository at this point in the history
…kwerk detect-stale violations
  • Loading branch information
cindygshopify committed Nov 9, 2020
1 parent cb64151 commit 063d616
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 1 deletion.
30 changes: 30 additions & 0 deletions lib/packwerk/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
require "packwerk/output_styles"
require "packwerk/run_context"
require "packwerk/updating_deprecated_references"
require "packwerk/detect_stale_deprecated_references"

module Packwerk
class Cli
Expand Down Expand Up @@ -41,6 +42,8 @@ def execute_command(args)
generate_configs
when "check"
check(args)
when "detect-stale-violations"
detect_stale_violations(args)
when "update"
update(args)
when "update-deprecations"
Expand Down Expand Up @@ -177,6 +180,33 @@ def check(paths)
all_offenses.empty?
end

def detect_stale_violations(paths)
detect_stale_deprecated_references = ::Packwerk::DetectStaleDeprecatedReferences.new(@configuration.root_path)
@run_context = Packwerk::RunContext.from_configuration(
@configuration,
reference_lister: detect_stale_deprecated_references
)

files = fetch_files_to_process(paths)

@progress_formatter.started(files)

all_offenses = T.let([], T.untyped)
execution_time = Benchmark.realtime do
all_offenses = files.flat_map do |path|
@run_context.file_processor.call(path).tap { |offenses| mark_progress(offenses) }
end
end

status = !detect_stale_deprecated_references.stale_violations?
msg = status ? "No stale violations detected" : "There were stale violations produced, please run packwerk update"

@out.puts
@out.puts(msg)
@progress_formatter.finished(execution_time)
status
end

def fetch_files_to_process(paths)
files = FilesForProcessing.fetch(paths: paths, configuration: @configuration)
abort("No files found or given. "\
Expand Down
19 changes: 18 additions & 1 deletion lib/packwerk/deprecated_references.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
require "packwerk/reference"
require "packwerk/reference_lister"
require "packwerk/violation_type"

module Packwerk
class DeprecatedReferences
extend T::Sig
Expand Down Expand Up @@ -47,6 +46,24 @@ def add_entries(reference, violation_type)
@new_entries[reference.constant.package.name] = package_violations
end

def stale_violations?
prepare_entries_for_dump
deprecated_references.any? do |package, package_violations|
package_violations.any? do |constant_name, entries_for_file|
new_entries_violation_types = @new_entries.dig(package, constant_name, "violations")
return true if new_entries_violation_types.nil?
if new_entries_violation_types.include?(entries_for_file["violations"])
stale_violations =
Array(package_violations.dig(constant_name, "files")) -
Array(@new_entries.dig(package, constant_name, "files"))
stale_violations.present?
else
return true
end
end
end
end

def dump
if @new_entries.empty?
File.delete(@filepath) if File.exist?(@filepath)
Expand Down
12 changes: 12 additions & 0 deletions lib/packwerk/detect_stale_deprecated_references.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# typed: true
# frozen_string_literal: true

require "packwerk/cache_deprecated_references"

module Packwerk
class DetectStaleDeprecatedReferences < CacheDeprecatedReferences
def stale_violations?
@deprecated_references.values.any?(&:stale_violations?)
end
end
end
105 changes: 105 additions & 0 deletions test/unit/deprecated_references_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,111 @@ class DeprecatedReferencesTest < Minitest::Test
)
end

test "#stale_violations? returns true if deprecated references exist but no violations can be found in code" do
deprecated_references = DeprecatedReferences.new(destination_package, "test/fixtures/deprecated_references.yml")
assert deprecated_references.stale_violations?
end

test "#stale_violations? returns false if deprecated references does not exist but violations are found in code" do
deprecated_references = DeprecatedReferences.new(destination_package, "nonexistant_file_path")
reference =
Reference.new(
nil,
"some/path.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Wallet",
"autoload/buyers/wallet.rb",
destination_package,
false
)
)
deprecated_references.add_entries(reference, "dependency")
refute deprecated_references.stale_violations?
end

test "#stale_violations? returns false if deprecated references violation match violations found in code" do
package = Package.new(name: "buyers", config: { "enforce_dependencies" => true })

first_violated_reference =
Reference.new(
nil,
"orders/app/jobs/orders/sweepers/purge_old_document_rows_task.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)
second_violated_reference =
Reference.new(
nil,
"orders/app/models/orders/services/adjustment_engine.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)

deprecated_references = DeprecatedReferences.new(package, "test/fixtures/deprecated_references.yml")
deprecated_references.add_entries(first_violated_reference, "dependency")
deprecated_references.add_entries(second_violated_reference, "dependency")
refute deprecated_references.stale_violations?
end

test "#stale_violations? returns true if dependency deprecated references violation turns into privacy deprecated references violation" do
package = Package.new(name: "buyers", config: { "enforce_dependencies" => true })

first_violated_reference =
Reference.new(
nil,
"orders/app/jobs/orders/sweepers/purge_old_document_rows_task.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)
second_violated_reference =
Reference.new(
nil,
"orders/app/models/orders/services/adjustment_engine.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)

deprecated_references = DeprecatedReferences.new(package, "test/fixtures/deprecated_references.yml")
deprecated_references.add_entries(first_violated_reference, "privacy")
deprecated_references.add_entries(second_violated_reference, "privacy")
assert deprecated_references.stale_violations?
end

test "#stale_violations? returns true if violations in deprecated_references.yml exist but are not found when checking for violations" do
package = Package.new(name: "buyers", config: { "enforce_dependencies" => true })

violated_reference =
Reference.new(
nil,
"orders/app/jobs/orders/sweepers/purge_old_document_rows_task.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)
deprecated_references = DeprecatedReferences.new(package, "test/fixtures/deprecated_references.yml")
deprecated_references.add_entries(violated_reference, "dependency")
assert deprecated_references.stale_violations?
end

test "#listed? returns false if constant is not violated" do
reference =
Reference.new(
Expand Down
47 changes: 47 additions & 0 deletions test/unit/detect_stale_deprecated_references_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# typed: false
# frozen_string_literal: true

require "test_helper"
require "packwerk/detect_stale_deprecated_references"
require "byebug"

module Packwerk
class DetectStaleDeprecatedReferencesTest < Minitest::Test
setup do
package = Package.new(name: "buyers", config: {})
violated_reference =
Reference.new(
nil,
"orders/app/jobs/orders/sweepers/purge_old_document_rows_task.rb",
ConstantDiscovery::ConstantContext.new(
"::Buyers::Document",
"autoload/buyers/document.rb",
package,
false
)
)
deprecated_reference = DeprecatedReferences.new(package, "test/fixtures/deprecated_references.yml")
deprecated_reference.add_entries(violated_reference, "dependency")
@detect_stale_deprecated_references = DetectStaleDeprecatedReferences.new(
".",
{ package.name => deprecated_reference }
)
end

test "#stale_violations? returns true if there are stale violations" do
Packwerk::DeprecatedReferences.any_instance
.expects(:stale_violations?)
.returns(true)

assert @detect_stale_deprecated_references.stale_violations?
end

test "#stale_violations? returns false if no stale violations" do
Packwerk::DeprecatedReferences.any_instance
.expects(:stale_violations?)
.returns(false)

refute @detect_stale_deprecated_references.stale_violations?
end
end
end

0 comments on commit 063d616

Please sign in to comment.