Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,13 @@ mode participates in graph-cache identity, and `clear_cache: true` rebuilds the
Invalid API input raises `ArchUnit::UserError` or `ArgumentError`; library or environment failures
raise `ArchUnit::TechnicalError`. Rule failures remain violation data.

Malformed included Ruby raises `ArchUnit::SourceParseError`, a `TechnicalError`, during graph or
AST-metric extraction. No partial graph is returned or cached. `error.diagnostics` is an immutable
array of `ArchUnit::ParseDiagnostic` values with `path`, one-based `line`/`column`, and `message`;
each value supports `to_h`. RSpec/Minitest assertions propagate this technical failure instead of
reporting a pass or an ordinary architecture violation. Excluded files are not parsed. The profile's
`malformed_files` counter records a rejected file; existing cache-clearing rules still apply.

See the [main guide](index.html) for installation and complete examples, [SUPPORT.md](https://github.com/LukasNiessen/ArchUnitRuby/blob/main/SUPPORT.md)
for help, and [SECURITY.md](https://github.com/LukasNiessen/ArchUnitRuby/blob/main/SECURITY.md) for
responsible vulnerability reporting.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## 0.0.2 (unreleased)

- Fail closed on malformed included Ruby in dependency graphs and AST metrics, with structured
source diagnostics and profile counters instead of silently treating incomplete analysis as clean.

- Add development-only locked dependency scanning with advisory refresh, fail-closed health checks,
Windows path normalization, and retained CI reports.

Expand Down
9 changes: 8 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,14 @@ 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.
a cache hit does not rerun diagnostics. Included malformed Ruby now raises
`ArchUnit::SourceParseError < ArchUnit::TechnicalError` in both modes and in AST metrics, rather
than silently returning a partial graph or zero counts. Its immutable `diagnostics` contain
`path`, one-based `line` and `column`, and the parser `message`; the extraction profile increments
`malformed_files`. Fix or explicitly exclude invalid files using the project source policy.
Fluent rule selectors do not limit graph discovery. Failed graph builds are not cached; after
editing previously cached valid source, the normal explicit `clear_cache: true` policy still applies.
This intentionally changes the prototype's permissive behavior before the stable release.

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
Expand Down
2 changes: 2 additions & 0 deletions lib/archunit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require_relative 'archunit/version'
require_relative 'archunit/error/technical_error'
require_relative 'archunit/error/source_parse_error'
require_relative 'archunit/error/user_error'
require_relative 'archunit/common/pattern'
require_relative 'archunit/common/filter'
Expand Down Expand Up @@ -54,6 +55,7 @@ module ArchUnit
ImportKind = Common::Extraction::ImportKind
Edge = Common::Extraction::Edge
DependencyEvidence = Common::Extraction::DependencyEvidence
ParseDiagnostic = Common::Extraction::ParseDiagnostic
Graph = Common::Extraction::Graph
MappedEdge = Common::Projection::MappedEdge
ProjectedEdge = Common::Projection::ProjectedEdge
Expand Down
23 changes: 23 additions & 0 deletions lib/archunit/common/extraction/parse_diagnostic.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module ArchUnit
module Common
module Extraction
# Immutable parser failure location; line and column are both one-based.
ParseDiagnostic = Data.define(:path, :line, :column, :message) do
def initialize(path:, line:, column:, message:)
unless [path, message].all? { |value| value.is_a?(String) && !value.empty? }
raise ArgumentError, 'parse diagnostic requires a non-empty path and message'
end
unless [line, column].all? { |value| value.is_a?(Integer) && value.positive? }
raise ArgumentError, 'parse diagnostic coordinates must be positive integers'
end

path = path.tr('\\', '/').freeze
message = message.dup.freeze
super
end
end
end
end
end
38 changes: 38 additions & 0 deletions lib/archunit/error/source_parse_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

require_relative 'technical_error'
require_relative '../common/extraction/parse_diagnostic'

module ArchUnit
# An included source could not be analyzed; never an ordinary architecture violation.
class SourceParseError < TechnicalError
# @return [Array<Common::Extraction::ParseDiagnostic>] immutable source diagnostics
attr_reader :diagnostics

# @return [Prism::ParseResult] the complete result, or a technical failure
def self.validate!(result, path:)
return result if result.success?

diagnostics = result.errors.map do |error|
Common::Extraction::ParseDiagnostic.new(
path: path.to_s, line: error.location.start_line,
column: error.location.start_column + 1, message: error.message
)
end
raise SourceParseError, diagnostics
end

def initialize(diagnostics)
unless diagnostics.is_a?(Array) && !diagnostics.empty? &&
diagnostics.all?(Common::Extraction::ParseDiagnostic)
raise ArgumentError, 'source parse error requires non-empty parse diagnostics'
end

@diagnostics = diagnostics.dup.freeze
details = @diagnostics.map do |item|
"#{item.path}:#{item.line}:#{item.column}: #{item.message}"
end
super("Cannot analyze malformed Ruby source:\n#{details.join("\n")}")
end
end
end
2 changes: 1 addition & 1 deletion lib/archunit/extraction/constant_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def stabilized_visitors
def collect(namespace_names: nil)
@analyses.to_h do |source, result|
visitor = ConstantVisitor.new(namespace_names:)
visitor.visit(result.value) if result.success?
visitor.visit(result.value)
[source, visitor]
end
end
Expand Down
7 changes: 4 additions & 3 deletions lib/archunit/extraction/extract_imports.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'prism'
require_relative '../error/technical_error'
require_relative '../error/source_parse_error'
require_relative '../error/user_error'
require_relative 'extraction_profile'
require_relative 'import_resolver'
Expand Down Expand Up @@ -57,17 +58,17 @@ def resolve_imports(imports, resolver)

def parse_source(source_path, profile)
source = ExtractionProfile.measure(profile, :file_read) { File.binread(source_path) }
ExtractionProfile.measure(profile, :prism_parse) do
result = ExtractionProfile.measure(profile, :prism_parse) do
Prism.parse(source, filepath: source_path.to_s)
end
profile&.increment(:malformed_files) unless result.success?
SourceParseError.validate!(result, path: source_path)
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)
Expand Down
1 change: 1 addition & 0 deletions lib/archunit/extraction/extraction_profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class ExtractionProfile
].freeze
COUNTERS = %i[
source_files
malformed_files
imports
unresolved_imports
unresolved_constant_references
Expand Down
9 changes: 5 additions & 4 deletions lib/archunit/metrics/extraction/extract_project_info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'pathname'
require 'prism'
require_relative '../../error/technical_error'
require_relative '../../error/source_parse_error'
require_relative '../../extraction/enumerate_source_files'
require_relative '../../extraction/locate_project'
require_relative 'metric_info'
Expand All @@ -26,15 +27,15 @@ def extract_file_info(source_file, relative_path: nil)
path = source_path(source_file)
identifier = (relative_path || path.basename.to_s).to_s.tr('\\', '/')
source = path.read
build_file_info(identifier, source)
result = SourceParseError.validate!(Prism.parse(source, filepath: path.to_s), path:)
build_file_info(identifier, source, result)
rescue SystemCallError => e
raise TechnicalError, "could not extract Ruby source metrics: #{e.message}"
end

def build_file_info(identifier, source)
parse_result = Prism.parse(source)
def build_file_info(identifier, source, parse_result)
visitor = SourceMetricsVisitor.new(identifier)
visitor.visit(parse_result.value) if parse_result.success?
visitor.visit(parse_result.value)
FileInfo.new(
path: identifier, lines_of_code: lines_of_code(source), **visitor_counts(visitor)
)
Expand Down
26 changes: 24 additions & 2 deletions scripts/check_dependency_analysis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,29 @@ def verify_project_exclusions(root, rule)
abort 'ignored file remained in metrics' unless remaining == ['app/api/request.rb']
end

def reject_malformed_source(rule)
rule.check
abort 'malformed source unexpectedly passed'
rescue ArchUnit::SourceParseError => e
diagnostic = e.diagnostics.first
abort 'parser location missing' unless diagnostic.path.end_with?('request.rb') &&
diagnostic.line.positive? && diagnostic.column.positive?
end

def verify_parse_diagnostics(request, rule)
File.write(request, "class Broken <\n")
ArchUnit.clear_graph_cache
reject_malformed_source(rule)
File.write(request, "path = '../retrieval/store'\nrequire_relative path\n")
ArchUnit.clear_graph_cache
end

def verify_rule_extensions(root, request, rule)
verify_rationale(rule)
verify_parse_diagnostics(request, rule)
verify_project_exclusions(root, rule)
end

# 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")
Expand All @@ -45,8 +68,7 @@ def verify_project_exclusions(root, rule)
abort 'local-string dependency missing' unless rule.check(refreshed).length == 1
ArchUnit.assert_passes(allowed)

verify_rationale(rule)
verify_project_exclusions(root, rule)
verify_rule_extensions(root, request, rule)
puts 'Installed dependency analysis, rule rationale, and project exclusions passed.'
ensure
ArchUnit.clear_graph_cache
Expand Down
26 changes: 26 additions & 0 deletions spec/common/extraction/parse_diagnostic_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

RSpec.describe ArchUnit::ParseDiagnostic do
it 'normalizes and freezes source data without freezing caller strings' do
path = +'app\\broken.rb'
message = +'unexpected token'
diagnostic = described_class.new(path:, line: 2, column: 3, message:)
path.replace('changed.rb')
message.replace('changed message')
expect(diagnostic.to_h).to eq(path: 'app/broken.rb', line: 2, column: 3,
message: 'unexpected token')
expect(diagnostic.path).to be_frozen
expect(diagnostic.message).to be_frozen
end

it 'rejects invalid text, coordinates, and malformed diagnostic collections' do
valid = { path: 'file.rb', line: 1, column: 1, message: 'syntax error' }
[{ path: nil }, { path: '' }, { message: nil }, { message: '' },
{ line: 0 }, { column: -1 }, { line: 1.5 }].each do |invalid|
expect { described_class.new(**valid, **invalid) }.to raise_error(ArgumentError)
end
[nil, [], ['not a diagnostic']].each do |invalid|
expect { ArchUnit::SourceParseError.new(invalid) }.to raise_error(ArgumentError)
end
end
end
8 changes: 4 additions & 4 deletions spec/extraction/extract_dependencies_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,14 @@ def create_file(relative_path, contents = '# fixture')
)
end

