fix(roslyn): protocol attribute names reserved only in the profile's shapes; a token callback outside a region entry is refused

frontend/roslyn/OwnSharp.Extractor/ProtocolLowering.cs

@@ -13,6 +13,12 @@
 //   [ProtocolToken]   a ref struct: every instance METHOD is a transition that consumes the
 //                     token (and borrows the entity for the call); a PROPERTY is a read.
 //
+// RESERVED NAMES. Matching by name makes the two names a convention the profile owns, but only
+// in those shapes: a method marked [ProtocolRegion] that takes a delegate, a ref struct marked
+// [ProtocolToken]. An attribute of the same name on anything else (a class, a plain struct, a
+// method with no callback) belongs to somebody else's code and is ignored — a scan of a
+// codebase that declares no state protocol is never refused over a name.
+//
 // This is a LOWERING, not an analysis. It reads the syntax of one method at a time and looks
 // symbols up; it never tracks state across statements, never follows aliases, and never
 // decides whether a program is right. Everything it emits is checked by the core:
@@ -93,6 +99,9 @@ private sealed class Ctx
         public Dictionary<ISymbol, (string Name, Region Region)> Tokens { get; } =
             new(SymbolEqualityComparer.Default);
         public HashSet<SyntaxNode> LoweredEntries { get; } = new();
+        /// The lambdas that were lowered as region bodies: the only callables in consumer
+        /// code that may take a token.
+        public HashSet<SyntaxNode> LoweredBodies { get; } = new();
         public int Counter;
         /// How many times the scan has lowered a mention of a borrowed entity. A construct
         /// that runs unknown code is refused UNLESS it moved this counter: then the core
