diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aaa39b5..21f9d71 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/API.md b/API.md index 46d074a..12c77a3 100644 --- a/API.md +++ b/API.md @@ -189,6 +189,7 @@ options = ArchUnit::CheckOptions.new( allow_empty_tests: false, clear_cache: false, load_paths: ['components/billing/lib'], + dependency_analysis: :imports, logging: logging ) @@ -200,6 +201,31 @@ when an empty scope is intentionally valid. Set `clear_cache: true` after modify project during one process. `load_paths` adds project-contained source roots without evaluating gemspecs or running application code. +`dependency_analysis` defaults to `:imports`. Imports accept literal arguments and preceding local +literal-string assignments in supported straight-line scopes. Aliases, mutation/escape, closures, +reflection, and unsupported control flow invalidate inferred values; runtime paths are not guessed. + +For autoloaded projects, opt into known project constant references alongside imports: + +~~~ruby +options = ArchUnit::CheckOptions.new(dependency_analysis: :imports_and_constants) +violations = rule.check(options) +json = ArchUnit.project_graph.with_check_options(options).to_json +values = ArchUnit.metrics.distance.instability.measure(options) +~~~ + +Expanded analysis resolves supported lexical, compact, and root-qualified references to their +definition files. Explicit superclass and mixin references also contribute edges. Missing or +ambiguous definitions, inherited-member lookup, and dynamic metaprogramming stay unresolved under +the bounded contract in the main guide. Namespaces that fail to stabilize within eight index passes +fall back to import-only results. Unknown third-party constants do not create architecture violations. + +Expanded edges include `:constant_reference` in `import_kinds` and retain `DependencyEvidence` with +names and source lines in `edge.evidence` and JSON reports. An import and a constant use supporting +the same file pair merge into one edge. Import-only edge evidence stays empty for compatibility; +`ArchUnit::Extraction.extract_imports` retains import call locations in both modes. The analysis +mode participates in graph-cache identity, and `clear_cache: true` rebuilds the symbol index. + ## Errors and support Invalid API input raises `ArchUnit::UserError` or `ArgumentError`; library or environment failures diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ffb50c..6c74829 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ - Log passing metric values without repeating custom calculations. - Document verbosity, CI log artifacts, and existing colored violation reports. +- 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 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. - Make trailing folder globs such as `app/api/**` include the named folder itself. - Validate the README quickstart as a passing and failing end-to-end architecture test. - Add a curated API guide, reliable Markdown rendering, responsive documentation navigation, and diff --git a/README.md b/README.md index 43bf646..a61bf9c 100644 --- a/README.md +++ b/README.md @@ -407,8 +407,79 @@ 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. + +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. 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 +`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 @@ -425,8 +496,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 @@ -474,8 +546,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..dff8337 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,30 @@ 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.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. 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 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. + ## 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 37296e6..1a886f9 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..8e5acda --- /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 + # 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 + + 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 5112487..fd03817 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 # Normalizes nil or an existing options value for terminal execution. # @param value [CheckOptions, nil] # @return [CheckOptions] @@ -21,11 +23,19 @@ def self.resolve(value) # @param logging [LoggingOptions, nil] per-check logging configuration # @param clear_cache [Boolean] rebuild the dependency graph for this check # @param load_paths [Array] additional project-contained source roots - def initialize(allow_empty_tests: false, logging: nil, clear_cache: false, load_paths: []) + # @param dependency_analysis [Symbol] :imports or opt-in :imports_and_constants + 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 @@ -39,6 +49,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..0c2f8ad 100644 --- a/lib/archunit/extraction/extract_dependencies.rb +++ b/lib/archunit/extraction/extract_dependencies.rb @@ -6,11 +6,14 @@ 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. 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 @@ -22,18 +25,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: {} + profile:, cache: {}, include_evidence: options.constant_references? ) - 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 +50,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) @@ -59,7 +78,11 @@ def dependencies_for_source(source, context) 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 @@ -76,18 +99,27 @@ 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( source: source, target: target || import.module_name, external: target.nil?, - import_kinds: [import.import_kind] + import_kinds: [import.import_kind], + evidence: include_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 b4390a9..b18ed3c 100644 --- a/lib/archunit/extraction/extract_graph.rb +++ b/lib/archunit/extraction/extract_graph.rb @@ -63,14 +63,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 @@ -123,7 +123,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..c079405 100644 --- a/lib/archunit/extraction/extract_imports.rb +++ b/lib/archunit/extraction/extract_imports.rb @@ -30,13 +30,16 @@ def extract_imports( raise TechnicalError, "could not parse Ruby source file: #{e.message}" end - def extract_imports_from( - source_path, project_root:, load_paths: nil, profile: nil, resolution_cache: nil + # 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 ) 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 @@ -52,16 +55,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 +118,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..deb6e12 100644 --- a/lib/archunit/graph/projection/graph_report_edge.rb +++ b/lib/archunit/graph/projection/graph_report_edge.rb @@ -1,18 +1,28 @@ # 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 + # 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) 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 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 0a63864..1fc2faf 100644 --- a/lib/archunit/metrics/fluentapi/metric_selection.rb +++ b/lib/archunit/metrics/fluentapi/metric_selection.rb @@ -40,9 +40,11 @@ def initialize(scope:, metric:) end # Evaluates this metric for every selected subject. + # @param options [CheckOptions, nil] dependency-analysis options for coupling metrics # @return [Array] - 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..be1d8d8 --- /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.push('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..12dda04 --- /dev/null +++ b/spec/extraction/dependency_analysis_integration_spec.rb @@ -0,0 +1,122 @@ +# 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(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 + 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_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