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
36 changes: 35 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ jobs:
frontend/roslyn/samples/StaticHandlerViewModel.cs \
frontend/roslyn/samples/StaticEventEscapeViewModel.cs \
frontend/roslyn/samples/WhenAnyValueViewModel.cs \
frontend/roslyn/samples/DiCaptiveSample.cs \
frontend/roslyn/samples/SampleTypes.cs \
-o "$RUNNER_TEMP/facts.json"
cat "$RUNNER_TEMP/facts.json"
Expand Down Expand Up @@ -273,7 +274,40 @@ jobs:
if echo "$out" | grep -q "CleanStaticEventViewModel"; then
echo "FAIL: an unsubscribed (released) static-event subscription was wrongly reported"; exit 1
fi
echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) at the C# location"
# P-006 DI001 (captive dependency): the registration + constructor graph
# extracted from DiCaptiveSample.cs feeds ownlang/di.py. A singleton that
# captures a scoped service — directly, transitively through a transient,
# or through an interface registration — is flagged at the registration
# site; a singleton->singleton edge and the clean registrations stay silent.
echo "$out" | grep -q "DI001" \
|| { echo "FAIL: expected DI001 captive-dependency findings"; exit 1; }
echo "$out" | grep -q "singleton 'EmailSender' captures scoped service 'AppDbContext'" \
|| { echo "FAIL: expected the direct captive (singleton EmailSender -> scoped AppDbContext)"; exit 1; }
# the transitive capture must thread through the transient UnitOfWork.
echo "$out" | grep -q "ReportService -> UnitOfWork -> AppDbContext" \
|| { echo "FAIL: expected the transitive captive path via the transient UnitOfWork"; exit 1; }
# the interface registration (AddScoped<IRepo, Repo>) must map so the
# singleton consuming IRepo is caught.
echo "$out" | grep -q "singleton 'CacheService' captures scoped service 'IRepo'" \
|| { echo "FAIL: expected the interface-registration captive (CacheService -> IRepo)"; exit 1; }
# C# 12 primary-constructor injection (deps on the class declaration, not a
# ctor member) must be read too.
echo "$out" | grep -q "singleton 'PrimaryCtorService' captures scoped service 'AppDbContext'" \
|| { echo "FAIL: expected the primary-constructor captive (PrimaryCtorService -> AppDbContext)"; exit 1; }
echo "$out" | grep -q "DiCaptiveSample.cs" \
|| { echo "FAIL: expected the DI001 findings at the DiCaptiveSample.cs registration site"; exit 1; }
# NOT captive: singleton->singleton (Metrics->Clock), and PublicCtorOnly —
# DI resolves its public parameterless ctor, so the wider PRIVATE ctor's
# scoped dependency is never used. None of these may be flagged.
if echo "$out" | grep -qE "captures scoped service '(Clock|Metrics)'" \
|| echo "$out" | grep -q "'PublicCtorOnly'"; then
echo "FAIL: a singleton->singleton, public-ctor-only, or clean registration was wrongly flagged captive"; exit 1
fi
# exactly four captive dependencies (direct + transitive + interface + primary-ctor).
nd=$(echo "$out" | grep -cE "DiCaptiveSample\.cs:[0-9]+:.*\[DI001\]")
[ "$nd" = "4" ] \
|| { echo "FAIL: expected exactly 4 DI001 captive findings, got $nd"; exit 1; }
echo "OK: real C# -> facts -> OWN001 (subscription + timer + field + Subscribe + pool + local) + OWN014 (static-event region escape) + DI001 (captive dependency) at the C# location"
- name: Flow-sensitive local IDisposables (--flow-locals, P-016 B0b/B2)
run: |
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1
Expand Down
12 changes: 9 additions & 3 deletions docs/ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ architectural strictness, and the borrow-checker showcase):
1. `WPF001` — event/subscription `+=` without `-=` (the WPF spike; P-001 v0 ✅)
2. `WPF002` — `DispatcherTimer`/`Timer` `Tick`/`Elapsed` without stop/detach ✅
3. `OWN001` — `IDisposable` field the class `new`s but never disposes ✅
4. `DI001` — singleton captures a scoped dependency ✅ (core check built;
C# registration-graph extractor pending)
4. `DI001` — singleton captures a scoped dependency ✅ (core check + C#
registration-graph extractor built; end-to-end on real C#)
5. `POOL001` — `ArrayPool` buffer `Rent`ed but never `Return`ed ✅
(`POOL002` `Span`/view used after `Return` next)

Expand Down Expand Up @@ -142,6 +142,12 @@ architectural strictness, and the borrow-checker showcase):
migrating the *injected*-source subscription tier (today an honest OWN001
warning) once lifetime modelling can prove or refute those sources.
3. **DI lifetimes** — registration + constructor graph; captive dependency (P-006).
◑ *In progress* — DI001 lands end to end: the C# extractor builds the
registration + constructor graph from `Add{Singleton,Scoped,Transient}` (generic
and `typeof(...)` forms) and the core flags the captive — direct, transitive
through a transient, or through an interface registration — CI-validated on
`DiCaptiveSample.cs`. Remaining: DI002 (weak-ref), DI003 (transient-`IDisposable`
from root), and the consuming-constructor anchor.
4. **Pool/Span** — `Rent`/`Return`, borrowed views, return-invalidates-views,
known-bug replay corpus (P-007). The borrow checker on stage at full height.
5. **Effects** — `pure` / `use !Db` / `use !Log` / `use Clock`, layer policies
Expand Down Expand Up @@ -211,7 +217,7 @@ own scan. Label them as estimates wherever they appear.
| [P-003](proposals/P-003-lifetime-visualization.md) | Lifetime visualization (RustOwl-style) | horizon | draft |
| [P-004](proposals/P-004-wpf-lifetime-profile.md) | WPF / UI lifetime leak profile | P0 | in progress (WPF001–005 built) |
| [P-005](proposals/P-005-idisposable-ownership.md) | `IDisposable` ownership profile | P0 | draft |
| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001 core check built) |
| [P-006](proposals/P-006-di-lifetimes.md) | DI lifetime / captive dependency | P0 | in progress (DI001 end-to-end: core + extractor) |
| [P-007](proposals/P-007-arraypool-span.md) | ArrayPool / Span borrow-view | P1 | draft |
| [P-008](proposals/P-008-effects-and-resources.md) | Effects & resources (`Own.Effects`) | P1/P2 | draft |
| [P-009](proposals/P-009-nogc-regions.md) | No-GC / allocation-free regions | horizon | draft |
Expand Down
94 changes: 94 additions & 0 deletions docs/notes/di-captive-extractor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# DI001 end-to-end — the captive-dependency extractor (P-006)