@@ -118,6 +127,8 @@ public static Result Lower(Compilation compilation,
             var model = compilation.GetSemanticModel(tree);
             var root = tree.GetRoot();
             var loweredEntries = new HashSet<SyntaxNode>();
+            var loweredBodies = new HashSet<SyntaxNode>();
+            var refusedMethods = new List<SyntaxNode>();
 
             foreach (var method in root.DescendantNodes().OfType<BaseMethodDeclarationSyntax>())
             {
@@ -189,6 +200,7 @@ public static Result Lower(Compilation compilation,
                             throw new Refused(At(file, v,
                                 $"protocol token '{l.Name}' is declared outside a protocol region"));
                     loweredEntries.UnionWith(ctx.LoweredEntries);
+                    loweredBodies.UnionWith(ctx.LoweredBodies);
 
                     if (ops.Count > 0)
                         functions.Add(new Dictionary<string, object?>
@@ -199,9 +211,33 @@ public static Result Lower(Compilation compilation,
                 catch (Refused r)
                 {
                     refusals.Add(r.Message);
+                    refusedMethods.Add(method);
                 }
             }
 
+            // A token exists only inside a region. A method cannot take one (refused above),
+            // and a lambda or local function may take one only as the in-place body of a
+            // region entry that was lowered. Any other callable with a token parameter is
+            // consumer code that receives a token some other way — from a method of the
+            // protocol's own type that is not marked as a region entry, typically — and that
+            // nothing reads: without this it is simply absent from the facts, and a token
+            // spent twice in it is clean.
+            foreach (var callable in root.DescendantNodes().Where(n =>
+                         n is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax))
+            {
+                if (loweredBodies.Contains(callable)
+                    || refusedMethods.Any(m => m.Span.Contains(callable.Span)))
+                    continue;   // a lowered region body, or inside a method already refused
+                var symbol = callable is LocalFunctionStatementSyntax
+                    ? model.GetDeclaredSymbol(callable) as IMethodSymbol
+                    : model.GetSymbolInfo(callable).Symbol as IMethodSymbol;
+                var taken = symbol?.Parameters.FirstOrDefault(p => IsToken(p.Type));
+                if (taken is null || Enclosing(model, callable).Any(IsApiType))
+                    continue;   // no token; or the protocol's own types, which hand tokens out
+                refusals.Add(At(file, callable,
+                    $"a callback that takes protocol token '{taken.Type.Name}' is not the in-place body of a region entry: a token exists only inside a region, and code that receives one any other way is not analysed"));
+            }
+
             // A region entry outside any method body (an accessor, a field initializer, a
             // top-level statement) is not modelled either.
             foreach (var inv in root.DescendantNodes().OfType<InvocationExpressionSyntax>())
@@ -481,7 +517,7 @@ bool Outside(INamedTypeSymbol? owner)
                         ? info.ConvertedType : info.Type ?? info.ConvertedType)?.OriginalDefinition;
                     if (IsToken(made) && !Within(api))
                         refusals.Add(At(file, node,
-                            $"a protocol token '{made!.Name}' is created outside the protocol's own types: a token comes only from a region entry or a transition"));
+                            $"a protocol token '{made!.Name}' is created outside the protocol's own types: a token comes only from a region entry or a transition" + ReservedNames));
                     else if (node is BaseObjectCreationExpressionSyntax
                              && Bound(model.GetSymbolInfo(node)) is IMethodSymbol
                                  { DeclaredAccessibility: not Accessibility.Public } ctor
@@ -583,11 +619,37 @@ private static bool HasAttribute(ISymbol? symbol, string name) =>
         symbol is not null && symbol.GetAttributes().Any(a =>
             a.AttributeClass?.Name == name || a.AttributeClass?.Name == name + "Attribute");
 
-    private static bool IsToken(ITypeSymbol? type) => HasAttribute(type, "ProtocolToken");
+    // The two attributes are matched by NAME, so the names are reserved — but only in the
+    // SHAPE the profile gives them. A codebase that never declared a state protocol can own an
+    // attribute called ProtocolToken (a wire-protocol token class, say), and a name alone must
+    // not turn its scan into a refusal. So:
+    //
+    //   a state token   is a REF STRUCT marked [ProtocolToken];
+    //   a region entry  is a method marked [ProtocolRegion] that TAKES A DELEGATE.
+    //
+    // Anything else carrying one of the names is not part of a protocol and is left alone.
+    // The shapes are deliberately the widest ones that still catch a protocol declared wrongly:
+    // a marked region entry whose callback does not take a token, or takes a marked type that
+    // is not a ref struct, is still a region entry and is refused where it is used.
+
+    /// Marked [ProtocolToken], whatever it is. Only for saying WHY a marked type is not a token.
+    private static bool HasTokenName(ITypeSymbol? type) => HasAttribute(type, "ProtocolToken");
+
+    private static bool IsToken(ITypeSymbol? type) =>
+        type is { IsRefLikeType: true } && HasTokenName(type);
+
+    private static bool IsRegionEntry(IMethodSymbol? method)
+    {
+        if (method is null)
+            return false;
+        var declared = (method.ReducedFrom ?? method).OriginalDefinition;
+        // an UNRESOLVED parameter type may be the callback of a degraded scan: keep it in
+        return HasAttribute(declared, "ProtocolRegion")
+            && declared.Parameters.Any(p => p.Type.TypeKind is TypeKind.Delegate or TypeKind.Error);
+    }
 
-    private static bool IsRegionEntry(IMethodSymbol? method) =>
-        method is not null && HasAttribute((method.ReducedFrom ?? method).OriginalDefinition,
-                                           "ProtocolRegion");
+    private const string ReservedNames =
+        " (the attribute names ProtocolRegion and ProtocolToken are reserved in these shapes: a method marked [ProtocolRegion] that takes a delegate is a region entry, a ref struct marked [ProtocolToken] is a state token)";
 
     private static bool IsApiType(INamedTypeSymbol? type) =>
         type is not null && (IsToken(type)
@@ -759,7 +821,7 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
     {
         var args = entry.ArgumentList.Arguments;
         if (args.Count != 2)
-            throw Refuse(ctx, entry, "a protocol region entry takes (entity, callback)");
+            throw Refuse(ctx, entry, "a protocol region entry takes (entity, callback)" + ReservedNames);
         var entityExpr = Unparen(args[0].Expression);
         var entity = ctx.Model.GetSymbolInfo(entityExpr).Symbol;
         if (entityExpr is not IdentifierNameSyntax || entity is not (ILocalSymbol or IParameterSymbol))
@@ -779,9 +841,9 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
         };
         if (parameter is null
             || ctx.Model.GetDeclaredSymbol(parameter) is not IParameterSymbol token
-            || !IsToken(token.Type))
+            || !HasTokenName(token.Type))
             throw Refuse(ctx, entry,
-                "the callback of a protocol region takes exactly one protocol token");
+                "the callback of a protocol region takes exactly one protocol token" + ReservedNames);
         // A token that is not a ref struct can be captured by a nested lambda, stored in a
         // field, or carried across an await — every one of which lets it reach ANOTHER
         // region, the shape the core cannot check. The language forbids all three for a
@@ -831,6 +893,7 @@ private static void LowerRegion(Ctx ctx, InvocationExpressionSyntax entry, List<
         if (existing is null)
             ops.Add(Release(owner, line));
         ctx.LoweredEntries.Add(entry);
+        ctx.LoweredBodies.Add(lambda);
     }
 
     private static void LowerTokenLocal(Ctx ctx, ILocalSymbol local, ExpressionSyntax? init,

frontend/roslyn/README.md

@@ -118,6 +118,18 @@ SQL, another process. Those belong to concurrency tokens, constraints and
 transactions (two of them are pinned as stated limits under
 `protocol-samples/efcore/known-gaps`).
 
+**Reserved names.** Matching by name makes `ProtocolToken` and `ProtocolRegion`
+names the profile owns — but only in its own shapes: a **ref struct** marked
+`[ProtocolToken]` is a state token, a method marked `[ProtocolRegion]` **that
+takes a delegate** is a region entry. An attribute with one of these names on
+anything else (a class, a plain struct, a record, a method with no callback)
+belongs to somebody else's code and is ignored, so a codebase that declares no
+state protocol is not refused over a name. The two shapes themselves stay
+reserved: an unrelated attribute of the same name on a ref struct, or on a method
+that takes a delegate, is read as a protocol declared wrongly and refused — the
+profile cannot tell the two apart, and a mis-declared protocol must not go
+unanalysed.
+
 **The trust boundary.** The types that DECLARE a protocol — the tokens, and the
 type holding a region entry — are its trusted definition surface: they construct
 tokens, enter regions and implement transitions, and their bodies are not
@@ -148,6 +160,10 @@ not skipped: the extractor exits `2` and writes no facts, for the whole scan.
 - *Trust boundary.* A region opened inside a type that declares the protocol
   (see above): write the code that uses a protocol outside the types that
   declare it.
+- *Tokens stay in regions.* A lambda or local function that takes a token must be
+  the in-place body of a region entry. A token handed to a callback any other
+  way — by a method of the protocol's own type that is not marked
+  `[ProtocolRegion]`, for one — reaches consumer code nobody lowered.
 - *Binding.* The scan does not read `obj/`, so usings a project only gets
   **implicitly** are not there. A region whose entity does not bind is refused;
   write the usings out in the files that open regions (or qualify the names).

frontend/roslyn/protocol-samples/refused/R16_token_handed_out_past_a_region_entry.cs.txt

@@ -0,0 +1,48 @@
+using System;
+using Own.Protocols.Sample;
+
+namespace Own.Protocols.Cases;
+
+// A token exists only inside a region. Here the protocol's own type hands one out through a
+// method that is NOT marked as a region entry, so the consumer's callback is not a region
+// body: nothing lowers it, and before this was refused it was simply absent from the facts —
+// a token spent twice, clean. The declaring type is trusted; the callback is consumer code,
+// and consumer code that receives a token is either a lowered region or a refusal.
+public sealed class Letter
+{
+    public bool Sealed { get; private set; }
+
+    internal void MarkSealed() => Sealed = true;
+}
+
+[ProtocolToken]
+public readonly ref struct OpenLetter
+{
+    private readonly Letter _letter;
+    internal OpenLetter(Letter letter) => _letter = letter;
+
+    public void Seal() => _letter.MarkSealed();
+}
+
+public delegate void OpenLetterRegion(OpenLetter open);
+
+public static class LetterProtocol
+{
+    [ProtocolRegion]
+    public static void WithOpen(Letter letter, OpenLetterRegion body) => body(new OpenLetter(letter));
+
+    // the same hand-out, without the mark
+    public static void Peek(Letter letter, OpenLetterRegion body) => body(new OpenLetter(letter));
+}
+
+public static class R16TokenHandedOutPastARegionEntry
+{
+    public static void Run(Letter letter)
+    {
+        LetterProtocol.Peek(letter, open =>
+        {
+            open.Seal();
+            open.Seal();             // a stale token: OWN002 inside a region
+        });
+    }
+}

frontend/roslyn/protocol-samples/refused/R17_token_taken_by_a_local_function.cs.txt

@@ -0,0 +1,47 @@
+using System;
+using Own.Protocols.Sample;
+
+namespace Own.Protocols.Cases;
+
+// The same escape with no lambda at all: a LOCAL FUNCTION that takes the token, passed as a
+// method group to a hand-out that is not a region entry. A method may not take a token; a
+// local function is a method the consumer can write inside a handler.
+public sealed class Crate
+{
+    public bool Shipped { get; private set; }
+
+    internal void MarkShipped() => Shipped = true;
+}
+
+[ProtocolToken]
+public readonly ref struct PackedCrate
+{
+    private readonly Crate _crate;
+    internal PackedCrate(Crate crate) => _crate = crate;
+
+    public void Ship() => _crate.MarkShipped();
+}
+
+public delegate void PackedCrateRegion(PackedCrate packed);
+
+public static class CrateProtocol
+{
+    [ProtocolRegion]
+    public static void WithPacked(Crate crate, PackedCrateRegion body) => body(new PackedCrate(crate));
+
+    public static void Borrow(Crate crate, PackedCrateRegion body) => body(new PackedCrate(crate));
+}
+
+public static class R17TokenTakenByALocalFunction
+{
+    public static void Run(Crate crate)
+    {
+        void Twice(PackedCrate packed)
+        {
+            packed.Ship();
+            packed.Ship();
+        }
+
+        CrateProtocol.Borrow(crate, Twice);
+    }
+}

frontend/roslyn/protocol-samples/refused/expected.json

@@ -5,6 +5,8 @@
   "R13_public_mutator_reaches_protocol_state": "'Doc.Publish' is public and writes 'Published', state the protocol's transitions own",
   "R14_public_setter_of_protocol_state": "'Ticket.State' is public and writes 'State', state the protocol's transitions own",
   "R15_region_opened_inside_protocol_type": "a protocol region is opened inside 'ParcelHandlers', one of the protocol's own types",
+  "R16_token_handed_out_past_a_region_entry": "a callback that takes protocol token 'OpenLetter' is not the in-place body of a region entry",
+  "R17_token_taken_by_a_local_function": "a callback that takes protocol token 'PackedCrate' is not the in-place body of a region entry",
   "R1_default_token": "has no lowerable origin",
   "R2_token_outside_region": "is declared outside a protocol region",
   "R3_return_inside_region": "`return` inside a protocol region",

frontend/roslyn/protocol-samples/unrelated/U1_token_name_on_ordinary_types.cs.txt

@@ -0,0 +1,42 @@
+namespace Net.Wire;
+
+// [ProtocolToken] on everything a state token is NOT: a class, a plain struct, a record, an
+// enum, an interface. They are created, copied and passed around like any other value. A
+// state token is a REF STRUCT, so none of these is one, and the scan has nothing to say.
+[ProtocolToken]
+public sealed class SessionToken
+{
+    public string Value = "";
+}
+
+[ProtocolToken]
+public struct Ticket
+{
+    public int Id;
+}
+
+[ProtocolToken]
+public sealed record Hello(int Version);
+
+[ProtocolToken]
+public enum FrameKind { Data, Control }
+
+[ProtocolToken]
+public interface IFrame
+{
+    int Size { get; }
+}
+
+public static class U1TokenNameOnOrdinaryTypes
+{
+    public static int Read(Ticket ticket) => ticket.Id;
+
+    public static int Run(FrameKind kind)
+    {
+        var session = new SessionToken { Value = "x" };
+        var hello = new Hello((int)kind);
+        var copy = new Ticket { Id = hello.Version };
+        var again = copy;
+        return Read(again) + session.Value.Length;
+    }
+}

frontend/roslyn/protocol-samples/unrelated/U2_region_name_without_a_callback.cs.txt

@@ -0,0 +1,28 @@
+namespace Net.Wire;
+
+// [ProtocolRegion] on methods that take no delegate: there is no callback, so there is no
+// region body and nothing a region entry could mean. Called from their own class and from
+// another one, as a statement and inside an expression.
+public static class Frames
+{
+    [ProtocolRegion]
+    public static int Parse(byte[] frame) => frame.Length;
+
+    [ProtocolRegion]
+    public static void Reset(byte[] frame, int offset)
+    {
+        frame[offset] = 0;
+    }
+
+    public static int Own() => Parse(new byte[4]);
+}
+
+public static class U2RegionNameWithoutACallback
+{
+    public static int Run()
+    {
+        var frame = new byte[8];
+        Frames.Reset(frame, 0);
+        return Frames.Parse(frame) + Frames.Own();
+    }
+}

frontend/roslyn/protocol-samples/unrelated/U3_region_name_with_a_callback.cs.txt

@@ -0,0 +1,29 @@
+using System;
+
+namespace Net.Wire;
+
+// THE RESIDUAL, pinned. [ProtocolRegion] on a method that DOES take a delegate has exactly the
+// shape of a region entry, and the profile cannot tell it from a protocol whose author forgot
+// to mark the token — which must stay a refusal, or a mis-declared protocol would go
+// unanalysed in silence. So this collision is still refused; the message says which names are
+// reserved and in which shapes.
+public static class Bytes
+{
+    [ProtocolRegion]
+    public static void Each(byte[] frame, Action<byte> body)
+    {
+        foreach (var b in frame)
+            body(b);
+    }
+}
+
+public static class U3RegionNameWithACallback
+{
+    public static int Run()
+    {
+        var frame = new byte[4];
+        var sum = 0;
+        Bytes.Each(frame, b => { sum += b; });
+        return sum;
+    }
+}