From 24e92478c13b38295b1e68bb6969fc76451eda6a Mon Sep 17 00:00:00 2001 From: TristanKruse Date: Thu, 15 Oct 2026 14:54:54 +0200 Subject: [PATCH 1/4] Resolve static constant dependencies and local-string imports Add opt-in Prism constant indexing, conservative local-string flow tracking, and source evidence merged through graph reports. Exercise both issue reproducers through public APIs and installed gems, document bounded lookup behavior, and profile cold/warm extraction. Addresses #54 and #55. --- .github/workflows/ci.yml | 10 + CHANGELOG.md | 4 + README.md | 81 ++++++- benchmark/README.md | 32 +++ benchmark/extraction.rb | 12 +- lib/archunit.rb | 1 + .../common/extraction/dependency_evidence.rb | 44 ++++ lib/archunit/common/extraction/edge.rb | 9 +- lib/archunit/common/extraction/import_kind.rb | 3 +- .../common/fluentapi/check_options.rb | 17 +- .../extraction/conservative_local_flow.rb | 100 ++++++++ lib/archunit/extraction/constant_index.rb | 110 +++++++++ lib/archunit/extraction/constant_visitor.rb | 124 ++++++++++ .../extraction/extract_dependencies.rb | 42 +++- lib/archunit/extraction/extract_graph.rb | 9 +- lib/archunit/extraction/extract_imports.rb | 22 +- lib/archunit/extraction/extraction_profile.rb | 4 + lib/archunit/extraction/import_visitor.rb | 47 +++- .../extraction/local_string_tracker.rb | 80 ++++++ lib/archunit/extraction/resolved_import.rb | 7 +- .../graph/projection/aggregate_edges.rb | 6 +- .../graph/projection/graph_report_edge.rb | 13 +- lib/archunit/graph/rendering/json_renderer.rb | 4 +- .../metrics/fluentapi/metric_selection.rb | 5 +- scripts/check_dependency_analysis.rb | 34 +++ .../extraction/dependency_evidence_spec.rb | 43 ++++ spec/common/extraction/import_kind_spec.rb | 3 +- spec/common/fluentapi/check_options_spec.rb | 11 +- spec/extraction/constant_dependencies_spec.rb | 228 ++++++++++++++++++ .../dependency_analysis_integration_spec.rb | 116 +++++++++ spec/extraction/extract_dependencies_spec.rb | 10 +- spec/extraction/extract_graph_spec.rb | 4 +- spec/extraction/extract_imports_spec.rb | 189 ++++++++++++++- .../extraction_profiling_integration_spec.rb | 10 + spec/extraction/load_paths_spec.rb | 2 +- 35 files changed, 1374 insertions(+), 62 deletions(-) create mode 100644 lib/archunit/common/extraction/dependency_evidence.rb create mode 100644 lib/archunit/extraction/conservative_local_flow.rb create mode 100644 lib/archunit/extraction/constant_index.rb create mode 100644 lib/archunit/extraction/constant_visitor.rb create mode 100644 lib/archunit/extraction/local_string_tracker.rb create mode 100644 scripts/check_dependency_analysis.rb create mode 100644 spec/common/extraction/dependency_evidence_spec.rb create mode 100644 spec/extraction/constant_dependencies_spec.rb create mode 100644 spec/extraction/dependency_analysis_integration_spec.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 191511a..6a0feae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,6 +41,9 @@ jobs: - name: Test run: bundle exec rspec + - name: Exercise dependency-analysis reproducers + run: bundle exec ruby scripts/check_dependency_analysis.rb + quality: name: Quality and package — Ruby 4.0 on Ubuntu runs-on: ubuntu-latest @@ -73,6 +76,12 @@ jobs: --files 200 --gems 4 --imports-per-file 3 --max-cold-seconds 30 --max-warm-seconds 2 + - name: Constant-index performance smoke benchmark + run: >- + bundle exec ruby benchmark/extraction.rb --constants + --files 200 --gems 4 --imports-per-file 3 + --max-cold-seconds 30 --max-warm-seconds 2 + - name: Build documentation run: bundle exec rake docs @@ -123,6 +132,7 @@ jobs: GEM_PATH: ${{ runner.temp }}/archunit-gem-home run: | gem install pkg/archunit.gem --no-document + ruby scripts/check_dependency_analysis.rb ruby -e " require 'json' require 'stringio' diff --git a/CHANGELOG.md b/CHANGELOG.md index 957d1ee..cf1f0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +- Add opt-in project constant-reference dependencies with lexical lookup and conservative ambiguity handling. +- Infer import paths from safely known local literal strings without evaluating application code. +- Preserve dependency source evidence through graph merging and JSON reporting. +- Support check options on metric measurements and add installed-gem extraction regression checks. - Discover adjacent `lib` directories in multi-gemspec repositories without evaluating gemspecs. - Support validated, project-local custom load paths through `CheckOptions`. - Add cold/warm extraction profiling, a reproducible benchmark, and resolution caching. diff --git a/README.md b/README.md index a7a220e..d986aac 100644 --- a/README.md +++ b/README.md @@ -361,8 +361,76 @@ require 'legacy/client' # archunit: ignore legacy/client require 'experimental/plugin' ``` -Dynamic imports such as `require dependency_name` or `require "plugins/#{name}"` are omitted rather -than guessed because resolving them would require executing application code. +Preceding local assignments to literal strings are also recognized for all four import forms: + +```ruby +path = '../retrieval/store' +require_relative path +``` + +Inference follows assignment order within straight-line lexical scopes. Reassignment replaces the +value; aliases, mutation, unknown calls receiving the value, reflection, and unsupported control +flow invalidate it. Values are not carried across branches, loops, exception handlers, or captured +blocks. Once a closure or reflective binding can access an enclosing scope, local inference remains +disabled for that scope. Independent literal assignments inside a method or block can still resolve. Runtime values +such as `ENV.fetch('PLUGIN_PATH')`, interpolation, and method results remain unresolved. No source, +Gemfile, gemspec, initializer, or `const_missing` hook is executed. + +### Constant References (Opt-in) + +For autoloaded applications, enable dependencies from known project constants alongside imports: + +```ruby +options = ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) + +# Detects Retrieval::Store.new even without require_relative. +rule = ArchUnit.project_files + .in_path('app/api/**') + .should_not.depend_on_files.in_path('app/retrieval/**') +violations = rule.check(options) + +report = ArchUnit.project_graph.with_check_options(options) +puts report.to_json +puts ArchUnit.project_slices.defined_by('app/(**)/').to_plantuml(options) +values = ArchUnit.metrics.distance.instability.measure(options) +``` + +The default remains `dependency_analysis: :imports`. Expanded mode indexes declarations from the +same included source files, then resolves references to definition files. Files remain graph nodes. +It supports lexical nesting, compact namespaces, root-qualified names, and explicit superclass, +`include`, `prepend`, and `extend` references. Nested `module A; module B` includes `A` in lexical +lookup; top-level `module A::B` does not. A nearer namespace shadows a root namespace even when its +requested child is missing. + +References with multiple definition files (reopened classes/modules), missing definitions, and +dynamic definitions are omitted, never assigned to an arbitrary file. `Alias = Original` defines +`Alias` in its own file and records the reference to `Original`; lookup through aliases such as +`Alias::Child` is not followed. Inherited constant members and singleton-class lexical lookup are +not modeled. Unqualified root fallback is conservatively suppressed in scopes with an explicit +superclass, mixins, or dynamic constant operations. Use explicit `::Namespace::Constant` references +or import rules where those limits matter. Static analysis does not establish runtime load order +or whether a conditional use executes. Qualified declaration owners are stabilized before lookup; +if they do not stabilize within eight index passes, the project contributes imports only and its +constant uses are counted as unresolved. + +An import and a constant use between the same files produce one logical edge. Its `import_kinds` +includes `:constant_reference`; `edge.evidence` and JSON graph exports retain the referenced name, +source span, and definition line for constant uses. Import evidence retains its call span. CSV, +HTML, and diagram reports use the same merged graph; CSV/HTML distinguish dependency kinds. +`# archunit: ignore` continues to suppress imports only, not constant references. + +For diagnostics, pass an `ArchUnit::Extraction::ExtractionProfile` to +`ArchUnit::Extraction.extract_graph(root, options: options, profile: profile)`. Its counters include +`unresolved_imports`, `unresolved_constant_references`, and `ambiguous_constant_references`; unknown +third-party constants do not become architecture violations. These counts describe a cold analysis; +a cache hit does not rerun diagnostics. Malformed source is skipped, as in import-only mode. + +Expanded analysis uses a bounded Prism index and adds no runtime dependency. The design spike +considered [RubyDEX](https://github.com/Shopify/rubydex), whose semantic index is broader but adds a +Rust/native extension and platform packaging requirements. Reusing our existing Prism parser keeps +the Ruby 3.3/3.4/4.0 Windows/Linux support contract; each included source is parsed once per graph +build. A later semantic backend can broaden the documented lookup subset. Lookup fixtures follow +the [Ruby constant-scope documentation](https://docs.ruby-lang.org/en/master/syntax/modules_and_classes_rdoc.html#label-Constants). Repositories containing multiple gems are discovered statically: every `*.gemspec` beneath the project root contributes its adjacent existing `lib` directory to feature resolution. Gemspecs are @@ -379,8 +447,9 @@ violations = rule.check(options) report = ArchUnit.project_graph.with_check_options(options) ``` -Load-path choices affect graph caching. Equivalent normalized choices reuse a cached graph, while a -different set builds a separate graph. Use `clear_cache: true` after changing files or gemspec +Load-path choices and dependency-analysis mode affect graph caching. Equivalent normalized choices +reuse a cached graph, while a different mode or load-path set builds a separate graph. Use +`clear_cache: true` after changing files, constant definitions, or gemspec layout within one process. ## 🐲 Example Repository @@ -426,8 +495,8 @@ The implementation conventions and intended dependency directions live in [`AGEN ## 📅 Plans and Current Limitations -- Ruby constants are not modeled as a separate graph. Files are the primary dependency vocabulary. -- Dynamic `require`, `autoload`, and `load` arguments cannot be resolved statically. +- Files are the primary vocabulary; opt-in constant analysis has the lookup limits described above. +- Runtime-derived import paths cannot be resolved; safely known local literal strings are supported. - PlantUML support is a deliberately small component-diagram subset, not a complete UML parser. - The API is still pre-release and may change before the first stable gem version. diff --git a/benchmark/README.md b/benchmark/README.md index 80d1863..490e3bc 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -10,6 +10,16 @@ Run the default 1,000-file corpus: bundle exec ruby benchmark/extraction.rb ``` +The corpus includes a declared class and a cross-file constant use per generated source. Compare +import-only extraction against expanded indexing on identical source with: + +```bash +bundle exec ruby benchmark/extraction.rb --constants +``` + +Both runs report cold/warm timing and memory. Expanded runs also report `constant_indexing`, +unresolved/ambiguous-reference counters, and the same-process graph cache reuse result. + Use a smaller CI smoke corpus with deliberately generous, non-flaky limits: ```bash @@ -29,6 +39,28 @@ output includes Ruby heap growth on every platform and process peak RSS where `/ regression ceilings; performance decisions should use several local runs of the default corpus or a non-blocking real-world canary. +## Constant-analysis measurement + +On 2026-10-15, Ruby 4.0.6 on Windows x64, three fresh-process runs per mode on the updated +1,000-file / 10-gem corpus produced these medians: + +| Measurement | Imports | Imports and constants | +| --- | ---: | ---: | +| Cold graph build | 1.959 s | 2.019 s | +| Warm graph lookup | 3.23 ms | 2.62 ms | +| Constant indexing/resolution | — | 42.8 ms | +| Merged graph edges | 4,001 | 4,001 | + +All runs reused the identical cached graph on their warm lookup. The corpus references the same +next file by import and constant, so expanded mode merges 5,001 raw edges into 4,001 edges while +retaining both evidence forms. Every source is parsed once per cold graph. The roughly 3% median +cold-time increase is a local observation, not a portable overhead guarantee. + +Heap-growth samples ranged from 4.18–5.55 MB for imports and 5.51–14.19 MB for expanded analysis; +these pre-GC snapshots are sensitive to collection timing and do not establish retained or peak +memory. Windows does not expose the Linux peak-RSS counter. ASTs and indexing structures are +temporary; the cached graph retains immutable edges and evidence only. + ## Reference baseline and target On 2026-08-14, Ruby 4.0.6 on the Windows development machine produced the following default-corpus diff --git a/benchmark/extraction.rb b/benchmark/extraction.rb index 9f40c8e..d8f7d5b 100644 --- a/benchmark/extraction.rb +++ b/benchmark/extraction.rb @@ -13,6 +13,7 @@ module ExtractionBenchmark files: 1_000, gems: 10, imports_per_file: 3, + dependency_analysis: :imports, max_cold_seconds: nil, max_warm_seconds: nil, output: nil @@ -59,6 +60,7 @@ def write_source(index) dependencies = dependency_features(index) contents = dependencies.map { |feature| "require '#{feature}'" }.join("\n") contents = '# isolated benchmark file' if contents.empty? + contents += "\nclass BenchFile#{index}; BenchFile#{(index + 1) % file_count}.new; end" write(source_path(index), "#{contents}\n") end @@ -127,13 +129,16 @@ def collect_values def extract_profiled profile = ArchUnit::Extraction::ExtractionProfile.new - graph = ArchUnit::Extraction.extract_graph(@root, profile:) + options = ArchUnit::CheckOptions.new( + dependency_analysis: @configuration.fetch(:dependency_analysis) + ) + graph = ArchUnit::Extraction.extract_graph(@root, profile:, options:) [graph, profile] end def benchmark_result(values) { - corpus: @configuration.slice(:files, :gems, :imports_per_file), + corpus: @configuration.slice(:files, :gems, :imports_per_file, :dependency_analysis), graph_edges: values.cold_graph.length, graph_cache_reused: values.cold_graph.equal?(values.warm_graph), cold: values.cold_profile.to_h.merge(memory_delta(values.before, values.after_cold)), @@ -195,6 +200,9 @@ def option_parser(options) configure_corpus_options(parser, options) configure_threshold_options(parser, options) parser.on('--output PATH', String) { |value| options[:output] = value } + parser.on('--constants', 'Include project constant references') do + options[:dependency_analysis] = :imports_and_constants + end end end diff --git a/lib/archunit.rb b/lib/archunit.rb index cbed9cf..9fe37e2 100644 --- a/lib/archunit.rb +++ b/lib/archunit.rb @@ -53,6 +53,7 @@ module ArchUnit Checkable = Common::FluentApi::Checkable ImportKind = Common::Extraction::ImportKind Edge = Common::Extraction::Edge + DependencyEvidence = Common::Extraction::DependencyEvidence Graph = Common::Extraction::Graph MappedEdge = Common::Projection::MappedEdge ProjectedEdge = Common::Projection::ProjectedEdge diff --git a/lib/archunit/common/extraction/dependency_evidence.rb b/lib/archunit/common/extraction/dependency_evidence.rb new file mode 100644 index 0000000..c0be068 --- /dev/null +++ b/lib/archunit/common/extraction/dependency_evidence.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require_relative 'import_kind' + +module ArchUnit + module Common + module Extraction + # Source-level support for a file dependency, without implying runtime execution. + DependencyEvidence = Data.define( + :source, :kind, :name, :line_number, :end_line, :target_line_number + ) do + # rubocop:disable Metrics/ParameterLists -- Keywords mirror this immutable source span. + def initialize(source:, kind:, name:, line_number:, end_line: line_number, + target_line_number: nil) + validate_identity(source, kind, name) + validate_lines(line_number, end_line, target_line_number) + source = source.tr('\\', '/').freeze + name = name.dup.freeze + super + end + # rubocop:enable Metrics/ParameterLists + + private + + def validate_identity(source, kind, name) + return if source.is_a?(String) && name.is_a?(String) && ImportKind.valid?(kind) + + raise ArgumentError, 'evidence requires a source, name, and valid dependency kind' + end + + def validate_lines(first, last, target) + valid = positive_line?(first) && positive_line?(last) && last >= first + return if valid && (target.nil? || positive_line?(target)) + + raise ArgumentError, 'evidence requires valid source line numbers' + end + + def positive_line?(value) + value.is_a?(Integer) && value.positive? + end + end + end + end +end diff --git a/lib/archunit/common/extraction/edge.rb b/lib/archunit/common/extraction/edge.rb index 9741217..c6c8f78 100644 --- a/lib/archunit/common/extraction/edge.rb +++ b/lib/archunit/common/extraction/edge.rb @@ -1,16 +1,21 @@ # frozen_string_literal: true require_relative 'import_kind' +require_relative 'dependency_evidence' module ArchUnit module Common module Extraction - Edge = Data.define(:source, :target, :external, :import_kinds) do - def initialize(source:, target:, external:, import_kinds: []) + Edge = Data.define(:source, :target, :external, :import_kinds, :evidence) do + def initialize(source:, target:, external:, import_kinds: [], evidence: []) source = immutable_identifier(source, :source) target = immutable_identifier(target, :target) validate_external(external) import_kinds = immutable_import_kinds(import_kinds) + evidence = Array(evidence).uniq.freeze + unless evidence.all?(DependencyEvidence) + raise ArgumentError, 'evidence must contain DependencyEvidence values' + end super end diff --git a/lib/archunit/common/extraction/import_kind.rb b/lib/archunit/common/extraction/import_kind.rb index 8fe420d..32d7430 100644 --- a/lib/archunit/common/extraction/import_kind.rb +++ b/lib/archunit/common/extraction/import_kind.rb @@ -9,8 +9,9 @@ module ImportKind REQUIRE_RELATIVE = :require_relative AUTOLOAD = :autoload LOAD = :load + CONSTANT_REFERENCE = :constant_reference - ALL = [REQUIRE, REQUIRE_RELATIVE, AUTOLOAD, LOAD].freeze + ALL = [REQUIRE, REQUIRE_RELATIVE, AUTOLOAD, LOAD, CONSTANT_REFERENCE].freeze def self.valid?(value) ALL.include?(value) diff --git a/lib/archunit/common/fluentapi/check_options.rb b/lib/archunit/common/fluentapi/check_options.rb index c0a91eb..59f1c56 100644 --- a/lib/archunit/common/fluentapi/check_options.rb +++ b/lib/archunit/common/fluentapi/check_options.rb @@ -6,7 +6,9 @@ module ArchUnit module Common module FluentApi # Immutable options shared by every terminal rule check. - CheckOptions = Data.define(:allow_empty_tests, :logging, :clear_cache, :load_paths) do + CheckOptions = Data.define( + :allow_empty_tests, :logging, :clear_cache, :load_paths, :dependency_analysis + ) do def self.resolve(value) return new if value.nil? return value if value.is_a?(self) @@ -14,11 +16,18 @@ def self.resolve(value) raise ArgumentError, 'options must be a CheckOptions value or nil' end - def initialize(allow_empty_tests: false, logging: nil, clear_cache: false, load_paths: []) + def initialize( + allow_empty_tests: false, logging: nil, clear_cache: false, load_paths: [], + dependency_analysis: :imports + ) validate_boolean(allow_empty_tests, :allow_empty_tests) validate_logging(logging) validate_boolean(clear_cache, :clear_cache) load_paths = immutable_load_paths(load_paths) + unless %i[imports imports_and_constants].include?(dependency_analysis) + raise ArgumentError, 'dependency_analysis must be :imports or :imports_and_constants' + end + super end @@ -30,6 +39,10 @@ def clear_cache? clear_cache end + def constant_references? + dependency_analysis == :imports_and_constants + end + private def validate_boolean(value, attribute) diff --git a/lib/archunit/extraction/conservative_local_flow.rb b/lib/archunit/extraction/conservative_local_flow.rb new file mode 100644 index 0000000..0dc78f1 --- /dev/null +++ b/lib/archunit/extraction/conservative_local_flow.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +module ArchUnit + module Extraction + # Keeps local-string inference inside straight-line code and lexical scopes. + module ConservativeLocalFlow + def visit_def_node(node) + @local_strings.isolated_scope { super } + end + + def visit_class_node(node) + @local_strings.isolated_scope { super } + end + + def visit_module_node(node) + @local_strings.isolated_scope { super } + end + + def visit_singleton_class_node(node) + @local_strings.isolated_scope { super } + end + + def visit_block_node(node) + @local_strings.isolated_scope { super } + @local_strings.capture_scopes + end + + def visit_lambda_node(node) + @local_strings.isolated_scope { super } + @local_strings.capture_scopes + end + + def visit_local_variable_operator_write_node(node) + super + @local_strings.assign(node.name, nil) + end + + def visit_local_variable_or_write_node(node) + super + @local_strings.assign(node.name, nil) + end + + def visit_local_variable_and_write_node(node) + super + @local_strings.assign(node.name, nil) + end + + def visit_multi_write_node(node) + super + @local_strings.invalidate_all + end + + def visit_post_execution_node(node) + @local_strings.uncertain_flow { super } + @local_strings.capture_scopes + end + + def visit_match_write_node(node) + super + @local_strings.invalidate_all + end + + def visit_instance_variable_write_node(node) + super + @local_strings.invalidate_escaped(node.value) + end + + def visit_global_variable_write_node(node) + super + @local_strings.invalidate_escaped(node.value) + end + + def visit_class_variable_write_node(node) + super + @local_strings.invalidate_escaped(node.value) + end + + def visit_constant_write_node(node) + super + @local_strings.invalidate_escaped(node.value) + end + + def visit_constant_path_write_node(node) + super + @local_strings.invalidate_escaped(node.value) + end + + # These constructs require control-flow analysis beyond the supported straight-line subset. + %i[ + begin defined pre_execution if unless and or case case_match while until for + rescue rescue_modifier + ].each do |kind| + define_method("visit_#{kind}_node") do |node| + @local_strings.uncertain_flow { super(node) } + end + end + end + private_constant :ConservativeLocalFlow + end +end diff --git a/lib/archunit/extraction/constant_index.rb b/lib/archunit/extraction/constant_index.rb new file mode 100644 index 0000000..6a99aa2 --- /dev/null +++ b/lib/archunit/extraction/constant_index.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require_relative '../common/extraction/edge' +require_relative 'constant_visitor' + +module ArchUnit + module Extraction + # Bounded project definition index. Unknown/ambiguous names never invent file targets. + class ConstantIndex + Location = Data.define(:source, :line_number) + MAX_NAMESPACE_PASSES = 8 + + def initialize(analyses, profile: nil) + @analyses = analyses + @profile = profile + @visitors = stabilized_visitors + @definitions = definition_locations + @namespaces = namespace_names(@visitors) + @inherited_namespaces = @visitors.values.flat_map(&:inherited_namespaces).to_set + end + + def edges + @visitors.flat_map do |source, visitor| + visitor.references.filter_map { |reference| resolve_edge(source, reference) } + end + end + + private + + def stabilized_visitors + visitors = collect + MAX_NAMESPACE_PASSES.times do + names = namespace_names(visitors) + visitors = collect(namespace_names: names) + return visitors if namespace_names(visitors) == names + end + unresolved_count = visitors.values.sum { |visitor| visitor.references.length } + @profile&.increment(:unresolved_constant_references, unresolved_count) + {} + end + + def collect(namespace_names: nil) + @analyses.to_h do |source, result| + visitor = ConstantVisitor.new(namespace_names:) + visitor.visit(result.value) if result.success? + [source, visitor] + end + end + + def namespace_names(visitors) + visitors.values.flat_map(&:definitions).each_with_object(Set.new) do |definition, names| + parts = definition.name.split('::') + parts.length.times { |index| names << parts.take(index + 1).join('::') } + end + end + + def definition_locations + map = Hash.new { |hash, key| hash[key] = [] } + @visitors.each do |source, visitor| + visitor.definitions.each do |definition| + map[definition.name] << Location.new(source:, line_number: definition.line_number) + end + end + map + end + + def resolve_edge(source, reference) + name = resolve_name(reference) + locations = name ? @definitions.fetch(name, []) : [] + return unresolved(:unresolved_constant_references) if locations.empty? + return unresolved(:ambiguous_constant_references) if locations.map(&:source).uniq.length > 1 + + location = locations.first + return if source == location.source + + evidence_edge(source, reference, location) + end + + def resolve_name(reference) + name = reference.name + return name.delete_prefix('::') if name.start_with?('::') + + head = name.split('::').first + owner = reference.nesting.find { |scope| @namespaces.include?("#{scope}::#{head}") } + return "#{owner}::#{name}" if owner + return if @inherited_namespaces.include?(reference.inherited_scope) + + name + end + + def unresolved(counter) + @profile&.increment(counter) + nil + end + + def evidence_edge(source, reference, location) + kind = Common::Extraction::ImportKind::CONSTANT_REFERENCE + evidence = Common::Extraction::DependencyEvidence.new( + source:, kind:, name: reference.name, line_number: reference.line_number, + end_line: reference.end_line, target_line_number: location.line_number + ) + Common::Extraction::Edge.new( + source:, target: location.source, external: false, + import_kinds: [kind], evidence: [evidence] + ) + end + end + private_constant :ConstantIndex + end +end diff --git a/lib/archunit/extraction/constant_visitor.rb b/lib/archunit/extraction/constant_visitor.rb new file mode 100644 index 0000000..c34205f --- /dev/null +++ b/lib/archunit/extraction/constant_visitor.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'prism' + +module ArchUnit + module Extraction + # Collects declarations and uses with Ruby's lexical (not textual path) nesting. + class ConstantVisitor < Prism::Visitor + Definition = Data.define(:name, :line_number) + Reference = Data.define(:name, :nesting, :inherited_scope, :line_number, :end_line) + + attr_reader :definitions, :references, :inherited_namespaces + + def initialize(namespace_names: nil) + super() + @namespace_names = namespace_names + @definitions = [] + @references = [] + @nesting = [] + @inherited_namespaces = [] + end + + def visit_class_node(node) + visit(node.superclass) + visit_namespace(node, inherited: !node.superclass.nil?) + end + + def visit_module_node(node) + visit_namespace(node, inherited: false) + end + + def visit_constant_write_node(node) + define_constant(qualify(node.name.to_s), node) + visit(node.value) + end + + def visit_constant_path_write_node(node) + name = static_name(node.target) + define_constant(declaration_name(name), node) if name + visit(node.value) + end + + def visit_constant_read_node(node) + record_reference(node.name.to_s, node) + end + + def visit_constant_path_node(node) + name = static_name(node) + name ? record_reference(name, node) : super + end + + def visit_call_node(node) + if %i[include prepend extend const_set remove_const autoload].include?(node.name) && + !@nesting.empty? + @inherited_namespaces << @nesting.first + end + super + end + + def visit_singleton_class_node(node) + visit(node.expression) + previous = @dynamic_scope + @dynamic_scope = true + visit(node.body) + ensure + @dynamic_scope = previous + end + + private + + def visit_namespace(node, inherited:) + name = static_name(node.constant_path) + return unless name + + full_name = declaration_name(name) + define_constant(full_name, node.constant_path) + @inherited_namespaces << full_name if inherited + previous = @nesting + @nesting = [full_name, *previous] + visit(node.body) + ensure + @nesting = previous if previous + end + + def declaration_name(name) + return name.delete_prefix('::') if name.start_with?('::') || @nesting.empty? + return qualify(name) unless name.include?('::') + + head = name.split('::').first + owner = @nesting.find { |scope| @namespace_names&.include?("#{scope}::#{head}") } + owner ? "#{owner}::#{name}" : name + end + + def qualify(name) + @nesting.empty? ? name : "#{@nesting.first}::#{name}" + end + + def define_constant(name, node) + return if @dynamic_scope + + @definitions << Definition.new(name:, line_number: node.location.start_line) + end + + def record_reference(name, node) + return if @dynamic_scope && !name.start_with?('::') + + @references << Reference.new( + name:, nesting: @nesting.dup.freeze, inherited_scope: @nesting.first, + line_number: node.location.start_line, end_line: node.location.end_line + ) + end + + def static_name(node) + return node.name.to_s if node.is_a?(Prism::ConstantReadNode) + return unless node.is_a?(Prism::ConstantPathNode) + return "::#{node.name}" if node.parent.nil? + + parent = static_name(node.parent) + "#{parent}::#{node.name}" if parent + end + end + private_constant :ConstantVisitor + end +end diff --git a/lib/archunit/extraction/extract_dependencies.rb b/lib/archunit/extraction/extract_dependencies.rb index 06eb64a..9b45117 100644 --- a/lib/archunit/extraction/extract_dependencies.rb +++ b/lib/archunit/extraction/extract_dependencies.rb @@ -6,6 +6,7 @@ require_relative 'enumerate_source_files' require_relative 'extract_imports' require_relative 'resolve_load_paths' +require_relative 'constant_index' module ArchUnit # Turns statically resolvable Ruby imports into dependency edges. @@ -22,18 +23,21 @@ def extract_dependencies(project_root, exclude_patterns: nil, options: nil) load_paths = LoadPaths.resolve( root, explicit_paths: options.load_paths, source_files: ) - extract_dependencies_from(root, source_files, load_paths:) + extract_dependencies_from(root, source_files, load_paths:, options:) end - def extract_dependencies_from(root, source_files, load_paths:, profile: nil) + def extract_dependencies_from(root, source_files, load_paths:, options:, profile: nil) context = DependencyContext.new( root:, targets: profiled_target_index(root, source_files, profile), load_paths:, profile:, cache: {} ) - source_files.flat_map do |source| - dependencies_for_source(source, context) - end.freeze + analyses = options.constant_references? ? parse_sources(root, source_files, profile) : {} + edges = source_files.flat_map do |source| + dependencies_for_source(source, context, analyses[source]) + end + edges.concat(constant_edges(analyses, profile)) if options.constant_references? + edges.freeze end private_class_method :extract_dependencies_from @@ -44,13 +48,26 @@ def profiled_target_index(root, source_files, profile) end private_class_method :profiled_target_index - def dependencies_for_source(source, context) + def parse_sources(root, sources, profile) + sources.to_h { |source| [source, parse_source(root.join(source), profile)] } + end + private_class_method :parse_sources + + def constant_edges(analyses, profile) + ExtractionProfile.measure(profile, :constant_indexing) do + ConstantIndex.new(analyses, profile:).edges + end + end + private_class_method :constant_edges + + def dependencies_for_source(source, context, parse_result) imports = extract_imports_from( context.root.join(source), project_root: context.root, load_paths: context.load_paths, profile: context.profile, - resolution_cache: context.cache + resolution_cache: context.cache, + parse_result: ) context.profile&.increment(:imports, imports.length) classify_imports(source, imports, context) @@ -83,11 +100,20 @@ def dependency_edge(source, import, internal_targets) source: source, target: target || import.module_name, external: target.nil?, - import_kinds: [import.import_kind] + import_kinds: [import.import_kind], + evidence: [import_evidence(source, import)] ) end private_class_method :dependency_edge + def import_evidence(source, import) + Common::Extraction::DependencyEvidence.new( + source:, kind: import.import_kind, name: import.module_name, + line_number: import.line_number, end_line: import.end_line + ) + end + private_class_method :import_evidence + def canonical_path(path) normalized = Pathname.new(path).expand_path.cleanpath.to_s.tr('\\', '/') File::FNM_SYSCASE.zero? ? normalized : normalized.downcase diff --git a/lib/archunit/extraction/extract_graph.rb b/lib/archunit/extraction/extract_graph.rb index 4c894ba..002e652 100644 --- a/lib/archunit/extraction/extract_graph.rb +++ b/lib/archunit/extraction/extract_graph.rb @@ -62,14 +62,14 @@ def extract_graph_uncached(root, exclude_patterns, options, profile) load_paths = ExtractionProfile.measure(profile, :load_path_discovery) do LoadPaths.resolve(root, explicit_paths: options.load_paths, source_files:) end - edges = extraction_edges(root, source_files, load_paths, profile) + edges = extraction_edges(root, source_files, load_paths, options, profile) graph_from_edges(edges, profile) end private_class_method :extract_graph_uncached - def extraction_edges(root, source_files, load_paths, profile) + def extraction_edges(root, source_files, load_paths, options, profile) edges = self_edges(source_files) + extract_dependencies_from( - root, source_files, load_paths:, profile: + root, source_files, load_paths:, profile:, options: ) profile&.increment(:raw_edges, edges.length) edges @@ -122,7 +122,8 @@ def merge_parallel_edges(edges) source: first.source, target: first.target, external: first.external, - import_kinds: parallel_edges.flat_map(&:import_kinds) + import_kinds: parallel_edges.flat_map(&:import_kinds), + evidence: parallel_edges.flat_map(&:evidence) ) end end diff --git a/lib/archunit/extraction/extract_imports.rb b/lib/archunit/extraction/extract_imports.rb index 6b84b53..f727b2e 100644 --- a/lib/archunit/extraction/extract_imports.rb +++ b/lib/archunit/extraction/extract_imports.rb @@ -30,17 +30,21 @@ def extract_imports( raise TechnicalError, "could not parse Ruby source file: #{e.message}" end + # rubocop:disable Metrics/ParameterLists -- Share the parsed AST with constant analysis. def extract_imports_from( - source_path, project_root:, load_paths: nil, profile: nil, resolution_cache: nil + source_path, project_root:, load_paths: nil, profile: nil, resolution_cache: nil, + parse_result: nil ) resolver = build_import_resolver( source_path:, project_root:, load_paths:, profile:, cache: resolution_cache ) - resolve_imports(parsed_imports(source_path, profile), resolver) + parse_result ||= parse_source(source_path, profile) + resolve_imports(parsed_imports(parse_result, profile), resolver) rescue SystemCallError => e raise TechnicalError, "could not parse Ruby source file: #{e.message}" end private_class_method :extract_imports_from + # rubocop:enable Metrics/ParameterLists def build_import_resolver(**) ImportResolver.new(**) @@ -52,16 +56,23 @@ def resolve_imports(imports, resolver) end private_class_method :resolve_imports - def parsed_imports(source_path, profile) + def parse_source(source_path, profile) source = ExtractionProfile.measure(profile, :file_read) { File.binread(source_path) } - parse_result = ExtractionProfile.measure(profile, :prism_parse) do + ExtractionProfile.measure(profile, :prism_parse) do Prism.parse(source, filepath: source_path.to_s) end + rescue SystemCallError => e + raise TechnicalError, "could not parse Ruby source file: #{e.message}" + end + private_class_method :parse_source + + def parsed_imports(parse_result, profile) return [] unless parse_result.success? ExtractionProfile.measure(profile, :import_extraction) do visitor = ImportVisitor.new visitor.visit(parse_result.value) + profile&.increment(:unresolved_imports, visitor.unresolved_imports) reject_ignored_imports(visitor.imports, parse_result.comments) end end @@ -108,11 +119,12 @@ def ignored_module?(module_name, scoped_name) private_class_method :ignored_module? def resolved_import(located_import, resolver) - module_name, import_kind, line_number, = located_import + module_name, import_kind, line_number, end_line = located_import ResolvedImport.new( module_name: module_name, import_kind: import_kind, line_number: line_number, + end_line: end_line, resolved_path: resolver.resolve(module_name, import_kind) ) end diff --git a/lib/archunit/extraction/extraction_profile.rb b/lib/archunit/extraction/extraction_profile.rb index 372f2d3..2adf0f6 100644 --- a/lib/archunit/extraction/extraction_profile.rb +++ b/lib/archunit/extraction/extraction_profile.rb @@ -12,6 +12,7 @@ class ExtractionProfile file_read prism_parse import_extraction + constant_indexing target_resolution target_index edge_classification @@ -20,6 +21,9 @@ class ExtractionProfile COUNTERS = %i[ source_files imports + unresolved_imports + unresolved_constant_references + ambiguous_constant_references resolution_cache_hits resolution_cache_misses raw_edges diff --git a/lib/archunit/extraction/import_visitor.rb b/lib/archunit/extraction/import_visitor.rb index 34175bb..873d704 100644 --- a/lib/archunit/extraction/import_visitor.rb +++ b/lib/archunit/extraction/import_visitor.rb @@ -2,11 +2,15 @@ require 'prism' require_relative '../common/extraction/import_kind' +require_relative 'conservative_local_flow' +require_relative 'local_string_tracker' module ArchUnit module Extraction - # Collects supported literal dependency calls from a Prism syntax tree. + # Collects supported dependency calls from a Prism syntax tree. class ImportVisitor < Prism::Visitor + include ConservativeLocalFlow + CALL_KINDS = { require: Common::Extraction::ImportKind::REQUIRE, require_relative: Common::Extraction::ImportKind::REQUIRE_RELATIVE, @@ -14,38 +18,54 @@ class ImportVisitor < Prism::Visitor load: Common::Extraction::ImportKind::LOAD }.freeze - attr_reader :imports + attr_reader :imports, :unresolved_imports def initialize super @imports = [] + @unresolved_imports = 0 + @local_strings = LocalStringTracker.new end def visit_call_node(node) - record_import(node) + import_call = supported_import_call?(node) + record_import(node) if import_call + super + invalidate_escaped_locals(node) unless import_call + end + + def visit_local_variable_write_node(node) super + @local_strings.assign(node.name, node.value) end private def record_import(node) import_kind = CALL_KINDS[node.name] - return unless import_kind && supported_receiver?(node, import_kind) - arguments = node.arguments&.arguments || [] argument = import_argument(arguments, import_kind) - return unless argument.is_a?(Prism::StringNode) - return unless valid_module_name?(argument.unescaped) + module_name = @local_strings.resolve(argument) + unless valid_module_name?(module_name) + @unresolved_imports += 1 + return + end - @imports << located_import(argument, import_kind, node) + @imports << located_import(module_name, import_kind, node) end - def located_import(argument, import_kind, node) - [argument.unescaped, import_kind, node.location.start_line, node.location.end_line] + def located_import(module_name, import_kind, node) + [module_name, import_kind, node.location.start_line, node.location.end_line] end def valid_module_name?(module_name) - !module_name.empty? && !module_name.include?("\0") + module_name.is_a?(String) && !module_name.empty? && !module_name.include?("\0") + end + + def supported_import_call?(node) + import_kind = CALL_KINDS[node.name] + import_kind && supported_receiver?(node, import_kind) && + !import_argument(node.arguments&.arguments || [], import_kind).nil? end def supported_receiver?(node, import_kind) @@ -73,6 +93,11 @@ def import_argument(arguments, import_kind) arguments[0] end end + + def invalidate_escaped_locals(node) + @local_strings.invalidate_escaped(node) + @local_strings.capture_scopes if %i[eval binding local_variable_set].include?(node.name) + end end private_constant :ImportVisitor end diff --git a/lib/archunit/extraction/local_string_tracker.rb b/lib/archunit/extraction/local_string_tracker.rb new file mode 100644 index 0000000..7f5be4e --- /dev/null +++ b/lib/archunit/extraction/local_string_tracker.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'prism' + +module ArchUnit + module Extraction + # Conservatively tracks local variables whose current value is a literal String. + class LocalStringTracker + UNKNOWN = Object.new.freeze + private_constant :UNKNOWN + + def initialize + @scopes = [{}] + @captured_scopes = [false] + @uncertain_depth = 0 + end + + def resolve(node) + return node.unescaped if node.is_a?(Prism::StringNode) + return UNKNOWN if @uncertain_depth.positive? || @captured_scopes.last + return scope.fetch(node.name, UNKNOWN) if node.is_a?(Prism::LocalVariableReadNode) + + UNKNOWN + end + + def assign(name, value) + invalidate_escaped(value) if value && !value.is_a?(Prism::StringNode) + scope[name] = resolve(value) + end + + def invalidate_escaped(node) + local_reads(node).each { |name| scope[name] = UNKNOWN if scope.key?(name) } + end + + def isolated_scope + @scopes << {} + @captured_scopes << false + yield + ensure + @scopes.pop + @captured_scopes.pop + end + + def invalidate_all + @scopes.each { |values| values.transform_values! { UNKNOWN } } + end + + def capture_scopes + @captured_scopes.map! { true } + invalidate_all + end + + def uncertain_flow + invalidate_scope + @uncertain_depth += 1 + yield + ensure + @uncertain_depth -= 1 + invalidate_scope + end + + private + + def local_reads(node) + return [node.name] if node.is_a?(Prism::LocalVariableReadNode) + + node.compact_child_nodes.flat_map { |child| local_reads(child) }.uniq + end + + def invalidate_scope + scope.transform_values! { UNKNOWN } + end + + def scope + @scopes.last + end + end + private_constant :LocalStringTracker + end +end diff --git a/lib/archunit/extraction/resolved_import.rb b/lib/archunit/extraction/resolved_import.rb index f988d4b..038897d 100644 --- a/lib/archunit/extraction/resolved_import.rb +++ b/lib/archunit/extraction/resolved_import.rb @@ -4,11 +4,14 @@ module ArchUnit module Extraction - ResolvedImport = Data.define(:module_name, :import_kind, :line_number, :resolved_path) do - def initialize(module_name:, import_kind:, line_number:, resolved_path: nil) + ResolvedImport = Data.define(:module_name, :import_kind, :line_number, :resolved_path, + :end_line) do + def initialize(module_name:, import_kind:, line_number:, resolved_path: nil, + end_line: line_number) validate_module_name(module_name) validate_import_kind(import_kind) validate_line_number(line_number) + validate_line_number(end_line) validate_resolved_path(resolved_path) module_name = module_name.dup.freeze diff --git a/lib/archunit/graph/projection/aggregate_edges.rb b/lib/archunit/graph/projection/aggregate_edges.rb index cb23262..3a2c1a0 100644 --- a/lib/archunit/graph/projection/aggregate_edges.rb +++ b/lib/archunit/graph/projection/aggregate_edges.rb @@ -12,7 +12,7 @@ module AggregateEdges def aggregate(edges, collapse:, include_self_dependencies:) groups = Hash.new do |hash, key| - hash[key] = { count: 0, external: false, import_kinds: [] } + hash[key] = { count: 0, external: false, import_kinds: [], evidence: [] } end edges.each do |edge| @@ -30,6 +30,7 @@ def accumulate(group, edge) group[:count] += 1 group[:external] ||= edge.external group[:import_kinds] |= edge.import_kinds + group[:evidence] |= edge.evidence end private_class_method :accumulate @@ -37,7 +38,8 @@ def build_edges(groups) groups.sort_by { |(source, target), _group| [source, target] }.map do |key, group| GraphReportEdge.new( source: key.first, target: key.last, count: group[:count], - external: group[:external], import_kinds: group[:import_kinds] + external: group[:external], import_kinds: group[:import_kinds], + evidence: group[:evidence] ) end.freeze end diff --git a/lib/archunit/graph/projection/graph_report_edge.rb b/lib/archunit/graph/projection/graph_report_edge.rb index e404b31..d2051d7 100644 --- a/lib/archunit/graph/projection/graph_report_edge.rb +++ b/lib/archunit/graph/projection/graph_report_edge.rb @@ -1,20 +1,29 @@ # frozen_string_literal: true require_relative '../../common/extraction/import_kind' +require_relative '../../common/extraction/dependency_evidence' module ArchUnit module GraphReporting module Projection # Aggregated dependency evidence in a graph report snapshot. - GraphReportEdge = Data.define(:source, :target, :count, :external, :import_kinds) do - def initialize(source:, target:, count:, external:, import_kinds: []) + GraphReportEdge = Data.define(:source, :target, :count, :external, :import_kinds, + :evidence) do + # rubocop:disable Metrics/ParameterLists -- Keywords mirror this immutable report value. + def initialize(source:, target:, count:, external:, import_kinds: [], evidence: []) source = immutable_string(source, :source) target = immutable_string(target, :target) validate_count(count) validate_external(external) import_kinds = immutable_import_kinds(import_kinds) + evidence = Array(evidence).uniq.freeze + unless evidence.all?(Common::Extraction::DependencyEvidence) + raise ArgumentError, 'evidence must contain DependencyEvidence values' + end + super end + # rubocop:enable Metrics/ParameterLists private diff --git a/lib/archunit/graph/rendering/json_renderer.rb b/lib/archunit/graph/rendering/json_renderer.rb index 551c548..e1169c8 100644 --- a/lib/archunit/graph/rendering/json_renderer.rb +++ b/lib/archunit/graph/rendering/json_renderer.rb @@ -26,10 +26,12 @@ def snapshot_hash(snapshot) private_class_method :snapshot_hash def edge_hash(edge) - { + result = { source: edge.source, target: edge.target, count: edge.count, external: edge.external, import_kinds: edge.import_kinds } + result[:evidence] = edge.evidence.map(&:to_h) unless edge.evidence.empty? + result end private_class_method :edge_hash end diff --git a/lib/archunit/metrics/fluentapi/metric_selection.rb b/lib/archunit/metrics/fluentapi/metric_selection.rb index 83f2a53..123e1a3 100644 --- a/lib/archunit/metrics/fluentapi/metric_selection.rb +++ b/lib/archunit/metrics/fluentapi/metric_selection.rb @@ -29,8 +29,9 @@ def initialize(scope:, metric:) freeze end - def measure - scope.__send__(:subjects_for, metric.subject_type).map do |subject| + def measure(options = nil) + options = Common::FluentApi::CheckOptions.resolve(options) + scope.__send__(:subjects_for, metric.subject_type, options:).map do |subject| MetricMeasurement.new( subject:, metric_name: metric.name, diff --git a/scripts/check_dependency_analysis.rb b/scripts/check_dependency_analysis.rb new file mode 100644 index 0000000..ce45d87 --- /dev/null +++ b/scripts/check_dependency_analysis.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'fileutils' +require 'json' +require 'tmpdir' +require 'archunit' + +# Also run after installing the built gem, outside Bundler's source checkout. +Dir.mktmpdir('archunit-installed-analysis') do |root| + File.write(File.join(root, 'Gemfile'), "raise 'must never execute project source'\n") + %w[api retrieval].each { |name| FileUtils.mkdir_p(File.join(root, 'app', name)) } + request = File.join(root, 'app/api/request.rb') + File.write(File.join(root, 'app/retrieval/store.rb'), 'module Retrieval; class Store; end; end') + File.write(request, 'module Api; class Request; Retrieval::Store.new; end; end') + options = ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) + rule = ArchUnit.project_files(root).in_path('app/api/**') + .should_not.depend_on_files.in_path('app/retrieval/**') + abort 'import-only mode changed' unless rule.check.empty? + abort 'constant dependency missing' unless rule.check(options).length == 1 + allowed = ArchUnit.project_files(root).in_path('app/api/**') + .should.depend_on_files.in_path('app/retrieval/**') + ArchUnit.assert_passes(allowed, options) + payload = JSON.parse(ArchUnit.project_graph(root).with_check_options(options).to_json) + evidence = payload.fetch('edges').first.fetch('evidence').first + abort 'constant evidence missing' unless evidence['name'] == 'Retrieval::Store' + + File.write(request, "path = '../retrieval/store'\nrequire_relative path\n") + refreshed = ArchUnit::CheckOptions.new(clear_cache: true) + abort 'local-string dependency missing' unless rule.check(refreshed).length == 1 + ArchUnit.assert_passes(allowed) + puts 'Installed dependency analysis: constant and local-string reproducers passed.' +ensure + ArchUnit.clear_graph_cache +end diff --git a/spec/common/extraction/dependency_evidence_spec.rb b/spec/common/extraction/dependency_evidence_spec.rb new file mode 100644 index 0000000..d81d610 --- /dev/null +++ b/spec/common/extraction/dependency_evidence_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +RSpec.describe ArchUnit::DependencyEvidence do + def evidence(**attributes) + described_class.new( + source: 'app/api.rb', kind: :constant_reference, name: 'Store', line_number: 3, + **attributes + ) + end + + it 'owns immutable source strings and validates source and target spans' do + name = +'Store' + value = evidence(source: 'app\\api.rb', name:, end_line: 5, target_line_number: 2) + name.replace('Other') + + expect(value).to have_attributes(source: 'app/api.rb', name: 'Store', end_line: 5) + expect(value.name).to be_frozen + expect(value.source).to be_frozen + expect(value).to be_frozen + end + + it 'rejects invalid evidence identities and line spans' do + expect { evidence(kind: :dynamic) }.to raise_error(ArgumentError, /valid dependency kind/) + expect { evidence(source: nil) }.to raise_error(ArgumentError, /requires a source/) + expect { evidence(name: nil) }.to raise_error(ArgumentError, /requires a source/) + [0, nil, '3'].each do |line| + expect { evidence(line_number: line) }.to raise_error(ArgumentError, /source line numbers/) + end + expect { evidence(end_line: 2) }.to raise_error(ArgumentError, /source line numbers/) + expect { evidence(target_line_number: 0) }.to raise_error(ArgumentError, /source line numbers/) + end + + it 'requires typed evidence on raw and report edges' do + expect do + ArchUnit::Edge.new(source: 'a.rb', target: 'b.rb', external: false, evidence: ['untyped']) + end.to raise_error(ArgumentError, /DependencyEvidence/) + expect do + ArchUnit::GraphReportEdge.new( + source: 'a.rb', target: 'b.rb', external: false, count: 1, evidence: ['untyped'] + ) + end.to raise_error(ArgumentError, /DependencyEvidence/) + end +end diff --git a/spec/common/extraction/import_kind_spec.rb b/spec/common/extraction/import_kind_spec.rb index f38c593..1ca1bce 100644 --- a/spec/common/extraction/import_kind_spec.rb +++ b/spec/common/extraction/import_kind_spec.rb @@ -6,7 +6,8 @@ :require, :require_relative, :autoload, - :load + :load, + :constant_reference ) end diff --git a/spec/common/fluentapi/check_options_spec.rb b/spec/common/fluentapi/check_options_spec.rb index 565fbf5..a6eea37 100644 --- a/spec/common/fluentapi/check_options_spec.rb +++ b/spec/common/fluentapi/check_options_spec.rb @@ -8,7 +8,8 @@ allow_empty_tests: false, logging: nil, clear_cache: false, - load_paths: [] + load_paths: [], + dependency_analysis: :imports ) expect(options.allow_empty_tests?).to be(false) expect(options.clear_cache?).to be(false) @@ -57,4 +58,12 @@ expect(ArchUnit::CheckOptions).to equal(described_class) expect(ArchUnit::LoggingOptions).to equal(ArchUnit::Common::Logging::LoggingOptions) end + + it 'supports explicit constant analysis and rejects unknown analysis modes' do + expect(described_class.new).not_to be_constant_references + expanded = described_class.new(dependency_analysis: :imports_and_constants) + expect(expanded).to be_constant_references + expect { described_class.new(dependency_analysis: :dynamic) } + .to raise_error(ArgumentError, /dependency_analysis/) + end end diff --git a/spec/extraction/constant_dependencies_spec.rb b/spec/extraction/constant_dependencies_spec.rb new file mode 100644 index 0000000..a816c08 --- /dev/null +++ b/spec/extraction/constant_dependencies_spec.rb @@ -0,0 +1,228 @@ +# frozen_string_literal: true + +require 'fileutils' +require 'pathname' +require 'tmpdir' + +RSpec.describe ArchUnit::Extraction, 'constant dependencies' do + let(:options) { ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) } + + around do |example| + Dir.mktmpdir('archunit-constants') do |directory| + @root = Pathname.new(directory).realpath + described_class.clear_graph_cache + example.run + described_class.clear_graph_cache + end + end + + def source(path, contents) + file = @root.join(path) + FileUtils.mkdir_p(file.dirname) + file.write(contents) + end + + def dependencies(**keywords) + described_class.extract_graph(@root, options:, **keywords).reject do |edge| + edge.source == edge.target + end + end + + def targets(path = 'app/api/request.rb') + dependencies.select { |edge| edge.source == path }.map(&:target) + end + + it 'resolves the exact autoloaded application example without executing source' do + source('app/api/request.rb', <<~RUBY) + module Api + class Request + def call = Retrieval::Store.new + end + end + raise 'source executed' + RUBY + source('app/retrieval/store.rb', 'module Retrieval; class Store; end; end') + + expect(targets).to eq(['app/retrieval/store.rb']) + expect(dependencies.first.evidence).to contain_exactly( + have_attributes(name: 'Retrieval::Store', line_number: 3, target_line_number: 1) + ) + expect(described_class.extract_graph(@root).reject { |edge| edge.source == edge.target }) + .to be_empty + end + + it 'distinguishes lexical nesting from a compact namespace and root qualification' do + source('lib/outer.rb', 'module Outer; TOKEN = 1; end') + source('lib/root.rb', 'TOKEN = 2') + source('lib/nested.rb', 'module Outer; module Nested; TOKEN; end; end') + source('lib/compact.rb', 'module Outer::Compact; TOKEN; ::TOKEN; Outer::TOKEN; end') + + expect(targets('lib/nested.rb')).to eq(['lib/outer.rb']) + expect(targets('lib/compact.rb')).to eq(%w[lib/root.rb lib/outer.rb]) + end + + it 'does not fall back to another namespace when a nearer head shadows it' do + source('lib/global.rb', 'module Store; class Item; end; end') + source('lib/shadow.rb', 'module Api; module Store; end; end') + source('app/api/request.rb', 'module Api; Store::Item; end') + + expect(targets).to be_empty + end + + it 'resolves nested relative compact declarations using the known parent namespace' do + source('lib/namespaces.rb', 'module Outer; module Inner; end; end') + source('lib/item.rb', 'module Outer; class Inner::Item; end; end') + source('app/api/request.rb', 'Outer::Inner::Item') + + expect(targets).to eq(['lib/item.rb']) + end + + it 'records superclass and include, prepend, and extend references' do + source('lib/base.rb', 'class Base; end') + source('lib/mix.rb', 'module Mix; end') + source('app/api/request.rb', <<~RUBY) + class Request < ::Base + include ::Mix + prepend ::Mix + extend ::Mix + end + RUBY + + expect(targets).to eq(%w[lib/base.rb lib/mix.rb]) + mix = dependencies.find { |edge| edge.target == 'lib/mix.rb' } + expect(mix.evidence.map(&:line_number)).to eq([2, 3, 4]) + end + + it 'stabilizes dependent relative compact declarations before resolving references' do + source('lib/namespaces.rb', 'module Outer; module Inner; module Deep; end; end; end') + source('lib/namespace.rb', 'module Outer; module Inner::Deep; module Items; end; end; end') + source('lib/item.rb', 'module Outer; module Inner::Deep; class Items::Item; end; end; end') + source('app/api/request.rb', 'Outer::Inner::Deep::Items::Item') + + expect(targets).to eq(['lib/item.rb']) + end + + it 'avoids guessed lexical ownership in singleton and dynamically changed namespaces' do + source('lib/item.rb', 'class Item; end; module Store; class Item; end; end') + source('app/api/request.rb', <<~RUBY) + module Api + const_set(:Store, build_store) + Store::Item.new + class << self + Item = build_item + Item.new + end + end + RUBY + + expect(targets).to be_empty + end + + it 'falls back to imports when dependent namespace owners exceed the bounded index passes' do + source('lib/known.rb', 'class Known; end') + declarations = ['module Root', 'module Branch; end'] + 10.times { declarations.concat(['module Branch::Nested', 'module Branch; end']) } + declarations << '::Known.new' + declarations.concat(Array.new(11, 'end')) + source('app/api/request.rb', declarations.join("\n")) + profile = ArchUnit::Extraction::ExtractionProfile.new + + expect(dependencies(profile:)).to be_empty + expect(profile.counters[:unresolved_constant_references]).to eq(1) + end + + it 'resolves root-qualified writes and uses inside singleton bodies' do + source('lib/item.rb', 'class ::Item; end; ::VALUE = 1') + source('app/api/request.rb', 'class Request; class << self; ::Item.new; end; end; ::VALUE') + + expect(targets).to eq(['lib/item.rb']) + expect(dependencies.first.evidence.map(&:name)).to eq(%w[::Item ::VALUE]) + end + + it 'does not guess inherited constants or substitute a colliding root constant' do + source('lib/base.rb', 'class Base; TOKEN = 1; end') + source('lib/root.rb', 'TOKEN = 2') + source('app/api/request.rb', 'class Request < Base; TOKEN; end') + + expect(targets).to eq(['lib/base.rb']) + end + + it 'ignores declaration names, comments, string text, and same-file uses' do + source('lib/item.rb', 'class Item; end') + source('app/api/request.rb', <<~RUBY) + # Item.new + text = 'Item.new' + module Api + class Local; end + Local.new + end + RUBY + + expect(dependencies).to be_empty + end + + it 'treats reopened definitions in different files as ambiguous, including local reopenings' do + source('lib/one.rb', 'class Shared; end') + source('lib/two.rb', 'class Shared; Shared.new; end') + source('app/api/request.rb', 'Shared.new; Missing.new') + profile = ArchUnit::Extraction::ExtractionProfile.new + + expect(dependencies(profile:)).to be_empty + expect(profile.counters).to include( + ambiguous_constant_references: 2, unresolved_constant_references: 1 + ) + end + + it 'models a constant alias as a definition with a dependency on its right hand side' do + source('lib/item.rb', 'class Item; end') + source('lib/alias.rb', 'Alias = Item') + source('app/api/request.rb', 'Alias.new') + + expect(targets).to eq(['lib/alias.rb']) + expect(targets('lib/alias.rb')).to eq(['lib/item.rb']) + end + + it 'merges imports and uses into one edge while preserving both source locations' do + source('lib/item.rb', 'class Item; end') + source('app/api/request.rb', "require_relative '../../lib/item'\nItem.new\n") + + expect(dependencies).to contain_exactly( + have_attributes( + import_kinds: %i[require_relative constant_reference], + evidence: contain_exactly( + have_attributes(kind: :require_relative, line_number: 1), + have_attributes(kind: :constant_reference, name: 'Item', line_number: 2) + ) + ) + ) + end + + it 'shares cached indices across rules and refreshes changed definitions on clear_cache' do + source('lib/item.rb', 'class Item; end') + source('app/api/request.rb', 'Item.new') + first = described_class.extract_graph(@root, options:) + expect(described_class.extract_graph(@root, options:)).to equal(first) + expect(described_class.extract_graph(@root)).not_to equal(first) + source('lib/item.rb', 'class Renamed; end') + expect(targets).to eq(['lib/item.rb']) + + refreshed = described_class.extract_graph(@root, options: options.with(clear_cache: true)) + expect(refreshed.reject { |edge| edge.source == edge.target }).to be_empty + end + + it 'parses each source only once and reuses the graph on a repeated check' do + source('lib/item.rb', 'class Item; end') + source('app/api/request.rb', 'Item.new') + expect(Prism).to receive(:parse).with(anything, filepath: anything).twice.and_call_original + + expect(targets).to eq(['lib/item.rb']) + expect(targets).to eq(['lib/item.rb']) + end + + it 'uses the same excluded source scope for definitions and references' do + source('lib/item.rb', 'class Item; end') + source('app/api/request.rb', 'Item.new') + filtered = described_class.extract_graph(@root, options:, exclude_patterns: ['lib']) + expect(filtered.reject { |edge| edge.source == edge.target }).to be_empty + end +end diff --git a/spec/extraction/dependency_analysis_integration_spec.rb b/spec/extraction/dependency_analysis_integration_spec.rb new file mode 100644 index 0000000..8109fca --- /dev/null +++ b/spec/extraction/dependency_analysis_integration_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'fileutils' +require 'json' +require 'minitest' +require 'pathname' +require 'tmpdir' + +RSpec.describe 'expanded dependency analysis through the public API' do + let(:options) { ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) } + + around do |example| + Dir.mktmpdir('archunit-dependency-analysis') do |directory| + @root = Pathname.new(directory).realpath + @root.join('Gemfile').write("raise 'Gemfile executed'") + source('app/retrieval/store.rb', 'module Retrieval; class Store; end; end') + source('app/api/request.rb', 'module Api; class Request; Retrieval::Store.new; end; end') + ArchUnit.clear_graph_cache + example.run + ArchUnit.clear_graph_cache + end + end + + def source(path, contents) + file = @root.join(path) + FileUtils.mkdir_p(file.dirname) + file.write(contents) + end + + def file_rule(mood = :should_not) + ArchUnit.project_files(@root).in_path('app/api/**') + .public_send(mood).depend_on_files.in_path('app/retrieval/**') + end + + it 'reports the forbidden dependency exactly once and accepts its permitted counterpart' do + expect(file_rule.check).to be_empty + expect(file_rule.check(options)).to contain_exactly( + have_attributes(dependency: have_attributes(target_label: 'app/retrieval/store.rb')) + ) + expect { ArchUnit.assert_passes(file_rule(:should), options) }.not_to raise_error + end + + it 'recognizes the local-string reproducer through the default public file rules' do + source('app/api/request.rb', "path = '../retrieval/store'\nrequire_relative path\n") + + expect(file_rule.check.length).to eq(1) + expect(file_rule(:should)).to pass + expect(file_rule.check.first.dependency.cumulated_edges.first).to have_attributes( + import_kinds: [:require_relative], + evidence: contain_exactly(have_attributes(line_number: 2, name: '../retrieval/store')) + ) + end + + it 'detects both a constant-only cycle and a local-string-only cycle' do + source('app/retrieval/store.rb', 'module Retrieval; class Store; Api::Request.new; end; end') + rule = ArchUnit.project_files(@root).should.have_no_cycles + expect(rule.check).to be_empty + expect(rule.check(options).length).to eq(1) + + source('app/api/request.rb', "path = '../retrieval/store'\nrequire_relative path\n") + source('app/retrieval/store.rb', "path = '../api/request'\nrequire_relative path\n") + expect(rule.check(ArchUnit::CheckOptions.new(clear_cache: true)).length).to eq(1) + end + + it 'feeds identical constant edges into layer, slice, diagram, and graph consumers' do + layers = ArchUnit.project_layers(@root) + .layer('api').defined_by('app/api/**') + .layer('retrieval').defined_by('app/retrieval/**') + .where_layer('api').may_not_depend_on_layers('retrieval') + slices = ArchUnit.project_slices(@root).defined_by('app/(**)/') + expect(layers.check(options).length).to eq(1) + expect(slices.should_not.contain_dependency('api', 'retrieval').check(options).length).to eq(1) + diagram = slices.to_plantuml(options) + expect(diagram).to include('[api] --> [retrieval]') + expect(slices.should.adhere_to_diagram(diagram).check(options)).to be_empty + + report = ArchUnit.project_graph(@root).with_check_options(options) + payload = JSON.parse(report.to_json) + expect(payload.fetch('edges')).to contain_exactly( + include( + 'source' => 'app/api/request.rb', 'target' => 'app/retrieval/store.rb', + 'import_kinds' => ['constant_reference'], + 'evidence' => [include('name' => 'Retrieval::Store', 'line_number' => 1)] + ) + ) + expect(report.to_csv).to include('constant_reference') + expect(report.to_mermaid).to include('-->') + end + + it 'uses the same analysis through the RSpec and Minitest adapters' do + allowed = file_rule(:should) + expect(allowed).to pass(options) + ArchUnit::Testing::MinitestAdapter.install! + context = Class.new do + include Minitest::Assertions + + attr_accessor :assertions + end.new + context.assertions = 0 + expect(context.assert_passes(allowed, options)).to be(true) + expect { context.assert_passes(file_rule, options) }.to raise_error(Minitest::Assertion) + + source('app/api/request.rb', "path = '../retrieval/store'\nrequire_relative path\n") + ArchUnit.clear_graph_cache + expect(context.assert_passes(file_rule(:should))).to be(true) + expect { context.assert_passes(file_rule) }.to raise_error(Minitest::Assertion) + end + + it 'uses constant edges for coupling measurements and metric threshold checks' do + metric = ArchUnit.metrics(@root).in_path('app/api/**').distance.instability + + expect(metric.measure.first.value).to eq(0.0) + expect(metric.measure(options).first.value).to eq(1.0) + expect(metric.should_be(1.0).check(options)).to be_empty + end +end diff --git a/spec/extraction/extract_dependencies_spec.rb b/spec/extraction/extract_dependencies_spec.rb index 637369e..6d041fd 100644 --- a/spec/extraction/extract_dependencies_spec.rb +++ b/spec/extraction/extract_dependencies_spec.rb @@ -32,18 +32,18 @@ def create_file(relative_path, contents = '# fixture') edges = described_class.extract_dependencies(@project_root) expect(edges).to include( - ArchUnit::Edge.new( + have_attributes( source: 'lib/example.rb', target: 'lib/support.rb', external: false, import_kinds: [:require_relative] ), - ArchUnit::Edge.new( + have_attributes( source: 'lib/example.rb', target: 'lib/sample/widget.rb', external: false, import_kinds: [:require] ), - ArchUnit::Edge.new( + have_attributes( source: 'lib/example.rb', target: 'json', external: true, import_kinds: [:require] ), - ArchUnit::Edge.new( + have_attributes( source: 'lib/example.rb', target: 'missing_archunit_dependency', external: true, import_kinds: [:require] ) @@ -87,7 +87,7 @@ def create_file(relative_path, contents = '# fixture') create_file('vendor/dependency.rb') expect(described_class.extract_dependencies(@project_root)).to contain_exactly( - ArchUnit::Edge.new( + have_attributes( source: 'lib/example.rb', target: 'vendor/dependency', external: true, import_kinds: [:require] ) diff --git a/spec/extraction/extract_graph_spec.rb b/spec/extraction/extract_graph_spec.rb index bef9229..e3c73cd 100644 --- a/spec/extraction/extract_graph_spec.rb +++ b/spec/extraction/extract_graph_spec.rb @@ -70,7 +70,7 @@ def extract dependencies = extract.reject { |edge| edge.source == edge.target } expect(dependencies).to contain_exactly( - ArchUnit::Edge.new( + have_attributes( source: 'lib/source.rb', target: 'lib/target.rb', external: false, @@ -83,7 +83,7 @@ def extract create_file('lib/source.rb', "require_relative 'source'\n") expect(extract).to contain_exactly( - ArchUnit::Edge.new( + have_attributes( source: 'lib/source.rb', target: 'lib/source.rb', external: false, diff --git a/spec/extraction/extract_imports_spec.rb b/spec/extraction/extract_imports_spec.rb index 3ac1d3a..b54c3aa 100644 --- a/spec/extraction/extract_imports_spec.rb +++ b/spec/extraction/extract_imports_spec.rb @@ -53,6 +53,78 @@ def extract(relative_path = 'lib/example.rb') ) end + it 'resolves supported imports from preceding local literal assignments' do + create_file('lib/example.rb', <<~RUBY) + required = 'sample/widget' + relative = './support' + autoloaded = 'sample/plugin' + loaded = 'scripts/boot.rb' + require required + require_relative relative + autoload :Plugin, autoloaded + load loaded + RUBY + create_file('lib/support.rb') + create_file('lib/sample/widget.rb') + create_file('lib/sample/plugin.rb') + create_file('scripts/boot.rb') + + expect(extract.map { |item| [item.module_name, item.import_kind] }).to eq( + [ + ['sample/widget', :require], + ['./support', :require_relative], + ['sample/plugin', :autoload], + ['scripts/boot.rb', :load] + ] + ) + end + + it 'uses assignment order and invalidates reassigned or escaped values' do + create_file('lib/example.rb', <<~RUBY) + require_relative later + later = 'support' + path = 'first' + path = 'second' + require_relative path + path = ENV.fetch('DYNAMIC_PATH') + require_relative path + escaped = 'support' + consume(escaped) + require_relative escaped + RUBY + + expect(extract.map(&:module_name)).to eq(['second']) + end + + it 'does not retain stale string values through aliases, uncertain flow, or scope boundaries' do + create_file('lib/example.rb', <<~RUBY) + path = 'support' + alias_path = path + alias_path.replace('other') + require_relative path + + branch_path = 'support' + if condition + branch_path = 'first' + else + branch_path = 'second' + end + require_relative branch_path + + outer = 'support' + def load_dependency + require_relative outer + end + + block_path = 'support' + items.each do + require_relative block_path + end + RUBY + + expect(extract).to be_empty + end + it 'uses Ruby resolution rules for relative, project, standard-library, and load targets' do create_file('lib/example.rb', <<~RUBY) require_relative 'support' @@ -74,6 +146,117 @@ def extract(relative_path = 'lib/example.rb') expect(imports.fetch('missing_archunit_dependency').resolved_path).to be_nil end + it 'invalidates compound assignments, container aliases, block writes, and reflection' do + create_file('lib/example.rb', <<~RUBY) + path = 'support' + path += '/other' + require_relative path + path = 'support' + path ||= 'other' + require_relative path + path = 'support' + path, other = dynamic_values + require_relative path + path = 'support' + aliases = [path] + require_relative path + path = 'support' + @escaped = path + require_relative path + path = 'support' + items.each { path = 'other' } + require_relative path + path = 'support' + eval('path = "other"') + require_relative path + RUBY + + expect(extract).to be_empty + end + + it 'does not infer local strings inside or across unsupported control flow joins' do + create_file('lib/example.rb', <<~RUBY) + if condition + path = 'wrong_branch' + else + require_relative path + end + path = 'support' + condition && (path = 'other') + require_relative path + path = 'support' + while condition + path = 'other' + end + require_relative path + path = 'support' + begin + path = 'other' + rescue + require_relative path + end + require_relative path + require_relative 'literal_still_known' + RUBY + + expect(extract.map(&:module_name)).to eq(['literal_still_known']) + end + + it 'tracks independent literals inside methods, classes, and blocks and preserves ignores' do + create_file('lib/example.rb', <<~RUBY) + def boot + path = 'method_path' + require path + end + module Application + path = 'module_path' + load path + end + path = 'ignored_path' + require path # archunit: ignore ignored_path + tasks.each do + path = 'block_path' + require_relative path + end + RUBY + + expect(extract.map(&:module_name)).to eq(%w[method_path module_path block_path]) + end + + it 'does not infer strings after a closure or binding can mutate future assignments' do + create_file('lib/example.rb', <<~RUBY) + path = nil + mutate = -> { path.replace('other') } + path = 'support' + mutate.call + require_relative path + def separate_scope + captured = binding + path = 'support' + captured.local_variable_set(:path, 'other') + require_relative path + end + RUBY + + expect(extract).to be_empty + end + + it 'does not propagate locals into singleton bodies or through defined? and regexp writes' do + create_file('lib/example.rb', <<~RUBY) + path = 'support' + class << self + require_relative path + end + defined?(path = 'other') + require_relative path + path = 'support' + /(?.+)/ =~ input + require_relative path + RUBY + + expect(extract).to be_empty + end + it 'does not execute imported source while resolving it' do create_file('lib/example.rb', "require_relative 'explosive'\n") create_file('lib/explosive.rb', "raise 'executed'\n") @@ -82,7 +265,7 @@ def extract(relative_path = 'lib/example.rb') expect(extract.first.resolved_path).to end_with('/lib/explosive.rb') end - it 'ignores dynamic arguments, invalid arity, and custom require methods' do + it 'infers a local literal while ignoring other dynamic arguments and unsupported calls' do create_file('lib/example.rb', <<~'RUBY') name = 'json' require name @@ -93,7 +276,9 @@ def extract(relative_path = 'lib/example.rb') require 'one', 'two' RUBY - expect(extract).to be_empty + expect(extract.map { |item| [item.module_name, item.import_kind] }).to eq( + [['json', :require], ['json', :autoload]] + ) end it 'ignores empty and null-byte dependency names without aborting the file' do diff --git a/spec/extraction/extraction_profiling_integration_spec.rb b/spec/extraction/extraction_profiling_integration_spec.rb index 384eaf7..8020680 100644 --- a/spec/extraction/extraction_profiling_integration_spec.rb +++ b/spec/extraction/extraction_profiling_integration_spec.rb @@ -61,4 +61,14 @@ def create_file(relative_path, contents = '# fixture') expect { described_class.extract_graph(@project_root, profile: Object.new) } .to raise_error(ArgumentError, /ExtractionProfile/) end + + it 'counts unresolved runtime import paths without executing them' do + create_file('lib/source.rb', "path = ENV.fetch('UNKNOWN_PLUGIN')\nrequire path\n") + profile = ArchUnit::Extraction::ExtractionProfile.new + + graph = described_class.extract_graph(@project_root, profile:) + + expect(graph.length).to eq(1) + expect(profile.counters).to include(imports: 0, unresolved_imports: 1) + end end diff --git a/spec/extraction/load_paths_spec.rb b/spec/extraction/load_paths_spec.rb index 6e57c23..e5ef3aa 100644 --- a/spec/extraction/load_paths_spec.rb +++ b/spec/extraction/load_paths_spec.rb @@ -22,7 +22,7 @@ def create_file(relative_path, contents = '# fixture') end def dependency(source, target) - ArchUnit::Edge.new( + have_attributes( source:, target:, external: false, import_kinds: [:require] ) end From 8e5fa9973c45bf1f98778ae44034f97c396df053 Mon Sep 17 00:00:00 2001 From: TristanKruse Date: Thu, 15 Oct 2026 15:03:10 +0200 Subject: [PATCH 2/4] Preserve import-only edge compatibility and fix CI lint --- CHANGELOG.md | 3 ++- README.md | 7 +++++-- benchmark/README.md | 14 ++++++++------ lib/archunit/extraction/extract_dependencies.rb | 16 +++++++++++----- spec/extraction/constant_dependencies_spec.rb | 2 +- .../dependency_analysis_integration_spec.rb | 8 +++++++- spec/extraction/extract_dependencies_spec.rb | 10 +++++----- spec/extraction/extract_graph_spec.rb | 4 ++-- spec/extraction/load_paths_spec.rb | 2 +- 9 files changed, 42 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf1f0c9..ca22977 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ - Add opt-in project constant-reference dependencies with lexical lookup and conservative ambiguity handling. - Infer import paths from safely known local literal strings without evaluating application code. -- Preserve dependency source evidence through graph merging and JSON reporting. +- Preserve dependency source evidence in expanded mode through graph merging and JSON reporting, + while keeping import-only edge values compatible with existing consumers. - Support check options on metric measurements and add installed-gem extraction regression checks. - Discover adjacent `lib` directories in multi-gemspec repositories without evaluating gemspecs. - Support validated, project-local custom load paths through `CheckOptions`. diff --git a/README.md b/README.md index d986aac..5c3d080 100644 --- a/README.md +++ b/README.md @@ -413,11 +413,14 @@ or whether a conditional use executes. Qualified declaration owners are stabiliz if they do not stabilize within eight index passes, the project contributes imports only and its constant uses are counted as unresolved. -An import and a constant use between the same files produce one logical edge. Its `import_kinds` +In expanded mode, an import and a constant use between the same files produce one logical edge. +Its `import_kinds` includes `:constant_reference`; `edge.evidence` and JSON graph exports retain the referenced name, source span, and definition line for constant uses. Import evidence retains its call span. CSV, HTML, and diagram reports use the same merged graph; CSV/HTML distinguish dependency kinds. -`# archunit: ignore` continues to suppress imports only, not constant references. +`# archunit: ignore` continues to suppress imports only, not constant references. Import-only mode +leaves `edge.evidence` empty for compatibility with existing edge-value comparisons; +`ArchUnit::Extraction.extract_imports` retains call locations in both modes. For diagnostics, pass an `ArchUnit::Extraction::ExtractionProfile` to `ArchUnit::Extraction.extract_graph(root, options: options, profile: profile)`. Its counters include diff --git a/benchmark/README.md b/benchmark/README.md index 490e3bc..dff8337 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -46,17 +46,19 @@ On 2026-10-15, Ruby 4.0.6 on Windows x64, three fresh-process runs per mode on t | Measurement | Imports | Imports and constants | | --- | ---: | ---: | -| Cold graph build | 1.959 s | 2.019 s | -| Warm graph lookup | 3.23 ms | 2.62 ms | -| Constant indexing/resolution | — | 42.8 ms | +| Cold graph build | 1.706 s | 1.533 s | +| Warm graph lookup | 1.93 ms | 1.95 ms | +| Constant indexing/resolution | — | 32.0 ms | | Merged graph edges | 4,001 | 4,001 | All runs reused the identical cached graph on their warm lookup. The corpus references the same next file by import and constant, so expanded mode merges 5,001 raw edges into 4,001 edges while -retaining both evidence forms. Every source is parsed once per cold graph. The roughly 3% median -cold-time increase is a local observation, not a portable overhead guarantee. +retaining both evidence forms. Every source is parsed once per cold graph. Cold samples ranged from +1.45 to 1.86 seconds across modes. Filesystem and run-order noise prevent deriving a meaningful +overhead percentage from this small sample; the separately measured indexing stage is more useful +here than the difference between total medians. -Heap-growth samples ranged from 4.18–5.55 MB for imports and 5.51–14.19 MB for expanded analysis; +Heap-growth samples ranged from 3.07–5.49 MB for imports and 6.13–9.90 MB for expanded analysis; these pre-GC snapshots are sensitive to collection timing and do not establish retained or peak memory. Windows does not expose the Linux peak-RSS counter. ASTs and indexing structures are temporary; the cached graph retains immutable edges and evidence only. diff --git a/lib/archunit/extraction/extract_dependencies.rb b/lib/archunit/extraction/extract_dependencies.rb index 9b45117..0c2f8ad 100644 --- a/lib/archunit/extraction/extract_dependencies.rb +++ b/lib/archunit/extraction/extract_dependencies.rb @@ -11,7 +11,9 @@ module ArchUnit # Turns statically resolvable Ruby imports into dependency edges. module Extraction - DependencyContext = Data.define(:root, :targets, :load_paths, :profile, :cache) + DependencyContext = Data.define( + :root, :targets, :load_paths, :profile, :cache, :include_evidence + ) private_constant :DependencyContext module_function @@ -29,7 +31,7 @@ def extract_dependencies(project_root, exclude_patterns: nil, options: nil) def extract_dependencies_from(root, source_files, load_paths:, options:, profile: nil) context = DependencyContext.new( root:, targets: profiled_target_index(root, source_files, profile), load_paths:, - profile:, cache: {} + profile:, cache: {}, include_evidence: options.constant_references? ) analyses = options.constant_references? ? parse_sources(root, source_files, profile) : {} @@ -76,7 +78,11 @@ def dependencies_for_source(source, context, parse_result) def classify_imports(source, imports, context) ExtractionProfile.measure(context.profile, :edge_classification) do - imports.map { |import| dependency_edge(source, import, context.targets) } + imports.map do |import| + dependency_edge( + source, import, context.targets, include_evidence: context.include_evidence + ) + end end end private_class_method :classify_imports @@ -93,7 +99,7 @@ def internal_target_index(root, source_files) end private_class_method :internal_target_index - def dependency_edge(source, import, internal_targets) + def dependency_edge(source, import, internal_targets, include_evidence:) target = import.resolved_path && internal_targets[canonical_path(import.resolved_path)] Common::Extraction::Edge.new( @@ -101,7 +107,7 @@ def dependency_edge(source, import, internal_targets) target: target || import.module_name, external: target.nil?, import_kinds: [import.import_kind], - evidence: [import_evidence(source, import)] + evidence: include_evidence ? [import_evidence(source, import)] : [] ) end private_class_method :dependency_edge diff --git a/spec/extraction/constant_dependencies_spec.rb b/spec/extraction/constant_dependencies_spec.rb index a816c08..be1d8d8 100644 --- a/spec/extraction/constant_dependencies_spec.rb +++ b/spec/extraction/constant_dependencies_spec.rb @@ -121,7 +121,7 @@ class << self it 'falls back to imports when dependent namespace owners exceed the bounded index passes' do source('lib/known.rb', 'class Known; end') declarations = ['module Root', 'module Branch; end'] - 10.times { declarations.concat(['module Branch::Nested', 'module Branch; end']) } + 10.times { declarations.push('module Branch::Nested', 'module Branch; end') } declarations << '::Known.new' declarations.concat(Array.new(11, 'end')) source('app/api/request.rb', declarations.join("\n")) diff --git a/spec/extraction/dependency_analysis_integration_spec.rb b/spec/extraction/dependency_analysis_integration_spec.rb index 8109fca..12dda04 100644 --- a/spec/extraction/dependency_analysis_integration_spec.rb +++ b/spec/extraction/dependency_analysis_integration_spec.rb @@ -45,10 +45,16 @@ def file_rule(mood = :should_not) expect(file_rule.check.length).to eq(1) expect(file_rule(:should)).to pass - expect(file_rule.check.first.dependency.cumulated_edges.first).to have_attributes( + expect(file_rule.check(options).first.dependency.cumulated_edges.first).to have_attributes( import_kinds: [:require_relative], evidence: contain_exactly(have_attributes(line_number: 2, name: '../retrieval/store')) ) + expect(file_rule.check.first.dependency.cumulated_edges.first).to eq( + ArchUnit::Edge.new( + source: 'app/api/request.rb', target: 'app/retrieval/store.rb', external: false, + import_kinds: [:require_relative] + ) + ) end it 'detects both a constant-only cycle and a local-string-only cycle' do diff --git a/spec/extraction/extract_dependencies_spec.rb b/spec/extraction/extract_dependencies_spec.rb index 6d041fd..637369e 100644 --- a/spec/extraction/extract_dependencies_spec.rb +++ b/spec/extraction/extract_dependencies_spec.rb @@ -32,18 +32,18 @@ def create_file(relative_path, contents = '# fixture') edges = described_class.extract_dependencies(@project_root) expect(edges).to include( - have_attributes( + ArchUnit::Edge.new( source: 'lib/example.rb', target: 'lib/support.rb', external: false, import_kinds: [:require_relative] ), - have_attributes( + ArchUnit::Edge.new( source: 'lib/example.rb', target: 'lib/sample/widget.rb', external: false, import_kinds: [:require] ), - have_attributes( + ArchUnit::Edge.new( source: 'lib/example.rb', target: 'json', external: true, import_kinds: [:require] ), - have_attributes( + ArchUnit::Edge.new( source: 'lib/example.rb', target: 'missing_archunit_dependency', external: true, import_kinds: [:require] ) @@ -87,7 +87,7 @@ def create_file(relative_path, contents = '# fixture') create_file('vendor/dependency.rb') expect(described_class.extract_dependencies(@project_root)).to contain_exactly( - have_attributes( + ArchUnit::Edge.new( source: 'lib/example.rb', target: 'vendor/dependency', external: true, import_kinds: [:require] ) diff --git a/spec/extraction/extract_graph_spec.rb b/spec/extraction/extract_graph_spec.rb index e3c73cd..bef9229 100644 --- a/spec/extraction/extract_graph_spec.rb +++ b/spec/extraction/extract_graph_spec.rb @@ -70,7 +70,7 @@ def extract dependencies = extract.reject { |edge| edge.source == edge.target } expect(dependencies).to contain_exactly( - have_attributes( + ArchUnit::Edge.new( source: 'lib/source.rb', target: 'lib/target.rb', external: false, @@ -83,7 +83,7 @@ def extract create_file('lib/source.rb', "require_relative 'source'\n") expect(extract).to contain_exactly( - have_attributes( + ArchUnit::Edge.new( source: 'lib/source.rb', target: 'lib/source.rb', external: false, diff --git a/spec/extraction/load_paths_spec.rb b/spec/extraction/load_paths_spec.rb index e5ef3aa..6e57c23 100644 --- a/spec/extraction/load_paths_spec.rb +++ b/spec/extraction/load_paths_spec.rb @@ -22,7 +22,7 @@ def create_file(relative_path, contents = '# fixture') end def dependency(source, target) - have_attributes( + ArchUnit::Edge.new( source:, target:, external: false, import_kinds: [:require] ) end From 894b9fc3299c39c055ba31ca616999526e296952 Mon Sep 17 00:00:00 2001 From: TristanKruse Date: Fri, 16 Oct 2026 16:05:48 +0200 Subject: [PATCH 3/4] Align extraction fixes with merged diagnostics and quality tooling --- CHANGELOG.md | 2 +- README.md | 16 ++++++++-------- .../common/extraction/dependency_evidence.rb | 8 ++++---- lib/archunit/extraction/extract_imports.rb | 5 ++--- .../graph/projection/graph_report_edge.rb | 7 ++++--- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60e8e5e..1b219fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 0.0.2 - 2026-09-30 +## 0.0.2 (unreleased) - Add opt-in debug inspection of project roots, graphs, and selected files on cold and cached checks. - Log passing metric values without repeating custom calculations. diff --git a/README.md b/README.md index 0e94ea1..3110575 100644 --- a/README.md +++ b/README.md @@ -351,12 +351,12 @@ Check options and returned violations retain their existing behavior. Example debug lines (timestamp prefixes omitted): -```text +~~~ text [DEBUG] inspect: discovered file: "lib/service.rb" [DEBUG] inspect: dependency: "lib/service.rb" -> "json" (external=true, kinds=[:require]) [DEBUG] inspect: selected file: "lib/service.rb" [DEBUG] log metric: method_count=3 [lib/service.rb:Service] -``` +~~~ For file-only CI artifacts, use `io: nil` with `output_directory:` and archive that directory. Logging sink errors still propagate. Debug details are produced lazily; checks with logging @@ -366,11 +366,11 @@ disabled create no output or log files. The same formatted, numbered reports work with RSpec, Minitest, or standalone checks: -```ruby +~~~ ruby puts ArchUnit.format_violations(violations) # Detect terminal support puts ArchUnit.format_violations(violations, color: true) # Force ANSI colors puts ArchUnit.format_violations(violations, color: false) # Plain CI/file output -``` +~~~ Failure headings are bold red, violation headings yellow, and successful results green. Reports include dependency, file, or metric evidence. Automatic color detection respects `NO_COLOR`, @@ -402,10 +402,10 @@ require 'experimental/plugin' Preceding local assignments to literal strings are also recognized for all four import forms: -```ruby +~~~ ruby path = '../retrieval/store' require_relative path -``` +~~~ Inference follows assignment order within straight-line lexical scopes. Reassignment replaces the value; aliases, mutation, unknown calls receiving the value, reflection, and unsupported control @@ -419,7 +419,7 @@ Gemfile, gemspec, initializer, or `const_missing` hook is executed. For autoloaded applications, enable dependencies from known project constants alongside imports: -```ruby +~~~ ruby options = ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) # Detects Retrieval::Store.new even without require_relative. @@ -432,7 +432,7 @@ report = ArchUnit.project_graph.with_check_options(options) puts report.to_json puts ArchUnit.project_slices.defined_by('app/(**)/').to_plantuml(options) values = ArchUnit.metrics.distance.instability.measure(options) -``` +~~~ The default remains `dependency_analysis: :imports`. Expanded mode indexes declarations from the same included source files, then resolves references to definition files. Files remain graph nodes. diff --git a/lib/archunit/common/extraction/dependency_evidence.rb b/lib/archunit/common/extraction/dependency_evidence.rb index c0be068..8e5acda 100644 --- a/lib/archunit/common/extraction/dependency_evidence.rb +++ b/lib/archunit/common/extraction/dependency_evidence.rb @@ -9,16 +9,16 @@ module Extraction DependencyEvidence = Data.define( :source, :kind, :name, :line_number, :end_line, :target_line_number ) do - # rubocop:disable Metrics/ParameterLists -- Keywords mirror this immutable source span. - def initialize(source:, kind:, name:, line_number:, end_line: line_number, - target_line_number: nil) + # Keywords mirror this immutable source span. + def initialize( # rubocop:disable Metrics/ParameterLists + source:, kind:, name:, line_number:, end_line: line_number, target_line_number: nil + ) validate_identity(source, kind, name) validate_lines(line_number, end_line, target_line_number) source = source.tr('\\', '/').freeze name = name.dup.freeze super end - # rubocop:enable Metrics/ParameterLists private diff --git a/lib/archunit/extraction/extract_imports.rb b/lib/archunit/extraction/extract_imports.rb index f727b2e..c079405 100644 --- a/lib/archunit/extraction/extract_imports.rb +++ b/lib/archunit/extraction/extract_imports.rb @@ -30,8 +30,8 @@ def extract_imports( raise TechnicalError, "could not parse Ruby source file: #{e.message}" end - # rubocop:disable Metrics/ParameterLists -- Share the parsed AST with constant analysis. - def extract_imports_from( + # Share the parsed AST with constant analysis. + def extract_imports_from( # rubocop:disable Metrics/ParameterLists source_path, project_root:, load_paths: nil, profile: nil, resolution_cache: nil, parse_result: nil ) @@ -44,7 +44,6 @@ def extract_imports_from( raise TechnicalError, "could not parse Ruby source file: #{e.message}" end private_class_method :extract_imports_from - # rubocop:enable Metrics/ParameterLists def build_import_resolver(**) ImportResolver.new(**) diff --git a/lib/archunit/graph/projection/graph_report_edge.rb b/lib/archunit/graph/projection/graph_report_edge.rb index d2051d7..deb6e12 100644 --- a/lib/archunit/graph/projection/graph_report_edge.rb +++ b/lib/archunit/graph/projection/graph_report_edge.rb @@ -9,8 +9,10 @@ module Projection # Aggregated dependency evidence in a graph report snapshot. GraphReportEdge = Data.define(:source, :target, :count, :external, :import_kinds, :evidence) do - # rubocop:disable Metrics/ParameterLists -- Keywords mirror this immutable report value. - def initialize(source:, target:, count:, external:, import_kinds: [], evidence: []) + # Keywords mirror this immutable report value. + def initialize( # rubocop:disable Metrics/ParameterLists + source:, target:, count:, external:, import_kinds: [], evidence: [] + ) source = immutable_string(source, :source) target = immutable_string(target, :target) validate_count(count) @@ -23,7 +25,6 @@ def initialize(source:, target:, count:, external:, import_kinds: [], evidence: super end - # rubocop:enable Metrics/ParameterLists private From ef48253626916241b76a4ef1c26030e0fe1a0865 Mon Sep 17 00:00:00 2001 From: TristanKruse Date: Fri, 16 Oct 2026 16:49:09 +0200 Subject: [PATCH 4/4] Keep combined metric documentation within lint limits --- lib/archunit/metrics/fluentapi/metric_selection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/archunit/metrics/fluentapi/metric_selection.rb b/lib/archunit/metrics/fluentapi/metric_selection.rb index 67be89c..1fc2faf 100644 --- a/lib/archunit/metrics/fluentapi/metric_selection.rb +++ b/lib/archunit/metrics/fluentapi/metric_selection.rb @@ -40,7 +40,7 @@ def initialize(scope:, metric:) end # Evaluates this metric for every selected subject. - # @param options [CheckOptions, nil] includes the dependency-analysis mode for coupling metrics + # @param options [CheckOptions, nil] dependency-analysis options for coupling metrics # @return [Array] def measure(options = nil) options = Common::FluentApi::CheckOptions.resolve(options)