The DI001 captive-dependency check has lived in the core (`ownlang/di.py`) for a
while, validated only on hand-written `services` facts. This slice gives it a
**C# front end**: the Roslyn extractor now builds the registration + constructor
graph from real code, so **DI001 fires end-to-end on C#** — no hand-authored
facts.

## The bug it catches

A **singleton** that depends on a **scoped** service captures that scoped instance
for the whole application lifetime — the classic ASP.NET Core *"Cannot consume
scoped service from singleton"* bug (a `DbContext` held by a singleton is the
canonical case: an open connection / request state promoted to process lifetime).
It is a deterministic property of the **registration graph**, which is exactly
OwnLang's lifetime ordering spelled in DI terms (`Transient ≲ Scoped < Singleton`).

The core rule (`di.py`, unchanged):

- `singleton -> scoped` — captive (the edge is the bug);
- `singleton -> transient -> scoped` — captive (a transient resolved by a
singleton is itself singleton-lived, and drags the scoped along);
- `singleton -> singleton -> scoped` — **not** reported here (the inner singleton
is the captor and is flagged on its own pass).

## What the extractor now reads

A purely **syntactic** pass over the same parsed trees (no SemanticModel needed —
in the spirit of the narrow frontend), in two steps:

1. **Constructor graph** — every class's widest **public** constructor's parameter
types (the dependency edges), including a C# 12 **primary constructor**
(`class Foo(Dep d)`, whose parameters live on the declaration, not a ctor
member). DI's default provider resolves through public ctors only, so a wider
non-public ctor's parameters are deliberately not counted (no false captive).
2. **Registrations** — every conventional `IServiceCollection` call:
`AddSingleton` / `AddScoped` / `AddTransient`, in the generic
`Add*<TService[, TImpl]>` form or the `Add*(typeof(TService)[, typeof(TImpl)])`
form.