it 'skips malformed source files without losing dependencies from valid files' do
it 'rejects malformed source instead of returning only dependencies from valid files' do
create_file('lib/broken.rb', "require 'hidden'\ndef broken(\n")
create_file('lib/valid.rb', "require_relative 'support'\n")
create_file('lib/support.rb')

sources = described_class.extract_dependencies(@project_root).map(&:source)

expect(sources).to eq(['lib/valid.rb'])
expect do
described_class.extract_dependencies(@project_root)
end.to raise_error(ArchUnit::SourceParseError, /broken.rb/)
end

it 'returns an immutable edge collection' do
Expand Down
6 changes: 1 addition & 5 deletions spec/extraction/extract_graph_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,10 @@ def extract
described_class.extract_graph(@project_root)
end

it 'emits a self-edge for every Ruby source, including isolated and malformed files' do
it 'emits a self-edge for every valid Ruby source, including isolated files' do
create_file('lib/isolated.rb')
create_file('lib/broken.rb', "def broken(\n")

expect(extract).to contain_exactly(
ArchUnit::Edge.new(
source: 'lib/broken.rb', target: 'lib/broken.rb', external: false
),
ArchUnit::Edge.new(
source: 'lib/isolated.rb', target: 'lib/isolated.rb', external: false
)
Expand Down
5 changes: 2 additions & 3 deletions spec/extraction/extract_imports_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,10 @@ def self.load_dependency
expect(extract.map(&:module_name)).to eq(%w[json set])
end

it 'skips a file that Prism cannot parse' do
it 'rejects a file that Prism cannot parse' do
create_file('lib/example.rb', "require 'json'\ndef broken(\n")

expect(extract).to eq([])
expect(extract).to be_frozen
expect { extract }.to raise_error(ArchUnit::SourceParseError, /example.rb/)
end

it 'returns immutable import collections and values' do
Expand Down
Loading
Loading