diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72c06c82..237ad88d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1608,6 +1608,74 @@ jobs: fi echo "OK: declared weak-subscribe wrapper = accepted release; += unaffected; own.toml plumbed; malformed config is a hard error" + # S0 (--fix-candidates, Part A): ADDITIVE fix-candidate metadata — a + # namespaced `fix` block on each `+=` subscription, component + # `qualified_name`/shape, and a top-level `fix_candidates_version`. With the + # flag OFF the facts are byte-identical (the extractor fact tests above run + # flag-off and would break otherwise); this step asserts the metadata is + # correct and that the off-run leaks none of it. + - name: S0 fix-candidates — extractor metadata (Part A) + run: | + fc=frontend/roslyn/samples/FixCandidatesSample.cs + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + "$fc" --fix-candidates -o "$RUNNER_TEMP/fc_on.json" + dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \ + "$fc" -o "$RUNNER_TEMP/fc_off.json" + python tests/check_fix_candidates_facts.py "$RUNNER_TEMP/fc_on.json" "$RUNNER_TEMP/fc_off.json" + # REAL byte parity: the flag-off output must equal the committed pre-S0 golden + # (generated by the base extractor at the S0 branch point) BYTE-for-byte -- a + # true comparison, not "no new keys". Regenerate the golden (documented in + # tests/goldens/README.md) only when an unrelated extractor change intentionally + # alters this sample's facts. + if ! diff -u tests/goldens/fix_candidates_off.golden.json "$RUNNER_TEMP/fc_off.json"; then + echo "FAIL: flag-off facts drifted from the pre-S0 golden (byte parity broken)"; exit 1 + fi + echo "OK: fix-candidate metadata correct; flag-off is byte-identical to the pre-S0 golden" + + # S0 Part B: the `own-fix subscriptions candidates` collector turns the fix + # metadata into a deterministic candidates.json (analysis-only). Reuses fc_on.json. + - name: S0 fix-candidates — own-fix collector (Part B) + run: | + on="$RUNNER_TEMP/fc_on.json" + printf '[weak-subscription]\nsubscribe = ["WeakEvents.AddPropertyChanged"]\n' > "$RUNNER_TEMP/fix.toml" + # A leaky INotifyPropertyChanged subscription -> one candidate, convert_acquire allowed. + python -m ownlang own-fix subscriptions candidates "$on" \ + --config "$RUNNER_TEMP/fix.toml" \ + --class Own.Samples.FixCandidates.InpcNoTeardown \ + --output "$RUNNER_TEMP/cand.json" --root . + python -c " + import json, sys + d = json.load(open(sys.argv[1])) + assert d['target_api'] == {'subscribe': 'WeakEvents.AddPropertyChanged'}, d['target_api'] + assert len(d['candidates']) == 1, d['candidates'] + c = d['candidates'][0] + assert c['allowed_actions'] == ['convert_acquire', 'manual_review'], c['allowed_actions'] + assert c['event_contract'] == 'inotify_property_changed' + assert c['finding_id'].startswith('OWN001:sha256:'), c['finding_id'] + assert d['source_files'][0]['sha256'].startswith('sha256:') + assert d['selection']['constraints']['max_types_changed'] == 1 + print('OK: candidate bundle well-formed') + " "$RUNNER_TEMP/cand.json" + # A name_only event -> manual_review only. + python -m ownlang own-fix subscriptions candidates "$on" \ + --config "$RUNNER_TEMP/fix.toml" \ + --class Own.Samples.FixCandidates.NameOnlySubscriber \ + --output "$RUNNER_TEMP/cand2.json" --root . + python -c "import json,sys; a=json.load(open(sys.argv[1]))['candidates'][0]['allowed_actions']; sys.exit(0 if a==['manual_review'] else 1)" "$RUNNER_TEMP/cand2.json" \ + || { echo "FAIL: name_only should be manual_review only"; exit 1; } + # A nested class -> hard error. + if python -m ownlang own-fix subscriptions candidates "$on" --config "$RUNNER_TEMP/fix.toml" \ + --class Own.Samples.FixCandidates.OuterWithNested.Nested --output "$RUNNER_TEMP/x.json" --root . 2>/dev/null; then + echo "FAIL: a nested class must be refused"; exit 1 + fi + # An unknown finding-id -> hard error. + if python -m ownlang own-fix subscriptions candidates "$on" --config "$RUNNER_TEMP/fix.toml" \ + --class Own.Samples.FixCandidates.InpcNoTeardown --finding-id OWN001:sha256:deadbeef \ + --output "$RUNNER_TEMP/x.json" --root . 2>/dev/null; then + echo "FAIL: an unknown finding-id must be refused"; exit 1 + fi + echo "OK: own-fix collector — bundle, permission tiering, and hard rejections" + # The OwnTS frontend spike (P-020 Own.React): the SAME OwnIR seam, fed from a # React .tsx instead of C#. Two analyses over the one core: (1) a useEffect # acquire (timer / subscribe / listener) with no cleanup return is the core's diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index d505fb7f..83d4133e 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -61,6 +61,14 @@ // (e.g. to run only the disposable/pool detectors); it is the first instance of // the broader check-selection surface tracked in P-015. bool emitEvents = true; +// --fix-candidates (S0, internal, default off): emit ADDITIVE fix-candidate metadata +// for the `own-fix subscriptions candidates` collector — a namespaced `fix` block on +// each eligible `+=` subscription fact, a `qualified_name` + type-shape metadata on the +// component, and a top-level `fix_candidates_version`. STRICTLY additive: with the flag +// OFF the facts JSON is byte-for-byte identical (no `fix`, no `qualified_name`, no +// `fix_candidates_version`), and `ownir_version` is unchanged either way. It changes NO +// existing field and NOT the `released` computation — only new optional metadata. +bool emitFixCandidates = false; // --flow-locals (P-016 B0b/B2, EXPERIMENTAL, default off): emit per-method flow // facts for non-escaping local IDisposables (acquire/use/release/if/return over a // CFG) so the core checks them path-sensitively (OWN001/002/003). Supersedes the @@ -128,6 +136,9 @@ events bind to real symbols instead of OWN050 (repeatable) --config own.toml, not by hand; own.toml is the public surface. --no-project-refs don't auto-add a .csproj/.sln project's bin/ output to the references --no-event-leaks skip event-subscription detection (run only disposable/pool detectors) + --fix-candidates emit additive S0 fix-candidate metadata (namespaced `fix` block + + component `qualified_name`/shape + top-level `fix_candidates_version`); + facts are byte-identical without it, `ownir_version` unchanged --flow-locals path-sensitive flow analysis of non-escaping local IDisposables --stats print flow-locals coverage (requires --flow-locals) --body-throw-edges treat escaping body-level may-throw as a dispose-on-throw point (needs --flow-locals) @@ -159,6 +170,7 @@ events bind to real symbols instead of OWN050 (repeatable) } else if (args0[i] == "--no-project-refs") noProjectRefs = true; else if (args0[i] == "--no-event-leaks") emitEvents = false; + else if (args0[i] == "--fix-candidates") emitFixCandidates = true; else if (args0[i] == "--flow-locals") flowLocals = true; else if (args0[i] == "--body-throw-edges") BodyThrowEdges = true; else if (args0[i] == "--stats") reportStats = true; @@ -521,6 +533,271 @@ static ExpressionSyntax NormalizeHandler(ExpressionSyntax e) static int LineOf(SyntaxNode node) => node.GetLocation().GetLineSpan().StartLinePosition.Line + 1; +// ---- S0 fix-candidate metadata (--fix-candidates only; strictly additive) ---- + +// The one documented FQN format for `qualified_name` and every symbol identity the +// fix block emits: fully-qualified with namespaces + nested-type path, no `global::`, +// generic type parameters included, special types spelled (`int`, not `System.Int32`). +static SymbolDisplayFormat FixFqnFormat() => new SymbolDisplayFormat( + globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted, + typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces, + genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters, + memberOptions: SymbolDisplayMemberOptions.IncludeContainingType | SymbolDisplayMemberOptions.IncludeParameters, + // IncludeParamsRefOut so `M(T)` and `M(ref T)` — legal C# overloads — get DISTINCT + // signatures; without it the enclosing-member component of a finding identity could + // collide across ref/value overloads. + parameterOptions: SymbolDisplayParameterOptions.IncludeType | SymbolDisplayParameterOptions.IncludeParamsRefOut, + miscellaneousOptions: SymbolDisplayMiscellaneousOptions.UseSpecialTypes); + +// Whitespace-normalized syntax fingerprint (collapse runs of whitespace to single +// spaces, trim) — used as the identity of a lambda handler / an unresolved receiver, +// where no symbol exists to name it. Deterministic; no regex. +static string FixNormWs(string s) => + string.Join(" ", s.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)); + +// A full Roslyn TextSpan for `node`: absolute UTF-16 (start,length) for a future +// rewriter, plus 1-based line/column for humans and diagnostics UI. +static object FixSpanOf(SyntaxNode node) +{ + var span = node.Span; + var ls = node.GetLocation().GetLineSpan(); + return new + { + start = span.Start, + length = span.Length, + start_line = ls.StartLinePosition.Line + 1, + start_column = ls.StartLinePosition.Character + 1, + end_line = ls.EndLinePosition.Line + 1, + end_column = ls.EndLinePosition.Character + 1, + }; +} + +// The receiver expression of an event access (`source` in `source.PropertyChanged`), +// or null when the event is named bare (`Changed += h`, an implicit `this`). +static ExpressionSyntax? FixEventReceiver(ExpressionSyntax eventAccess) => + eventAccess is MemberAccessExpressionSyntax m ? m.Expression : null; + +// Semantic INotifyPropertyChanged classification — NEVER by the name alone. Only the +// actual `INotifyPropertyChanged.PropertyChanged` member, or a symbol that provably +// implements it, is `inotify_property_changed`; a same-named event on an unrelated +// type is `name_only`; anything else is `other`. (The `unresolved` category is for the +// unresolved-subscription lane where no event symbol binds, not reached here.) +static string FixEventContract(IEventSymbol ev, Compilation compilation) +{ + var inpc = compilation.GetTypeByMetadataName("System.ComponentModel.INotifyPropertyChanged"); + var inpcEvent = inpc?.GetMembers("PropertyChanged").OfType().FirstOrDefault(); + if (inpcEvent is not null) + { + if (SymbolEqualityComparer.Default.Equals(ev.OriginalDefinition, inpcEvent)) + return "inotify_property_changed"; + var impl = ev.ContainingType?.FindImplementationForInterfaceMember(inpcEvent); + if (impl is not null && SymbolEqualityComparer.Default.Equals(impl, ev)) + return "inotify_property_changed"; + } + return ev.Name == "PropertyChanged" ? "name_only" : "other"; +} + +// Best-effort "did this file reach the extractor already generated?" — a name marker +// (`.g.cs`/`.g.i.cs`/`.designer.cs`/`.generated.cs`) or an `` header. +static bool FixIsGeneratedFile(SyntaxTree tree, string file) +{ + var lower = file.Replace('\\', '/').ToLowerInvariant(); + var name = lower.Substring(lower.LastIndexOf('/') + 1); + if (name.EndsWith(".g.cs", StringComparison.Ordinal) + || name.EndsWith(".g.i.cs", StringComparison.Ordinal) + || name.EndsWith(".designer.cs", StringComparison.Ordinal) + || name.EndsWith(".generated.cs", StringComparison.Ordinal)) + return true; + foreach (var tr in tree.GetRoot().GetFirstToken(includeZeroWidth: true).LeadingTrivia) + if (tr.IsKind(SyntaxKind.SingleLineCommentTrivia) || tr.IsKind(SyntaxKind.MultiLineCommentTrivia)) + { + var t = tr.ToString().ToLowerInvariant(); + if (t.Contains(" + n is BaseMethodDeclarationSyntax or AccessorDeclarationSyntax + or PropertyDeclarationSyntax or EventDeclarationSyntax or IndexerDeclarationSyntax); + if (member is null) + return ""; + var sym = model.GetDeclaredSymbol(member); + return sym?.ToDisplayString(FixFqnFormat()) ?? FixNormWs(member.ToString()); +} + +// Strip enclosing parentheses so `(x)` classifies like `x`. +static ExpressionSyntax FixStripParens(ExpressionSyntax e) => + e is ParenthesizedExpressionSyntax p ? FixStripParens(p.Expression) : e; + +// Conservative RECEIVER (event-source) identity. Only a receiver that names a +// definitely-stable instance may ground an `exact` teardown; a receiver whose runtime +// instance is computed per evaluation must not — two `GetPublisher()` calls, or a +// property getter, can return different objects even though the SYMBOL is identical. +// "stable_symbol": `this` / local / parameter / instance field of `this` / static +// field — the same syntactic reference is the same instance; +// "computed": invocation / property / indexer / conditional access / a field of +// some OTHER value (`a.Publisher`) — instance identity not proven; +// "unresolved": no symbol bound. +static (string Kind, ISymbol? Sym) FixReceiverIdentity( + ExpressionSyntax? recv, SemanticModel model, INamedTypeSymbol? clsSymbol) +{ + if (recv is null) + return ("stable_symbol", clsSymbol); // bare event => implicit `this` + recv = FixStripParens(recv); + if (recv is ThisExpressionSyntax) + return ("stable_symbol", clsSymbol); + var sym = model.GetSymbolInfo(recv).Symbol; + switch (recv) + { + case IdentifierNameSyntax: + // bare identifier: local / parameter / instance-field-of-`this` / static field + if (sym is ILocalSymbol or IParameterSymbol or IFieldSymbol) + return ("stable_symbol", sym); + return sym is null ? ("unresolved", null) : ("computed", sym); + case MemberAccessExpressionSyntax ma: + // only `this._field` or a static field is a proven-stable instance; a field + // of another value (`a.Publisher`) or a property/method is computed. + if (sym is IFieldSymbol f + && (f.IsStatic || FixStripParens(ma.Expression) is ThisExpressionSyntax)) + return ("stable_symbol", sym); + return sym is null ? ("unresolved", null) : ("computed", sym); + default: + return sym is null ? ("unresolved", null) : ("computed", sym); + } +} + +// Conservative HANDLER identity. A method SYMBOL is not a delegate identity, and a +// storage SYMBOL is not its (mutable) value — so `stable_symbol` is limited to the two +// forms whose delegate is provably fixed for the class WITHOUT dataflow: +// * a STATIC method group (null target); or +// * an INSTANCE method group on `this` — bare `OnChanged` or `this.OnChanged`. +// A method group on any OTHER receiver (`left.OnChanged`) binds to that receiver's +// instance — `left.OnChanged` and `right.OnChanged` are the same IMethodSymbol but +// different delegates. A delegate held in a local / parameter / field / property is a +// storage location that can be reassigned between the `+=` and the `-=` (same symbol, +// different value). A lambda is a fresh delegate each evaluation. All of these are +// `computed` — none may ground an `exact` (proving them stable is dataflow, out of S0). +static (string Kind, ISymbol? Sym) FixHandlerIdentity(ExpressionSyntax handler, SemanticModel model) +{ + var nh = NormalizeHandler(handler); + if (nh is AnonymousFunctionExpressionSyntax) + return ("computed", null); + var info = model.GetSymbolInfo(nh); + var sym = info.Symbol ?? info.CandidateSymbols.FirstOrDefault(); + if (sym is IMethodSymbol method) + { + if (method.IsStatic) + return ("stable_symbol", sym); // static method group: null target + if (nh is IdentifierNameSyntax) + return ("stable_symbol", sym); // bare `OnChanged` => implicit `this` + if (nh is MemberAccessExpressionSyntax ma + && FixStripParens(ma.Expression) is ThisExpressionSyntax) + return ("stable_symbol", sym); // `this.OnChanged` + return ("computed", sym); // method group on some other receiver instance + } + // A local / parameter / field / property / indexer delegate: a storage location or a + // computed value, not a proven-immutable delegate. + return sym is null ? ("unresolved", null) : ("computed", sym); +} + +// Build the namespaced `fix` block for one eligible `+=` acquire `a` (event `ev`). An +// `exact` teardown requires a single `-=` site whose event symbol, RECEIVER identity, and +// HANDLER identity all match by STABLE symbol — a computed/unresolved identity, or a +// text-only agreement, tops out at `ambiguous`. The occurrence ordinal is keyed by the +// FULL identity INCLUDING the enclosing member (so a subscription added to a different +// member never shifts an existing one's ordinal). Teardown scan is scoped to `a`'s +// IMMEDIATE containing type. +static object FixBuildSubscriptionFix( + AssignmentExpressionSyntax a, IEventSymbol ev, SemanticModel model, Compilation compilation, + INamedTypeSymbol? clsSymbol, ClassDeclarationSyntax cls, Dictionary occ) +{ + var eventId = ev.OriginalDefinition.ToDisplayString(FixFqnFormat()); + var contract = FixEventContract(ev, compilation); + var enclosing = FixEnclosingMemberSignature(a, model); + + var recv = FixEventReceiver(a.Left); + var sourceText = recv is null ? "this" : recv.ToString(); + var (srcKind, srcSym) = FixReceiverIdentity(recv, model, clsSymbol); + var sourceId = srcKind == "stable_symbol" && srcSym is not null + ? srcSym.ToDisplayString(FixFqnFormat()) + : FixNormWs(sourceText); + + var handlerText = NormalizeHandler(a.Right).ToString(); + var (hKind, hSym) = FixHandlerIdentity(a.Right, model); + var handlerId = hKind == "stable_symbol" && hSym is not null + ? hSym.ToDisplayString(FixFqnFormat()) + : FixNormWs(handlerText); + + var acquireStable = srcKind == "stable_symbol" && hKind == "stable_symbol"; + + var candidates = new List(); + var anyNonStable = false; + foreach (var t in cls.DescendantNodes().OfType()) + { + if (!t.IsKind(SyntaxKind.SubtractAssignmentExpression)) + continue; + if (!ReferenceEquals(t.FirstAncestorOrSelf(), cls)) + continue; // nested-type boundary: only this immediate type's teardown sites + if (model.GetSymbolInfo(t.Left).Symbol is not IEventSymbol tev + || !SymbolEqualityComparer.Default.Equals(tev, ev)) + continue; // must be the same event symbol + var tRecv = FixEventReceiver(t.Left); + var tSourceText = tRecv is null ? "this" : tRecv.ToString(); + var (tSrcKind, tSrcSym) = FixReceiverIdentity(tRecv, model, clsSymbol); + var tHandlerText = NormalizeHandler(t.Right).ToString(); + var (tHKind, tHSym) = FixHandlerIdentity(t.Right, model); + + var srcStableEq = srcKind == "stable_symbol" && tSrcKind == "stable_symbol" + && SymbolEqualityComparer.Default.Equals(srcSym, tSrcSym); + var hStableEq = hKind == "stable_symbol" && tHKind == "stable_symbol" + && SymbolEqualityComparer.Default.Equals(hSym, tHSym); + var srcAgree = srcStableEq || FixNormWs(sourceText) == FixNormWs(tSourceText); + var hAgree = hStableEq || FixNormWs(handlerText) == FixNormWs(tHandlerText); + if (!srcAgree || !hAgree) + continue; + + var stableMatch = srcStableEq && hStableEq; + if (!stableMatch) + anyNonStable = true; // a text-only match cannot ground an `exact` + candidates.Add(new + { + source = tSourceText, + handler = tHandlerText, + match = stableMatch ? "stable" : "text", + span = FixSpanOf(t), + }); + } + var status = candidates.Count == 0 ? "none" + : (candidates.Count == 1 && acquireStable && !anyNonStable) ? "exact" + : "ambiguous"; + + var key = $"{enclosing}\0{eventId}\0{sourceId}\0{handlerId}"; + var ordinal = occ.TryGetValue(key, out var seen) ? seen : 0; + occ[key] = ordinal + 1; + + return new + { + enclosing_member = enclosing, + event_identity = eventId, + event_contract = contract, + source_identity = sourceId, + source_identity_kind = srcKind, + handler_identity = handlerId, + handler_identity_kind = hKind, + occurrence_ordinal = ordinal, + span = FixSpanOf(a), + teardown = new { status, candidates }, + }; +} + // The receiver of `target.Member` ("_timer" for `_timer.Tick`), or null when the // left side is a bare identifier (`Changed += h`). static string? Receiver(ExpressionSyntax expr) => @@ -4717,6 +4994,9 @@ or ImplicitObjectCreationExpressionSyntax var clsIsBehavior = IsBehaviorSubscriber(cls); var subs = new List(); + // S0 (--fix-candidates): per-identity occurrence counter, scoped to THIS type, + // so two otherwise-identical acquires get distinct finding identities. + var fixOcc = new Dictionary(StringComparer.Ordinal); foreach (var a in assigns) { if (!emitEvents || !a.IsKind(SyntaxKind.AddAssignmentExpression)) @@ -4871,17 +5151,51 @@ or ImplicitObjectCreationExpressionSyntax // unknown source stays a token `subscription` (OWN001, // severity-tiered); timers are their own kind. (P-004 WPF005; // see ownlang/ownir.py `capture`.) + // S0: attach a `fix` block only for a non-timer `+=` whose IMMEDIATE + // containing type is THIS class (a nested-class acquire is fixed by that + // nested class's own iteration, never the outer's). Purely additive; the + // no-fix shapes below are byte-for-byte the pre-S0 facts. + var ownAcquire = ReferenceEquals(a.FirstAncestorOrSelf(), cls); + var wantFix = emitFixCandidates && !isTimer && ownAcquire; if (returnedFresh) + { + if (wantFix) + subs.Add(new + { + @event = a.Left.ToString(), + handler = a.Right.ToString(), + line = LineOf(a.Left), + released, + resource = "subscription", + source, + lambda = IsLambdaHandler(a.Right), + source_provenance = "returned_fresh", + fix = FixBuildSubscriptionFix(a, ev, model, compilation, clsSymbol, cls, fixOcc), + }); + else + subs.Add(new + { + @event = a.Left.ToString(), + handler = a.Right.ToString(), + line = LineOf(a.Left), + released, + resource = "subscription", + source, + lambda = IsLambdaHandler(a.Right), + source_provenance = "returned_fresh", + }); + } + else if (wantFix) subs.Add(new { @event = a.Left.ToString(), handler = a.Right.ToString(), line = LineOf(a.Left), released, - resource = "subscription", + resource = source == "static" ? "capture" : "subscription", source, lambda = IsLambdaHandler(a.Right), - source_provenance = "returned_fresh", + fix = FixBuildSubscriptionFix(a, ev, model, compilation, clsSymbol, cls, fixOcc), }); else subs.Add(new @@ -5841,29 +6155,64 @@ or ImplicitObjectCreationExpressionSyntax } init } if (subs.Count > 0) - components.Add(new { name = cls.Identifier.Text, file, subscriptions = subs }); + { + if (emitFixCandidates) + components.Add(new + { + name = cls.Identifier.Text, + qualified_name = clsSymbol?.ToDisplayString(FixFqnFormat()) ?? cls.Identifier.Text, + is_partial = clsSymbol is not null + ? clsSymbol.DeclaringSyntaxReferences.Length > 1 + || cls.Modifiers.Any(SyntaxKind.PartialKeyword) + : cls.Modifiers.Any(SyntaxKind.PartialKeyword), + is_nested = clsSymbol?.ContainingType is not null, + declaration_count = clsSymbol?.DeclaringSyntaxReferences.Length ?? 1, + is_generated = FixIsGeneratedFile(tree, file), + file, + subscriptions = subs, + }); + else + components.Add(new { name = cls.Identifier.Text, file, subscriptions = subs }); + } } } // ownir_version stamps the fact-schema vocabulary; the Python core rejects a // mismatch loudly (ownlang/ownir.py OWNIR_VERSION) rather than mis-reading facts. // `stats` is additive coverage metadata — the core's load() ignores unknown keys. -var facts = new +// 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. +var factServices = ExtractServices(parsed); +var factStats = new { - 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 - { - methods_with_local = statMethodsWithLocal, - methods_flow_analysed = statMethodsAnalysed, - methods_skipped_unmodelled = statMethodsSkipped, - }, + methods_with_local = statMethodsWithLocal, + methods_flow_analysed = statMethodsAnalysed, + methods_skipped_unmodelled = statMethodsSkipped, }; +// `fix_candidates_version` is a top-level ADDITIVE metadata field, present ONLY under +// --fix-candidates; `ownir_version` stays 0 (the fact-schema vocabulary is unchanged — +// no new resource-kind or analysis-routing value). Without the flag the object is +// byte-for-byte the pre-S0 shape. +object facts = emitFixCandidates + ? new + { + ownir_version = 0, + fix_candidates_version = 1, + module = "Extracted", + components, + services = factServices, + functions = flowFunctions, + stats = factStats, + } + : new + { + ownir_version = 0, + module = "Extracted", + components, + services = factServices, + functions = flowFunctions, + stats = factStats, + }; var json = JsonSerializer.Serialize(facts, new JsonSerializerOptions { WriteIndented = true }); if (reportStats) diff --git a/frontend/roslyn/samples/FixCandidatesSample.cs b/frontend/roslyn/samples/FixCandidatesSample.cs new file mode 100644 index 00000000..4c8a6927 --- /dev/null +++ b/frontend/roslyn/samples/FixCandidatesSample.cs @@ -0,0 +1,262 @@ +// S0 fix-candidate metadata sample. Scanned WITH `--fix-candidates`; the +// `fix` block on each `+=` subscription fact is asserted by +// tests/check_fix_candidates_facts.py. Every case here is deliberate: +// * INPC contract is SEMANTIC (implements INotifyPropertyChanged), not +// name-matching -- FakePub's same-named event must classify name_only. +// * teardown is symbol-based: none / exact (one proven -=) / ambiguous (>1). +// * a nested class's subscription must NOT get a fix block in the OUTER +// component (it is fixed by the nested type's own iteration). +using System; +using System.ComponentModel; + +namespace Own.Samples.FixCandidates +{ + // A genuine INotifyPropertyChanged publisher. + public interface IPub : INotifyPropertyChanged { } + + // INPC subscription with an EXACT teardown (ctor +=, Dispose -=). + public sealed class InpcExactTeardown + { + private readonly IPub _pub; + public InpcExactTeardown(IPub pub) + { + _pub = pub; + _pub.PropertyChanged += OnChanged; + } + + public void Dispose() => _pub.PropertyChanged -= OnChanged; + private void OnChanged(object sender, PropertyChangedEventArgs e) { } + } + + // INPC subscription, NO teardown -> status none. + public sealed class InpcNoTeardown + { + public InpcNoTeardown(IPub pub) => pub.PropertyChanged += OnChanged; + private void OnChanged(object sender, PropertyChangedEventArgs e) { } + } + + // Two -= for the same acquire -> ambiguous (do not guess the lifecycle). + public sealed class InpcAmbiguousTeardown + { + private readonly IPub _pub; + public InpcAmbiguousTeardown(IPub pub) + { + _pub = pub; + _pub.PropertyChanged += OnChanged; + } + + public void Detach1() => _pub.PropertyChanged -= OnChanged; + public void Detach2() => _pub.PropertyChanged -= OnChanged; + private void OnChanged(object sender, PropertyChangedEventArgs e) { } + } + + // An event NAMED PropertyChanged but NOT INotifyPropertyChanged -> name_only. + public class FakePub + { + public event EventHandler PropertyChanged; + } + + public sealed class NameOnlySubscriber + { + public NameOnlySubscriber(FakePub p) => p.PropertyChanged += OnChanged; + private void OnChanged(object sender, EventArgs e) { } + } + + // An unrelated event -> other. + public class ClickPub + { + public event EventHandler Clicked; + } + + public sealed class OtherEventSubscriber + { + public OtherEventSubscriber(ClickPub p) => p.Clicked += OnClick; + private void OnClick(object sender, EventArgs e) { } + } + + // Two subscriptions on ONE physical line -> a single `line` cannot tell them + // apart, but their full spans (start/length) must differ. + public sealed class TwoOnOneLine + { + public TwoOnOneLine(IPub a, IPub b) { a.PropertyChanged += OnA; b.PropertyChanged += OnB; } + private void OnA(object s, PropertyChangedEventArgs e) { } + private void OnB(object s, PropertyChangedEventArgs e) { } + } + + // Wrapped delegate creation on the +=; bare method group on the -=. The + // handler identity must NORMALIZE so the teardown is still exact. + public sealed class WrappedDelegate + { + private readonly IPub _pub; + public WrappedDelegate(IPub pub) + { + _pub = pub; + _pub.PropertyChanged += new PropertyChangedEventHandler(OnChanged); + } + + public void Dispose() => _pub.PropertyChanged -= OnChanged; + private void OnChanged(object sender, PropertyChangedEventArgs e) { } + } + + // A nested class's subscription must not appear as a fix candidate on the + // OUTER component; the outer's own subscription still does. + public sealed class OuterWithNested + { + public OuterWithNested(IPub pub) => pub.PropertyChanged += OnOuter; + private void OnOuter(object s, PropertyChangedEventArgs e) { } + + public sealed class Nested + { + public Nested(IPub pub) => pub.PropertyChanged += OnNested; + private void OnNested(object s, PropertyChangedEventArgs e) { } + } + } + + // --- Blocker-1 regressions: a computed receiver/handler must NOT be exact --- + + // Receiver is an INVOCATION: two GetPublisher() calls can return different + // instances even though the method symbol is identical -> ambiguous, not exact. + public sealed class ComputedReceiverInvocation + { + private IPub GetPublisher() => null!; + + public ComputedReceiverInvocation() => GetPublisher().PropertyChanged += OnChanged; + public void Dispose() => GetPublisher().PropertyChanged -= OnChanged; + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // Receiver is a PROPERTY: the getter may return different instances -> ambiguous. + public sealed class ComputedReceiverProperty + { + private IPub Pub => null!; + + public ComputedReceiverProperty() => Pub.PropertyChanged += OnChanged; + public void Dispose() => Pub.PropertyChanged -= OnChanged; + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + public sealed class Holder + { + public IPub Publisher = null!; + } + + // Different ROOT objects, same final field member: `_a.Publisher` != `_b.Publisher` + // as instances -> the -= is not even a candidate -> none (certainly not exact). + public sealed class DifferentRoots + { + private readonly Holder _a; + private readonly Holder _b; + + public DifferentRoots(Holder a, Holder b) + { + _a = a; + _b = b; + _a.Publisher.PropertyChanged += OnChanged; + } + + public void Dispose() => _b.Publisher.PropertyChanged -= OnChanged; + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // Handler is a PROPERTY returning a delegate: not stable even though both += and -= + // resolve to the same IPropertySymbol -> ambiguous. + public sealed class ComputedHandler + { + private readonly IPub _pub; + private PropertyChangedEventHandler H => (_, __) => { }; + + public ComputedHandler(IPub pub) + { + _pub = pub; + _pub.PropertyChanged += H; + } + + public void Dispose() => _pub.PropertyChanged -= H; + } + + // --- Blocker-2 regressions: occurrence ordinal is scoped by enclosing member --- + + // The SAME identity tuple in two different members -> each ordinal 0. + public sealed class OrdinalAcrossMembers + { + private readonly IPub _pub; + + public OrdinalAcrossMembers(IPub pub) + { + _pub = pub; + _pub.PropertyChanged += OnChanged; + } + + public void Reattach() => _pub.PropertyChanged += OnChanged; + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // Two identical acquires in ONE member -> ordinals 0 and 1. + public sealed class OrdinalWithinMember + { + public OrdinalWithinMember(IPub pub) + { + pub.PropertyChanged += OnChanged; + pub.PropertyChanged += OnChanged; + } + + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // Value vs ref overload -> DISTINCT enclosing_member signatures (IncludeParamsRefOut). + public sealed class RefOverloadEnclosing + { + private readonly IPub _pub; + + public RefOverloadEnclosing(IPub pub) => _pub = pub; + public void Attach(int x) => _pub.PropertyChanged += OnChanged; + public void Attach(ref int x) => _pub.PropertyChanged += OnChanged; + private void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // --- Blocker-1 (handler half): a method symbol is not a delegate identity --- + + public sealed class Sibling + { + public void OnChanged(object s, PropertyChangedEventArgs e) { } + } + + // Method group on a DIFFERENT receiver instance: `_left.OnChanged` and + // `_right.OnChanged` are the SAME IMethodSymbol but different delegates -> not exact. + public sealed class HandlerDifferentTarget + { + private readonly IPub _pub; + private readonly Sibling _left; + private readonly Sibling _right; + + public HandlerDifferentTarget(IPub pub, Sibling left, Sibling right) + { + _pub = pub; + _left = left; + _right = right; + _pub.PropertyChanged += _left.OnChanged; + } + + public void Dispose() => _pub.PropertyChanged -= _right.OnChanged; + } + + // Delegate held in a FIELD reassigned between += and -=: same IFieldSymbol, different + // delegate values -> not exact. + public sealed class HandlerReassignedField + { + private readonly IPub _pub; + private PropertyChangedEventHandler _handler; + + public HandlerReassignedField(IPub pub) + { + _pub = pub; + _handler = OnFirst; + _pub.PropertyChanged += _handler; + _handler = OnSecond; + _pub.PropertyChanged -= _handler; + } + + private void OnFirst(object s, PropertyChangedEventArgs e) { } + private void OnSecond(object s, PropertyChangedEventArgs e) { } + } +} diff --git a/ownlang/__main__.py b/ownlang/__main__.py index 124ac4dc..73112fa5 100644 --- a/ownlang/__main__.py +++ b/ownlang/__main__.py @@ -403,6 +403,95 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", return 1 if leaks else 0 +def cmd_own_fix(rest: list[str]) -> int: + """`own-fix subscriptions candidates --config + --class [--finding-id ]... --output [--root ]`.""" + import json + + usage = ( + "usage: python -m ownlang own-fix subscriptions candidates " + "--config --class [--finding-id ]... " + "--output [--root ]" + ) + if len(rest) < 2 or rest[0] != "subscriptions" or rest[1] != "candidates": + print(usage, file=sys.stderr) + return 2 + + args = rest[2:] + facts_path: str | None = None + config_path: str | None = None + class_fqn: str | None = None + output: str | None = None + root = "." + finding_ids: list[str] = [] + i = 0 + while i < len(args): + a = args[i] + if a.startswith("--"): + if i + 1 >= len(args): + print(f"own-fix: {a} requires a value", file=sys.stderr) + return 2 + value = args[i + 1] + i += 2 + if a == "--config": + config_path = value + elif a == "--class": + class_fqn = value + elif a == "--output": + output = value + elif a == "--root": + root = value + elif a == "--finding-id": + finding_ids.append(value) + else: + print(f"own-fix: unknown flag {a}", file=sys.stderr) + return 2 + elif facts_path is None: + facts_path = a + i += 1 + else: + print(f"own-fix: unexpected argument {a!r}", file=sys.stderr) + return 2 + + if not (facts_path and config_path and class_fqn and output): + print( + "own-fix: a facts.json, --config, --class and --output are all required", + file=sys.stderr, + ) + print(usage, file=sys.stderr) + return 2 + + from ownlang.config import ConfigError, load_target_subscribe + from ownlang.fix_candidates import CollectError, collect_candidates + + try: + target = load_target_subscribe(config_path) + except ConfigError as exc: + print(f"own-fix: {exc}", file=sys.stderr) + return 2 + try: + with open(facts_path, encoding="utf-8") as fh: + facts = json.load(fh) + except (OSError, ValueError) as exc: + print(f"own-fix: cannot read facts {facts_path}: {exc}", file=sys.stderr) + return 2 + + try: + envelope = collect_candidates(facts, target, class_fqn, finding_ids or None, root) + except CollectError as exc: + print(f"own-fix: {exc}", file=sys.stderr) + return 2 + + try: + with open(output, "w", encoding="utf-8") as fh: + fh.write(json.dumps(envelope, indent=2, ensure_ascii=False) + "\n") + except OSError as exc: + print(f"own-fix: cannot write {output}: {exc}", file=sys.stderr) + return 2 + print(f"own-fix: wrote {len(envelope['candidates'])} candidate(s) -> {output}") + return 0 + + _FORMATS = {"human", "github", "msbuild", "sarif", "json"} _SEVERITIES = {"error", "warning"} _VERBOSITY = {"quiet", "normal", "verbose"} @@ -410,10 +499,15 @@ def cmd_ownir(path: str, fmt: str = "human", severity: str = "error", def main(argv: list[str]) -> int: if not argv or argv[0] not in {"check", "emit", "cfg", "report", "ownir", - "summaries", "explain", "config"}: + "summaries", "explain", "config", "own-fix"}: print(__doc__) return 2 cmd = argv[0] + # `own-fix subscriptions candidates` (S0 Part B): analysis-only candidate export + # for the subscription-autofix pipeline. Has its own nested shape, so it is handled + # before the single-positional path below. + if cmd == "own-fix": + return cmd_own_fix(argv[1:]) # `config` is the minimal P-015 carrier (P-035): read an explicit own.toml and # print the declared weak-subscribe "SimpleType.Method" names, one per line, for # own-check.sh to forward to the extractor. A malformed config is a HARD error diff --git a/ownlang/config.py b/ownlang/config.py index 4eabbcf7..bccbcfb2 100644 --- a/ownlang/config.py +++ b/ownlang/config.py @@ -47,14 +47,15 @@ def _weak_subscribe_from(data: dict[str, object], path: str) -> list[str]: if not isinstance(table, dict): raise ConfigError(f"{path}: [weak-subscription] must be a table") - # Only `subscribe` is honoured in this slice. Reject any other key so a typo + # Only `subscribe` (P-035 recognition) and `target` (the S0 fix target, read by + # `load_target_subscribe`) are honoured. Reject any other key so a typo # (`subscribes`, `unsubscribe` before it is designed, ...) is a hard error, not # a silently-ignored no-op that hides a caller mistake. - unknown = sorted(set(table) - {"subscribe"}) + unknown = sorted(set(table) - {"subscribe", "target"}) if unknown: raise ConfigError( f"{path}: [weak-subscription] has unsupported key(s): " - f"{', '.join(unknown)} (only `subscribe` is supported in this slice)" + f"{', '.join(unknown)} (only `subscribe` and `target` are supported)" ) entries = table.get("subscribe", []) @@ -67,6 +68,47 @@ def _weak_subscribe_from(data: dict[str, object], path: str) -> list[str]: return list(entries) +def load_target_subscribe(path: str) -> str: + """Return the ONE weak-subscribe wrapper the fixer should emit (S0 `target_api`). + + Pinned explicitly, never guessed: either ``[weak-subscription].target`` (a single + ``"SimpleType.Method"``), or — as a convenience when there is no ambiguity — the sole + ``subscribe`` entry when the list has exactly one. Zero or several ``subscribe`` entries + with no explicit ``target`` is a :class:`ConfigError`: silently taking the first would + bake an unintended API into a public candidates contract. + """ + try: + with open(path, "rb") as fh: + data = tomllib.load(fh) + except FileNotFoundError as exc: + raise ConfigError(f"config file not found: {path}") from exc + except tomllib.TOMLDecodeError as exc: + raise ConfigError(f"{path}: invalid TOML: {exc}") from exc + + # Validate the WHOLE table first (unknown keys, a malformed `subscribe`, ...) so an + # explicit `target` can never smuggle a broken sibling key past the fail-loud + # contract. `_weak_subscribe_from` raises on any table malformation and returns [] if + # the table is absent. + subscribe = _weak_subscribe_from(data, path) + table = data.get("weak-subscription") + if not isinstance(table, dict): + raise ConfigError( + f"{path}: a [weak-subscription] table is required to pin a fix target" + ) + target = table.get("target") + if target is not None: + if not isinstance(target, str): + raise ConfigError(f"{path}: [weak-subscription].target must be a string") + _validate_entry(target, path) + return target # an explicit target wins over the (already-validated) subscribe list + if len(subscribe) == 1: + return subscribe[0] + raise ConfigError( + f"{path}: cannot pin a fix target: set [weak-subscription].target, or declare " + f"exactly one [weak-subscription].subscribe entry (found {len(subscribe)})" + ) + + def _validate_entry(entry: str, path: str) -> None: """A declared entry must be exactly ``"SimpleType.Method"``. diff --git a/ownlang/fix_candidates.py b/ownlang/fix_candidates.py new file mode 100644 index 00000000..c6274e05 --- /dev/null +++ b/ownlang/fix_candidates.py @@ -0,0 +1,316 @@ +"""S0 Part B — the `own-fix subscriptions candidates` collector (analysis-only). + +Reads the extractor's `--fix-candidates` facts and, for ONE fully-qualified class, +emits a deterministic `candidates.json`: a selection-request safety envelope plus a +candidate bundle per eligible leaky subscription. It changes no source. The heavy +C# semantics (spans, INotifyPropertyChanged classification, symbol-based teardown, +conservative source/handler identity) are already in the `fix` block; this module +only assembles, identifies, filters and orders. + +Locked contract (arbiter): + * `--class` is an EXACT fully-qualified name; a partial / nested / generated / or + ambiguously-resolved type is a hard error. + * finding_id is line-independent and versioned: + SHA256(version . containing_type . enclosing_member . event_identity . + source_identity . handler_identity . occurrence_ordinal) + (NUL-separated) — the span/line are location metadata, never in the id. + * the target subscribe API is PINNED from config (never the first of a list). + * S0 permits only `convert_acquire` (INotifyPropertyChanged contract only) and + `manual_review`; `convert_exact_teardown` is deferred to S2 with a pinned remove + API, so teardown metadata is carried but never a conversion permission. + * candidates are deterministically ordered and every source file gets a SHA-256. +""" + +from __future__ import annotations + +import hashlib +import os +from typing import Any + +_FINDING_ID_VERSION = "own-fix-subscription-v1" +_CONSTRAINTS: dict[str, object] = { + "max_types_changed": 1, + "max_files_changed": 1, + "allow_helper_changes": False, + "allow_config_changes": False, + "allow_suppressions": False, +} + + +class CollectError(Exception): + """A candidate-collection request that cannot be honoured (bad class, unknown + finding id, unreadable source). Callers surface it as a hard (non-zero) error.""" + + +_FIX_VERSION = 1 + + +def _field(obj: dict[str, Any], key: str, kind: str, ctx: str) -> Any: + """Fetch `obj[key]`, hard-failing (CollectError) on a missing key or a value of the + wrong JSON kind — so malformed external facts surface as a controlled error, never a + KeyError/TypeError traceback. `int` deliberately excludes `bool`.""" + if key not in obj: + raise CollectError(f"{ctx}: missing field {key!r}") + v = obj[key] + ok = { + "str": isinstance(v, str), + "int": isinstance(v, int) and not isinstance(v, bool), + "bool": isinstance(v, bool), + "list": isinstance(v, list), + "dict": isinstance(v, dict), + }[kind] + if not ok: + raise CollectError(f"{ctx}: field {key!r} must be {kind}, got {type(v).__name__}") + return v + + +_SPAN_INTS = ("start", "length", "start_line", "start_column", "end_line", "end_column") + + +def _validate_span(span: Any, ctx: str) -> None: + if not isinstance(span, dict): + raise CollectError(f"{ctx}: span must be an object") + for k in _SPAN_INTS: + _field(span, k, "int", f"{ctx}.span") + + +def _validate_teardown(td: Any, ctx: str) -> None: + if not isinstance(td, dict): + raise CollectError(f"{ctx}: teardown must be an object") + status = _field(td, "status", "str", f"{ctx}.teardown") + if status not in ("none", "exact", "ambiguous"): + raise CollectError(f"{ctx}.teardown: unknown status {status!r}") + for i, cand in enumerate(_field(td, "candidates", "list", f"{ctx}.teardown")): + cctx = f"{ctx}.teardown.candidates[{i}]" + if not isinstance(cand, dict): + raise CollectError(f"{cctx}: must be an object") + _field(cand, "source", "str", cctx) + _field(cand, "handler", "str", cctx) + _field(cand, "match", "str", cctx) + _validate_span(cand.get("span"), cctx) + + +_FIX_STR_FIELDS = ( + "enclosing_member", + "event_identity", + "event_contract", + "source_identity", + "source_identity_kind", + "handler_identity", + "handler_identity_kind", +) + + +def _validate_fix(fx: Any, ctx: str) -> None: + """Narrow shape check of the `fix` block this collector consumes / republishes — not + a full JSON Schema, only the S0 contract.""" + if not isinstance(fx, dict): + raise CollectError(f"{ctx}: fix must be an object") + for k in _FIX_STR_FIELDS: + _field(fx, k, "str", ctx) + _field(fx, "occurrence_ordinal", "int", ctx) + _validate_span(fx.get("span"), ctx) + _validate_teardown(fx.get("teardown"), ctx) + + +def _validate_version(facts: dict[str, Any]) -> None: + v = facts.get("fix_candidates_version") + if not (isinstance(v, int) and not isinstance(v, bool) and v == _FIX_VERSION): + raise CollectError( + f"facts fix_candidates_version must be integer {_FIX_VERSION} (got {v!r}) — " + f"produce facts with a compatible --fix-candidates extractor" + ) + if not isinstance(facts.get("components"), list): + raise CollectError("facts.components must be a list") + + +def _resolve_source(root: str, rel: str) -> tuple[str, str]: + """Canonicalize a facts-supplied source path and CONFINE it to `root`. Returns + (canonical root-relative path with `/`, absolute real path). A `..` escape, an + absolute path outside `root`, a symlink pointing out, or a non-regular file are all + hard errors — `file` arrives from external facts and flows into the public envelope, + so it must never reference anything outside the selected repo.""" + root_real = os.path.realpath(root) + joined = rel if os.path.isabs(rel) else os.path.join(root_real, rel) + src_real = os.path.realpath(joined) + try: + common = os.path.commonpath([root_real, src_real]) + except ValueError as exc: # different drives / mixed forms + raise CollectError(f"source path {rel!r} is not inside the root {root!r}") from exc + if common != root_real: + raise CollectError(f"source path {rel!r} escapes the root {root!r}") + if not os.path.isfile(src_real): + raise CollectError(f"source path {rel!r} is not a regular file") + canonical = os.path.relpath(src_real, root_real).replace("\\", "/") + return canonical, src_real + + +def finding_id( + containing_type: str, + enclosing_member: str, + event_identity: str, + source_identity: str, + handler_identity: str, + occurrence_ordinal: int, +) -> str: + """Versioned, LINE-INDEPENDENT identity. Only semantic constituents — inserting a + blank line (which shifts span/line) must not change it.""" + payload = "\0".join( + [ + _FINDING_ID_VERSION, + containing_type, + enclosing_member, + event_identity, + source_identity, + handler_identity, + str(occurrence_ordinal), + ] + ) + digest = hashlib.sha256(payload.encode("utf-8")).hexdigest() + return f"OWN001:sha256:{digest}" + + +def _sha_of(abs_path: str) -> str: + try: + with open(abs_path, "rb") as fh: + data = fh.read() + except OSError as exc: + raise CollectError(f"cannot read source file {abs_path!r}: {exc}") from exc + return "sha256:" + hashlib.sha256(data).hexdigest() + + +def _resolve_class(facts: dict[str, Any], class_fqn: str) -> dict[str, Any]: + comps = [ + c + for c in facts["components"] + if isinstance(c, dict) and c.get("qualified_name") == class_fqn + ] + if not comps: + raise CollectError( + f"class {class_fqn!r} not found — scan with --fix-candidates and pass an " + f"exact fully-qualified name" + ) + if len(comps) > 1: + raise CollectError( + f"class {class_fqn!r} resolves to {len(comps)} declarations (partial); " + f"refusing an ambiguous type" + ) + comp: dict[str, Any] = comps[0] + cctx = f"component {class_fqn}" + _field(comp, "file", "str", cctx) + _field(comp, "subscriptions", "list", cctx) + for flag, why in ( + ("is_partial", "partial"), + ("is_nested", "nested"), + ("is_generated", "generated"), + ): + if _field(comp, flag, "bool", cctx): + raise CollectError(f"class {class_fqn!r} is {why}; refused by MVP policy") + return comp + + +def _bundle( + sub: dict[str, Any], fx: dict[str, Any], class_fqn: str, file_rel: str +) -> dict[str, Any]: + event_full = sub["event"] + event_name = fx["event_identity"].rsplit(".", 1)[-1] + source_display = event_full[: event_full.rfind(".")] if "." in event_full else "this" + contract = fx["event_contract"] + diagnostic = "OWN014" if sub["resource"] == "capture" else "OWN001" + # convert_acquire is permitted ONLY for a proven INotifyPropertyChanged contract; + # everything else (name_only / other / unresolved) is manual_review. Teardown + # conversion is NOT offered in S0 regardless of an `exact` status. + actions = ( + ["convert_acquire", "manual_review"] + if contract == "inotify_property_changed" + else ["manual_review"] + ) + return { + "finding_id": finding_id( + class_fqn, + fx["enclosing_member"], + fx["event_identity"], + fx["source_identity"], + fx["handler_identity"], + fx["occurrence_ordinal"], + ), + "diagnostic_code": diagnostic, + "containing_type": class_fqn, + "file": file_rel, + "enclosing_member": fx["enclosing_member"], + "event": event_name, + "event_identity": fx["event_identity"], + "event_contract": contract, + "source": source_display, + "source_identity": fx["source_identity"], + "source_identity_kind": fx["source_identity_kind"], + "handler": sub["handler"], + "handler_identity": fx["handler_identity"], + "handler_identity_kind": fx["handler_identity_kind"], + "occurrence_ordinal": fx["occurrence_ordinal"], + "acquire_span": fx["span"], + "teardown": fx["teardown"], + "allowed_actions": actions, + } + + +def collect_candidates( + facts: dict[str, Any], + target_subscribe: str, + class_fqn: str, + finding_ids: list[str] | None, + root: str = ".", +) -> dict[str, Any]: + """Build the candidates.json envelope for `class_fqn`. `finding_ids=None` selects + every eligible candidate; a list filters to those exact ids and hard-fails if any + is unknown (or belongs to another class).""" + _validate_version(facts) + comp = _resolve_class(facts, class_fqn) + # ONE canonical, root-confined path for the whole class — reused in every bundle, + # in allowed_types, in the sort key, and in source_files. + class_file, class_abs = _resolve_source(root, comp["file"]) + + bundles: list[dict[str, Any]] = [] + for index, sub in enumerate(comp["subscriptions"]): + sctx = f"{class_fqn}.subscriptions[{index}]" + if not isinstance(sub, dict): + raise CollectError(f"{sctx}: must be an object") + fx = sub.get("fix") + if fx is None: + continue # not a fix-eligible acquire (timer / nested / unresolved lane) + if _field(sub, "released", "bool", sctx): + continue # a released subscription is not a leak, so not a candidate + _field(sub, "event", "str", sctx) + _field(sub, "handler", "str", sctx) + _field(sub, "resource", "str", sctx) + _validate_fix(fx, sctx) + bundles.append(_bundle(sub, fx, class_fqn, class_file)) + + available = {b["finding_id"] for b in bundles} + if finding_ids is not None: + missing = [fid for fid in finding_ids if fid not in available] + if missing: + raise CollectError( + f"finding id(s) not found in class {class_fqn}: {', '.join(missing)}" + ) + wanted = set(finding_ids) + bundles = [b for b in bundles if b["finding_id"] in wanted] + + # Deterministic ordering: by file, then acquire start offset, then id (a stable + # tie-break for two acquires that somehow share a start). + bundles.sort(key=lambda b: (b["file"], b["acquire_span"]["start"], b["finding_id"])) + + source_files = [{"path": class_file, "sha256": _sha_of(class_abs)}] + + return { + "version": 1, + "operation": "fix-subscriptions", + "target_api": {"subscribe": target_subscribe}, + "selection": { + "allowed_types": [{"full_name": class_fqn, "file": class_file}], + "selected_findings": list(finding_ids) if finding_ids is not None else None, + "constraints": dict(_CONSTRAINTS), + }, + "source_files": source_files, + "candidates": bundles, + } diff --git a/spec/CLI.md b/spec/CLI.md index 6d74fd67..c217de93 100644 --- a/spec/CLI.md +++ b/spec/CLI.md @@ -10,6 +10,7 @@ | `cfg` | prints the control-flow graph (blocks + instructions) for inspection | — | | `report`| prints the compile-time buffer report and writes `*.ownreport.json` | — | | `config`| reads an explicit `own.toml` and prints the declared P-035 `[weak-subscription].subscribe` names, one per line (the minimal P-015 config carrier). `python -m ownlang config ` | non-zero on a **malformed** config (hard error) | +| `own-fix subscriptions candidates`| S0 (analysis-only): reads a `--fix-candidates` facts file and, for one **exact** `--class `, emits a deterministic `candidates.json` — a selection-request safety envelope plus a candidate bundle per leaky subscription (line-independent `finding_id`, pinned `target_api`, `allowed_actions` = `convert_acquire` for a proven INotifyPropertyChanged contract else `manual_review`, per-file SHA-256). `python -m ownlang own-fix subscriptions candidates --config --class [--finding-id ]... --output [--root ]` | non-zero on a partial/nested/generated/unknown class, an unknown finding-id, an unpinnable target, or an unreadable source | Notes: - `check`'s non-zero exit on errors is what makes it usable as a CI gate. diff --git a/tests/check_fix_candidates_facts.py b/tests/check_fix_candidates_facts.py new file mode 100644 index 00000000..3a4b4d03 --- /dev/null +++ b/tests/check_fix_candidates_facts.py @@ -0,0 +1,218 @@ +"""Assert the S0 `--fix-candidates` extractor metadata on FixCandidatesSample.cs. + +Not a ``test_*`` (it needs the C# extractor to produce the facts, so CI runs the +extractor first and passes the JSON path). Encodes the Part-A extractor contract +at the fact level; exits non-zero on any violation. + +Usage: + python tests/check_fix_candidates_facts.py [] + + fix_on = FixCandidatesSample.cs scanned WITH --fix-candidates + off = the SAME sample WITHOUT the flag (optional; asserts NO fix metadata leaks) +""" + +from __future__ import annotations + +import copy +import json +import sys + +_ADDITIVE_COMPONENT_KEYS = ( + "qualified_name", + "is_partial", + "is_nested", + "declaration_count", + "is_generated", +) + + +def _strip_additive(facts: dict) -> dict: + """The flag-ON facts with every S0-additive field removed.""" + f = copy.deepcopy(facts) + f.pop("fix_candidates_version", None) + for c in f.get("components", []): + for k in _ADDITIVE_COMPONENT_KEYS: + c.pop(k, None) + for s in c.get("subscriptions") or []: + s.pop("fix", None) + return f + + +def _load(path: str) -> dict[str, object]: + with open(path, encoding="utf-8") as fh: + return json.load(fh) + + +def _component(facts: dict, name: str) -> dict | None: + for c in facts.get("components", []): # type: ignore[union-attr] + if c.get("name") == name: + return c + return None + + +def _fixes(facts: dict, name: str) -> list[dict]: + comp = _component(facts, name) + if comp is None: + return [] + return [s["fix"] for s in (comp.get("subscriptions") or []) if s.get("fix")] + + +def main(on_path: str, off_path: str | None) -> int: + on = _load(on_path) + fails: list[str] = [] + + def check(cond: bool, msg: str) -> None: + if not cond: + fails.append(msg) + + # Top-level: additive version present, ownir_version untouched. + check(on.get("fix_candidates_version") == 1, "top-level fix_candidates_version must be 1") + check(on.get("ownir_version") == 0, "ownir_version must stay 0") + + def only_fix(name: str) -> dict | None: + fx = _fixes(on, name) + check(len(fx) == 1, f"{name}: expected exactly one fix block, got {len(fx)}") + return fx[0] if fx else None + + # INPC + exact teardown (stable source + stable handler, stable candidate match). + f = only_fix("InpcExactTeardown") + if f: + check(f["event_contract"] == "inotify_property_changed", "InpcExactTeardown: contract") + check(f["teardown"]["status"] == "exact", "InpcExactTeardown: teardown exact") + cands = f["teardown"]["candidates"] + check(len(cands) == 1, "InpcExactTeardown: one teardown candidate") + check(f["source_identity_kind"] == "stable_symbol", "InpcExactTeardown: source stable") + check(f["handler_identity_kind"] == "stable_symbol", "InpcExactTeardown: handler stable") + check(bool(cands) and cands[0]["match"] == "stable", "InpcExactTeardown: candidate stable") + + # INPC + no teardown. + f = only_fix("InpcNoTeardown") + if f: + check(f["event_contract"] == "inotify_property_changed", "InpcNoTeardown: contract") + check(f["teardown"]["status"] == "none", "InpcNoTeardown: teardown none") + + # INPC + two -= -> ambiguous. + f = only_fix("InpcAmbiguousTeardown") + if f: + check(f["teardown"]["status"] == "ambiguous", "InpcAmbiguousTeardown: teardown ambiguous") + check(len(f["teardown"]["candidates"]) == 2, "InpcAmbiguousTeardown: 2 candidates") + + # Event NAMED PropertyChanged but not INotifyPropertyChanged. + f = only_fix("NameOnlySubscriber") + if f: + check(f["event_contract"] == "name_only", "NameOnlySubscriber: must be name_only") + + # Unrelated event. + f = only_fix("OtherEventSubscriber") + if f: + check(f["event_contract"] == "other", "OtherEventSubscriber: must be other") + + # Two subscriptions on one line: same start_line, DIFFERENT span.start. + two = _fixes(on, "TwoOnOneLine") + check(len(two) == 2, f"TwoOnOneLine: expected two fix blocks, got {len(two)}") + if len(two) == 2: + s0, s1 = two[0]["span"], two[1]["span"] + check(s0["start_line"] == s1["start_line"], "TwoOnOneLine: same line") + check(s0["start"] != s1["start"], "TwoOnOneLine: spans must differ (full span)") + + # Wrapped delegate: handler NORMALIZED to the method, teardown still exact. + f = only_fix("WrappedDelegate") + if f: + hid = f["handler_identity"] + check( + "OnChanged(" in hid and "PropertyChangedEventHandler" not in hid, + f"WrappedDelegate: handler must normalize to the method, got {hid!r}", + ) + check(f["teardown"]["status"] == "exact", "WrappedDelegate: teardown must be exact") + + # Nested-type isolation: the outer component carries ONLY its own subscription + # as a fix candidate; the nested class's subscription is fixed under Nested. + outer = _fixes(on, "OuterWithNested") + check(len(outer) == 1, f"OuterWithNested: outer must have exactly one fix, got {len(outer)}") + if outer: + check("OnOuter(" in outer[0]["handler_identity"], "OuterWithNested: fix must be OnOuter") + nested_comp = _component(on, "Nested") + is_nested = nested_comp is not None and nested_comp.get("is_nested") is True + check(is_nested, "Nested: is_nested must be true") + nested = _fixes(on, "Nested") + check(len(nested) == 1, "Nested: must carry its own OnNested fix") + + # Component qualified_name is a real FQN. + comp = _component(on, "InpcExactTeardown") + fqn = comp.get("qualified_name") if comp else None + check( + fqn == "Own.Samples.FixCandidates.InpcExactTeardown", + "InpcExactTeardown: qualified_name must be the FQN", + ) + + # Blocker-1: a computed/unresolved receiver or handler must NEVER be exact. + b1 = [ + ("ComputedReceiverInvocation", "ambiguous", "computed", "stable_symbol"), + ("ComputedReceiverProperty", "ambiguous", "computed", "stable_symbol"), + ("DifferentRoots", "none", "computed", "stable_symbol"), + ("ComputedHandler", "ambiguous", "stable_symbol", "computed"), + # handler half: method symbol != delegate identity, storage symbol != value + ("HandlerDifferentTarget", "none", "stable_symbol", "computed"), + ("HandlerReassignedField", "ambiguous", "stable_symbol", "computed"), + ] + for name, status, srck, hk in b1: + f = only_fix(name) + if f: + check(f["teardown"]["status"] == status, f"{name}: teardown must be {status}") + check(f["teardown"]["status"] != "exact", f"{name}: must NOT be exact") + check(f["source_identity_kind"] == srck, f"{name}: source_identity_kind {srck}") + check(f["handler_identity_kind"] == hk, f"{name}: handler_identity_kind {hk}") + + # Blocker-2: occurrence_ordinal is scoped by enclosing member. + across = _fixes(on, "OrdinalAcrossMembers") + check(len(across) == 2, f"OrdinalAcrossMembers: expected 2 fixes, got {len(across)}") + if len(across) == 2: + check(all(x["occurrence_ordinal"] == 0 for x in across), "OrdinalAcrossMembers: each ord 0") + check( + across[0]["enclosing_member"] != across[1]["enclosing_member"], + "OrdinalAcrossMembers: distinct enclosing members", + ) + within = _fixes(on, "OrdinalWithinMember") + check(len(within) == 2, f"OrdinalWithinMember: expected 2 fixes, got {len(within)}") + if len(within) == 2: + check( + {x["occurrence_ordinal"] for x in within} == {0, 1}, + "OrdinalWithinMember: ordinals must be 0 and 1", + ) + check( + within[0]["enclosing_member"] == within[1]["enclosing_member"], + "OrdinalWithinMember: same enclosing member", + ) + refov = _fixes(on, "RefOverloadEnclosing") + check(len(refov) == 2, f"RefOverloadEnclosing: expected 2 fixes, got {len(refov)}") + if len(refov) == 2: + encls = {x["enclosing_member"] for x in refov} + check(len(encls) == 2, "RefOverloadEnclosing: ref/value overloads need distinct signatures") + check(any("ref " in e for e in encls), "RefOverloadEnclosing: a signature must show `ref`") + + # Off-run must carry NO fix metadata at all. + if off_path is not None: + off = _load(off_path) + check("fix_candidates_version" not in off, "flag-off: no fix_candidates_version") + for c in off.get("components", []): # type: ignore[union-attr] + check("qualified_name" not in c, f"flag-off: {c.get('name')} has qualified_name") + for s in c.get("subscriptions") or []: + check("fix" not in s, "flag-off: a subscription carries a fix block") + # Additivity, positively: strip every additive field from the flag-ON facts and + # the result must EQUAL the flag-off facts (same records, same order, same old + # values) -- enabling the metadata changed nothing pre-existing. + check(_strip_additive(on) == off, "flag-on minus additive fields must equal flag-off") + + if fails: + for fmsg in fails: + print("FAIL:", fmsg, file=sys.stderr) + return 1 + print("fix-candidates facts: all checks pass") + return 0 + + +if __name__ == "__main__": + if len(sys.argv) not in (2, 3): + print(__doc__, file=sys.stderr) + raise SystemExit(2) + raise SystemExit(main(sys.argv[1], sys.argv[2] if len(sys.argv) == 3 else None)) diff --git a/tests/goldens/README.md b/tests/goldens/README.md new file mode 100644 index 00000000..06e7372c --- /dev/null +++ b/tests/goldens/README.md @@ -0,0 +1,25 @@ +# Goldens + +## `fix_candidates_off.golden.json` + +The pre-S0 extractor output for `frontend/roslyn/samples/FixCandidatesSample.cs` +with **no** `--fix-candidates` flag. The CI "S0 fix-candidates" step diffs the +current extractor's flag-off output against this file **byte-for-byte** — the +proof that `--fix-candidates` is strictly additive (flag-off is identical to the +extractor before S0 existed), not merely "no new keys". + +It was generated by the extractor at the S0 branch point (`ff21d4a`), so it +represents genuine pre-S0 output rather than the S0 branch's own flag-off run. + +### Regenerating (only when an unrelated extractor change intentionally alters this sample's facts) + +```bash +# from the repo root, with a base checkout of the pre-change extractor built: +dotnet run --project /frontend/roslyn/OwnSharp.Extractor -c Release -- \ + frontend/roslyn/samples/FixCandidatesSample.cs -o tests/goldens/fix_candidates_off.golden.json +``` + +Run both the base extractor and the current one from the **same working directory** +with the **same relative sample path** so the `file` field matches. Never regenerate +it from the S0 (or later) extractor's flag-off run — that would make the parity gate +compare the change against itself. diff --git a/tests/goldens/fix_candidates_off.golden.json b/tests/goldens/fix_candidates_off.golden.json new file mode 100644 index 00000000..39cfe1a1 --- /dev/null +++ b/tests/goldens/fix_candidates_off.golden.json @@ -0,0 +1,328 @@ +{ + "ownir_version": 0, + "module": "Extracted", + "components": [ + { + "name": "InpcExactTeardown", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 24, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "InpcNoTeardown", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "pub.PropertyChanged", + "handler": "OnChanged", + "line": 34, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "InpcAmbiguousTeardown", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 45, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "NameOnlySubscriber", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "p.PropertyChanged", + "handler": "OnChanged", + "line": 61, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "OtherEventSubscriber", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "p.Clicked", + "handler": "OnClick", + "line": 73, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "TwoOnOneLine", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "a.PropertyChanged", + "handler": "OnA", + "line": 81, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + }, + { + "event": "b.PropertyChanged", + "handler": "OnB", + "line": 81, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "WrappedDelegate", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "new PropertyChangedEventHandler(OnChanged)", + "line": 94, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "OuterWithNested", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "pub.PropertyChanged", + "handler": "OnOuter", + "line": 105, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + }, + { + "event": "pub.PropertyChanged", + "handler": "OnNested", + "line": 110, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "Nested", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "pub.PropertyChanged", + "handler": "OnNested", + "line": 110, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "ComputedReceiverInvocation", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "GetPublisher().PropertyChanged", + "handler": "OnChanged", + "line": 123, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "ComputedReceiverProperty", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "Pub.PropertyChanged", + "handler": "OnChanged", + "line": 133, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "DifferentRoots", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_a.Publisher.PropertyChanged", + "handler": "OnChanged", + "line": 154, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "ComputedHandler", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "H", + "line": 171, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "OrdinalAcrossMembers", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 187, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + }, + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 190, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "OrdinalWithinMember", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "pub.PropertyChanged", + "handler": "OnChanged", + "line": 199, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + }, + { + "event": "pub.PropertyChanged", + "handler": "OnChanged", + "line": 200, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "RefOverloadEnclosing", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 212, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + }, + { + "event": "_pub.PropertyChanged", + "handler": "OnChanged", + "line": 213, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "HandlerDifferentTarget", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "_left.OnChanged", + "line": 237, + "released": false, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + }, + { + "name": "HandlerReassignedField", + "file": "frontend/roslyn/samples/FixCandidatesSample.cs", + "subscriptions": [ + { + "event": "_pub.PropertyChanged", + "handler": "_handler", + "line": 254, + "released": true, + "resource": "subscription", + "source": "injected", + "lambda": false + } + ] + } + ], + "services": [], + "functions": [], + "stats": { + "methods_with_local": 0, + "methods_flow_analysed": 0, + "methods_skipped_unmodelled": 0 + } +} \ No newline at end of file diff --git a/tests/test_fix_candidates.py b/tests/test_fix_candidates.py new file mode 100644 index 00000000..c5b905a3 --- /dev/null +++ b/tests/test_fix_candidates.py @@ -0,0 +1,274 @@ +#!/usr/bin/env python3 +"""S0 Part B — the `own-fix subscriptions candidates` collector (SDK-free tests). + +Drives `ownlang/fix_candidates.py` + `ownlang/config.py::load_target_subscribe` over +SYNTHETIC fix-candidate facts (no .NET SDK): finding-id line-independence, the +partial/nested/generated and unknown/wrong-class hard rejections, deterministic +ordering, per-file SHA-256, the convert_acquire-only-for-INotifyPropertyChanged +permission, released-is-not-a-leak, and target-API pinning. The end-to-end run over +the real extractor facts lives in the "C# leak extractor" CI job. + +Run: python tests/test_fix_candidates.py + python tests/run_tests.py (auto-discovered) +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from ownlang.config import ConfigError, load_target_subscribe +from ownlang.fix_candidates import CollectError, collect_candidates, finding_id + +_EV = "System.ComponentModel.INotifyPropertyChanged.PropertyChanged" + + +def _fix(**kw: object) -> dict: + f = { + "enclosing_member": "N.C.C()", + "event_identity": _EV, + "event_contract": "inotify_property_changed", + "source_identity": "N.C._pub", + "source_identity_kind": "stable_symbol", + "handler_identity": "N.C.OnChanged(object, ...)", + "handler_identity_kind": "stable_symbol", + "occurrence_ordinal": 0, + "span": {"start": 100, "length": 30, "start_line": 1, + "start_column": 1, "end_line": 1, "end_column": 31}, + "teardown": {"status": "none", "candidates": []}, + } + f.update(kw) + return f + + +def _sub(fix: dict | None, released: bool = False, event: str = "_pub.PropertyChanged", + handler: str = "OnChanged", resource: str = "subscription") -> dict: + s = {"event": event, "handler": handler, "line": 1, "released": released, + "resource": resource, "source": "injected", "lambda": False} + if fix is not None: + s["fix"] = fix + return s + + +_OMIT = object() + + +def _facts(subs: list[dict], qn: str = "N.C", file: str = "N/C.cs", + is_partial: bool = False, is_nested: bool = False, is_generated: bool = False, + extra: list[dict] | None = None, version: object = 1) -> dict: + comp = {"name": qn.rsplit(".", 1)[-1], "qualified_name": qn, "is_partial": is_partial, + "is_nested": is_nested, "declaration_count": 1, "is_generated": is_generated, + "file": file, "subscriptions": subs} + facts: dict = {"ownir_version": 0, "components": [comp, *(extra or [])]} + if version is not _OMIT: + facts["fix_candidates_version"] = version + return facts + + +def run() -> int: + ok = 0 + bad = 0 + + def check(cond: bool, label: str) -> None: + nonlocal ok, bad + if cond: + ok += 1 + else: + bad += 1 + print(f" FAIL: {label}") + + def raises(fn: object, *a: object) -> bool: + try: + fn(*a) # type: ignore[operator] + except CollectError: + return True + return False + + # A real source file so the per-file SHA can be computed + verified. + with tempfile.TemporaryDirectory() as root: + src_rel = "N/C.cs" + src_abs = os.path.join(root, src_rel) + os.makedirs(os.path.dirname(src_abs), exist_ok=True) + content = b"// pretend source\nclass C {}\n" + with open(src_abs, "wb") as fh: + fh.write(content) + + # --- finding_id is line-independent (span/line are not constituents) --- + env_a = collect_candidates(_facts([_sub(_fix())]), + "WeakEvents.AddPropertyChanged", "N.C", None, root) + env_b = collect_candidates(_facts([_sub(_fix(span={"start": 999, "length": 30, + "start_line": 42, "start_column": 1, + "end_line": 42, "end_column": 31}))]), + "WeakEvents.AddPropertyChanged", "N.C", None, root) + check( + env_a["candidates"][0]["finding_id"] == env_b["candidates"][0]["finding_id"], + "finding_id is line/span-independent", + ) + # ... and it IS the versioned SHA over the constituents. + check( + env_a["candidates"][0]["finding_id"] + == finding_id("N.C", "N.C.C()", _EV, "N.C._pub", "N.C.OnChanged(object, ...)", 0), + "finding_id matches the versioned formula", + ) + + # --- convert_acquire only for a proven INotifyPropertyChanged contract --- + inpc = collect_candidates(_facts([_sub(_fix())]), + "WeakEvents.AddPropertyChanged", "N.C", None, root) + check( + inpc["candidates"][0]["allowed_actions"] == ["convert_acquire", "manual_review"], + "INPC contract -> convert_acquire + manual_review", + ) + name_only = collect_candidates(_facts([_sub(_fix(event_contract="name_only"))]), + "WeakEvents.AddPropertyChanged", "N.C", None, root) + check( + name_only["candidates"][0]["allowed_actions"] == ["manual_review"], + "name_only contract -> manual_review only", + ) + + # --- released subscription is not a leak -> not a candidate --- + rel = collect_candidates(_facts([_sub(_fix(), released=True)]), + "WeakEvents.AddPropertyChanged", "N.C", None, root) + check(len(rel["candidates"]) == 0, "released subscription is not a candidate") + + # --- per-file SHA-256 recorded + correct --- + want_sha = "sha256:" + hashlib.sha256(content).hexdigest() + check( + inpc["source_files"][0]["path"] == "N/C.cs" + and inpc["source_files"][0]["sha256"] == want_sha, + "source file SHA-256 recorded and correct", + ) + + # --- target_api pinned from config (never the first of a list) --- + check( + inpc["target_api"] == {"subscribe": "WeakEvents.AddPropertyChanged"}, + "target_api pinned", + ) + + # --- partial / nested / generated -> hard error --- + check(raises(collect_candidates, _facts([_sub(_fix())], is_partial=True), + "W.X", "N.C", None, root), "partial type refused") + check(raises(collect_candidates, _facts([_sub(_fix())], is_nested=True), + "W.X", "N.C", None, root), "nested type refused") + check(raises(collect_candidates, _facts([_sub(_fix())], is_generated=True), + "W.X", "N.C", None, root), "generated type refused") + # two declarations with the same FQN (partial split) -> ambiguous + dup = _facts([_sub(_fix())]) + dup["components"].append(dict(dup["components"][0])) + check(raises(collect_candidates, dup, "W.X", "N.C", None, root), + "duplicate FQN declarations refused") + # missing class + check(raises(collect_candidates, _facts([_sub(_fix())]), "W.X", "N.Missing", None, root), + "unknown class refused") + + # --- unknown finding-id -> hard error --- + check( + raises(collect_candidates, _facts([_sub(_fix())]), "W.X", "N.C", + ["OWN001:sha256:deadbeef"], root), + "unknown finding-id refused", + ) + + # --- deterministic ordering (by file, span.start, id) --- + two = _facts([ + _sub(_fix(occurrence_ordinal=1, + span={"start": 300, "length": 10, "start_line": 3, "start_column": 1, + "end_line": 3, "end_column": 11})), + _sub(_fix(occurrence_ordinal=0, + span={"start": 100, "length": 10, "start_line": 1, "start_column": 1, + "end_line": 1, "end_column": 11})), + ]) + e1 = collect_candidates(two, "WeakEvents.AddPropertyChanged", "N.C", None, root) + starts = [c["acquire_span"]["start"] for c in e1["candidates"]] + check(starts == sorted(starts) == [100, 300], "candidates ordered by span.start") + + # --- Blocker 1: schema/version + shape validation -> CollectError --- + check(raises(collect_candidates, _facts([_sub(_fix())], version=_OMIT), + "W.X", "N.C", None, root), "missing fix_candidates_version refused") + check(raises(collect_candidates, _facts([_sub(_fix())], version=True), + "W.X", "N.C", None, root), "boolean version refused") + check(raises(collect_candidates, _facts([_sub(_fix())], version=2), + "W.X", "N.C", None, root), "future version 2 refused") + bad_fix = _fix() + del bad_fix["event_identity"] + check(raises(collect_candidates, _facts([_sub(bad_fix)]), + "W.X", "N.C", None, root), "missing identity field refused") + check(raises(collect_candidates, _facts([_sub(_fix(span={"length": 5}))]), + "W.X", "N.C", None, root), "malformed span refused") + check(raises(collect_candidates, + _facts([_sub(_fix(teardown={"status": 7, "candidates": []}))]), + "W.X", "N.C", None, root), "malformed teardown refused") + + # --- Blocker 3: a source path must stay inside --root --- + check(raises(collect_candidates, _facts([_sub(_fix())], file="../escape.cs"), + "W.X", "N.C", None, root), "../ escape refused") + outside_abs = os.path.join(os.path.dirname(os.path.realpath(root)), "outside.cs") + check(raises(collect_candidates, _facts([_sub(_fix())], file=outside_abs), + "W.X", "N.C", None, root), "absolute path outside root refused") + with tempfile.TemporaryDirectory() as outside: + secret = os.path.join(outside, "secret.cs") + with open(secret, "wb") as fh: + fh.write(b"secret") + link = os.path.join(root, "link.cs") + try: + os.symlink(secret, link) + has_symlink = True + except (OSError, NotImplementedError): + has_symlink = False + if has_symlink: + check( + raises(collect_candidates, _facts([_sub(_fix())], file="link.cs"), + "W.X", "N.C", None, root), + "symlink escape refused", + ) + + # --- config: target-API pinning rules --- + def _pin(text: str) -> str: + with tempfile.NamedTemporaryFile("w", suffix=".toml", delete=False) as fh: + fh.write(text) + path = fh.name + try: + return load_target_subscribe(path) + finally: + os.unlink(path) + + def _pin_raises(text: str) -> bool: + try: + _pin(text) + except ConfigError: + return True + return False + + check( + _pin('[weak-subscription]\nsubscribe = ["WeakEvents.AddPropertyChanged"]\n') + == "WeakEvents.AddPropertyChanged", + "single subscribe entry is the target", + ) + check( + _pin('[weak-subscription]\ntarget = "WeakEvents.AddPropertyChanged"\n' + 'subscribe = ["A.B", "C.D"]\n') == "WeakEvents.AddPropertyChanged", + "explicit target wins over a valid multi-entry subscribe list", + ) + # An explicit target must NOT smuggle a broken sibling key past table validation. + check( + _pin_raises('[weak-subscription]\ntarget = "A.B"\nsubscribes = ["C.D"]\n'), + "target + unsupported key -> hard error", + ) + check( + _pin_raises('[weak-subscription]\ntarget = "A.B"\nsubscribe = "not-a-list"\n'), + "target + malformed subscribe -> hard error", + ) + check( + _pin_raises('[weak-subscription]\nsubscribe = ["A.B", "C.D"]\n'), + "several subscribe entries with no target -> hard error (no silent first-pick)", + ) + check(_pin_raises("[other]\nx = 1\n"), "no [weak-subscription] table -> hard error") + + print(f"fix-candidates collector: {ok} ok, {bad} bad") + return bad + + +if __name__ == "__main__": + raise SystemExit(run())