Each registration emits one `services` fact — the shape the core already consumes:

```json
{"name": "EmailSender", "lifetime": "singleton", "deps": ["AppDbContext"],
"file": "DiCaptiveSample.cs", "line": 40}
```

`name` is the **service** type others inject (so an `AddScoped<IRepo, Repo>` keys
under `IRepo`, and a consumer injecting `IRepo` resolves to it); `deps` are the
**implementation**'s constructor parameter types; `file`/`line` anchor the finding
at the registration site.

### Non-goals stay silent, never guessed

Factory lambdas, reflection/assembly scanning, Scrutor, open generics and
config-driven wiring defeat a static graph. The extractor records what it *can*
read and treats the rest as an **unknown-dep node** (`deps: []`) — a node others
can still capture, but one that contributes no edges of its own. Per the P-006
non-goals: report the conventional shape, stay silent (not wrong) on the rest.

## Validation

- **Bridge-side, locally** — the `services` facts the extractor emits, fed through
the real core, produce exactly the four expected DI001s (direct, transitive,
interface, primary-ctor) and stay silent on the singleton→singleton edge and the
public-ctor-only service. (No local .NET SDK, so the C#→facts step itself is
validated in CI, like the rest of the frontend.)
- **End-to-end, in CI** — `frontend/roslyn/samples/DiCaptiveSample.cs` is wired
into the `wpf-extractor` job. The assertions pin: a direct capture
(`EmailSender -> AppDbContext`), a transitive one through a transient
(`ReportService -> UnitOfWork -> AppDbContext`), an interface-registration one
(`CacheService -> IRepo`), a C# 12 primary-constructor one
(`PrimaryCtorService -> AppDbContext`), **exactly four** findings, and silence on
`Metrics -> Clock` (singleton→singleton) and `PublicCtorOnly` (DI uses its public
parameterless ctor; the wider private ctor's scoped dep is never resolved).

## Why it matters for differentiation

The captive dependency is an **ASP.NET-specific lifetime contract** that
general-purpose analyzers don't model — the same complementary story as the
subscription-leak class the cross-tool oracle already documented (CodeQL and
Infer# cover the Dispose/RAII family; neither flags these). It is also a clean
reuse win: a whole new diagnostic class on real C# with **zero core changes** —
the frontend just produces the `services` facts the lifetime core already checks.

## Next (separate slices)

- **DI002** — a singleton capturing a scoped dependency *weakly*
(`WeakReference<Scoped>`): a warning, since a weak reference fixes retention but
not the lifetime-contract violation.
- **DI003** — a transient `IDisposable` resolved from the **root** provider, never
disposed until the app exits (a slow leak).
- Anchoring the finding at the **consuming constructor** as well as the
registration site (P-006 open question #1), with the capture path shown.
13 changes: 9 additions & 4 deletions docs/proposals/P-006-di-lifetimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@

- **Status:** in progress (P0 — clean lifetime model, little R&D, sells to
ASP.NET). DI001 captive-dependency check built in the core (`ownlang/di.py`)
over an OwnIR `services` registration graph, surfaced through the bridge with
hand-written facts + tests. Next: the C# extractor that builds the registration
graph from `services.Add{Singleton,Scoped,Transient}` + constructor injection
(CI-only, like the rest of the extractor).
over an OwnIR `services` registration graph, surfaced through the bridge. The
C# extractor now **builds that graph from real code** — `services.Add{Singleton,
Scoped,Transient}` (the generic `<TService[, TImpl]>` and `typeof(...)` forms)
plus each implementation's constructor parameters — so **DI001 fires end-to-end
on C#**, CI-validated on `frontend/roslyn/samples/DiCaptiveSample.cs` (direct,
transitive-via-transient, and interface-registration captures flagged;
singleton→singleton and clean registrations silent). See
[docs/notes/di-captive-extractor.md](../notes/di-captive-extractor.md). Next:
DI002 (weak-ref) and DI003 (transient-`IDisposable`-from-root).
- **Depends on:** `spec/Lifetimes.md` (the region-ordering model behind OWN014),
[P-001](P-001-csharp-extractor.md) (the C# seam). See
[`docs/ROADMAP.md`](../ROADMAP.md) (Milestone 3).
Expand Down
140 changes: 140 additions & 0 deletions frontend/roslyn/OwnSharp.Extractor/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,143 @@
|| t.EndsWith("Stream") || t.EndsWith("Reader") || t.EndsWith("Writer")
|| t.EndsWith("Subscription");

// --- P-006: DI registration + constructor graph (DI001 captive dependency) ---
// A syntactic pass over the same trees, independent of the event/disposable
// detectors: collect each class's constructor parameter types (the dependency
// graph), then each conventional IServiceCollection registration
// (Add{Singleton,Scoped,Transient}, the generic `<TService[, TImpl]>` form or the
// `typeof(...)` form) and emit `services` facts {name, lifetime, deps, file, line}
// that ownlang/di.py checks for captive dependencies. The registration's `name`
// is the SERVICE type others inject; `deps` are the IMPLEMENTATION's constructor
// parameter types. Factory/reflection/open-generic shapes we cannot read are
// recorded as unknown-dep nodes (deps: []) — silent, never guessed (P-006 scope).
static List<object> ExtractServices(List<(string file, SyntaxTree tree)> parsed)
{
// 1. class name -> its widest constructor's parameter type names (the DI ctor).
var ctorDeps = new Dictionary<string, List<string>>();
foreach (var (_, tree) in parsed)
foreach (var node in tree.GetRoot().DescendantNodes())
{
if (node is not ClassDeclarationSyntax cls)
continue;
// Candidate parameter lists: the C# 12 primary constructor (on the
// class declaration itself) plus the PUBLIC explicit constructors —
// take the widest. Primary-constructor injection (`class Foo(Dep d)`)
// has no ConstructorDeclarationSyntax member, so it must be read off
// the declaration or DI001 misses modern .NET 8 services; and the
// default IServiceProvider only uses public constructors, so a wider
// non-public ctor's parameters must not count as real deps (Codex).
ParameterListSyntax? widest = cls.ParameterList;
foreach (var m in cls.Members)
if (m is ConstructorDeclarationSyntax ctor
Comment on lines +748 to +749

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read primary constructors for service deps

When a registered service uses C# 12 primary-constructor injection, such as public sealed class EmailSender(AppDbContext db), there is no ConstructorDeclarationSyntax member; the parameters live on the class declaration itself. This loop leaves widest null and emits an empty dependency list, so DI001 misses singleton-to-scoped captures in current .NET 8/ASP.NET-style code.

Useful? React with 👍 / 👎.

&& IsPublicCtor(ctor.Modifiers)
&& (widest is null
|| ctor.ParameterList.Parameters.Count > widest.Parameters.Count))
widest = ctor.ParameterList;
var deps = new List<string>();
if (widest is not null)
foreach (var p in widest.Parameters)
{
var tn = p.Type is null ? null : DiTypeName(p.Type);
if (tn is not null)
deps.Add(tn);
}
ctorDeps[cls.Identifier.Text] = deps; // last decl wins (core dedups by name)
}

// 2. registrations -> service facts at the registration site.
var services = new List<object>();
foreach (var (file, tree) in parsed)
foreach (var node in tree.GetRoot().DescendantNodes())
{
if (node is not InvocationExpressionSyntax inv
|| inv.Expression is not MemberAccessExpressionSyntax ma)
continue;
var lifetime = RegistrationLifetime(ma.Name.Identifier.Text);
if (lifetime is null)
continue;
ResolveRegistration(ma.Name, inv.ArgumentList, out var service, out var impl);
if (service is null) // not a conventional typed registration -> skip
continue;
var deps = impl is not null && ctorDeps.TryGetValue(impl, out var d)
? d : new List<string>();
services.Add(new
{
name = service,
lifetime,
deps,
file,
line = node.GetLocation().GetLineSpan().StartLinePosition.Line + 1,
});
}
return services;
}

// The DI lifetime for an IServiceCollection registration method, or null when the
// method name is not one of the three conventional registrations.
static string? RegistrationLifetime(string method) => method switch
{
"AddSingleton" => "singleton",
"AddScoped" => "scoped",
"AddTransient" => "transient",
_ => null,
};

// Service + implementation type for a registration, from the generic type
// arguments (`Add*<TService[, TImpl]>`) or the `typeof(...)` arguments
// (`Add*(typeof(TService)[, typeof(TImpl)])`). A null service means it is not a
// typed registration we can read (a bare instance / factory) -> the caller skips.
static void ResolveRegistration(SimpleNameSyntax name, ArgumentListSyntax args,
out string? service, out string? impl)
{
service = null;
impl = null;
if (name is GenericNameSyntax gen)
{
var targs = gen.TypeArgumentList.Arguments;
if (targs.Count >= 1)
service = DiTypeName(targs[0]);
impl = targs.Count >= 2 ? DiTypeName(targs[1]) : service;
return;
}
string? first = null, second = null;
var seen = 0;
foreach (var a in args.Arguments)
if (a.Expression is TypeOfExpressionSyntax tof)
{
if (seen == 0) first = DiTypeName(tof.Type);
else if (seen == 1) second = DiTypeName(tof.Type);
seen++;
}
if (first is not null)
{
service = first;
impl = second ?? first;
}
}

// The simple (rightmost) name of a type, for matching a registration's service
// type to a constructor parameter's type. A generic name drops its arguments
// (`ILogger<T>` -> `ILogger`); a predefined type (`int`) yields null.
static string? DiTypeName(TypeSyntax t) => t switch
{
IdentifierNameSyntax id => id.Identifier.Text,
GenericNameSyntax g => g.Identifier.Text,
QualifiedNameSyntax q => DiTypeName(q.Right),
AliasQualifiedNameSyntax aq => DiTypeName(aq.Name),
_ => null,
};

// DI's default IServiceProvider resolves through PUBLIC constructors only — an
// explicit ctor with no access modifier defaults to private and DI never uses it.
static bool IsPublicCtor(SyntaxTokenList modifiers)
{
foreach (var m in modifiers)
if (m.IsKind(SyntaxKind.PublicKeyword))
return true;
return false;
}

var components = new List<object>();
// P-016 B0b/B2: per-method flow bodies (only when --flow-locals).
var flowFunctions = new List<object>();
Expand Down Expand Up @@ -760,7 +897,7 @@
.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
.Where(p => p.EndsWith(".dll", StringComparison.OrdinalIgnoreCase))
.ToList();
var refNames = new HashSet<string>(tpa.Select(Path.GetFileName), StringComparer.OrdinalIgnoreCase);

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check SARIF -> GitHub code scanning (dog-food)

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / C# leak extractor (Roslyn) -> OwnIR -> core

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.

Check warning on line 900 in frontend/roslyn/OwnSharp.Extractor/Program.cs

View workflow job for this annotation

GitHub Actions / own-check repo scan (github + msbuild) + composite action

Argument of type 'IEnumerable<string?>' cannot be used for parameter 'collection' of type 'IEnumerable<string>' in 'HashSet<string>.HashSet(IEnumerable<string> collection, IEqualityComparer<string>? comparer)' due to differences in the nullability of reference types.
var references = tpa.Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList();
// P-004 WPF profile: widen the reference set with assemblies named by the
// OWN_EXTRA_REF_DIRS env var (colon-separated dirs) — e.g. the WindowsDesktop ref
Expand Down Expand Up @@ -1141,6 +1278,9 @@
ownir_version = 0,
module = "Extracted",
components,
// P-006: the DI registration + ctor graph (empty when the scan has no
// Add{Singleton,Scoped,Transient} calls). ownlang/di.py turns it into DI001.
services = ExtractServices(parsed),
functions = flowFunctions,
stats = new
{
Expand Down
Loading
Loading