Rotate [<return: X>] attributes during binding normalization

.github/instructions/SyntaxTree.instructions.md

@@ -1,7 +1,31 @@
 ---
 applyTo:
   - "src/Compiler/SyntaxTree/SyntaxTree.{fs,fsi}"
+  - "src/Compiler/SyntaxTree/SyntaxTreeOps.{fs,fsi}"
+  - "src/Compiler/SyntaxTree/ParseHelpers.{fs,fsi}"
   - "src/Compiler/pars.fsy"
 ---
 
+# The Untyped Syntax Tree
+
 Read `docs/changing-the-ast.md`.
+
+## The parse tree describes the source, not the semantics
+
+`SynBinding`, `SynExpr`, `SynPat` and friends answer one question: **what did the user write, and where?** They are not a private staging area for the type checker — they are the public output of `FSharpParseFileResults`, and for formatters, analyzers, source generators and refactoring tooling they are the *only* view of the file.
+
+The parser is the last stage that sees the source. Anything it discards or rewrites is gone: no downstream consumer can recover it.
+
+So do not move, merge, synthesize or drop nodes in the parser to suit a downstream consumer, even when the relocation is semantically correct. Lower it in `BindingNormalization`, in the checker, or wherever the consumer actually reads — those stages can rewrite freely because the parse tree survives them intact.
+
+Watch for the lossy variants specifically. Narrowing a range (an attribute's own span instead of the `[< >]` that encloses it) and flattening a grouping (splicing several `SynAttributeList`s into one) both destroy information that no later stage can reconstruct.
+
+If a checker-side fix tempts you to edit `mkSynBinding` or a `pars.fsy` action, ask what the untyped tree now claims about source it can no longer describe.
+
+## Changing tree shape requires baseline coverage
+
+`tests/service/data/SyntaxTree` pretty-prints parse trees to `.bsl` files, so a shape change surfaces as a baseline diff a reviewer has to accept. That safety net only works for syntax the corpus actually contains — an uncovered case produces no diff, which reads exactly like a change that broke nothing.
+
+When you change what the parser produces, add a `.fs`/`.bsl` pair for the syntax you touched before relying on a green run. Typed-tree tests (`AttributeCheckingTests.fs`, `Symbols.fs`, component tests) cannot substitute: they observe the tree *after* lowering, and will pass while the parse tree is wrong.
+
+See `docs/postmortems/regression-parse-tree-fidelity-return-attributes.md` for what this cost when it was ignored.

docs/postmortems/README.md

@@ -9,3 +9,4 @@ These are referenced from [agentic instructions](../../.github/instructions/) an
 - [`regression-fs0229-bstream-misalignment.md`](regression-fs0229-bstream-misalignment.md) — a conditional write with an unconditional read shifted the pickle B-stream, producing `FS0229` when reading older metadata.
 - [`regression-legacy-inline-metadata-dynamic-invocation.md`](regression-legacy-inline-metadata-dynamic-invocation.md) — a new inline-flag case reused a serialized bit pattern that already meant "required inline" in F# 5 binaries, breaking cross-assembly SRTP at runtime.
 - [`regression-sourcebuild-cpm-runtime-version-floor.md`](regression-sourcebuild-cpm-runtime-version-floor.md) — renaming the CPM runtime-package pins to computed `$(System*CentralVersion)` aliases with a floor defeated source-build's `$(System*Version)` override, causing prebuilt/`NU1109` failures in the VMR that fsharp CI could not see.
+- [`regression-parse-tree-fidelity-return-attributes.md`](regression-parse-tree-fidelity-return-attributes.md) — a semantic lowering moved into the parser made `SynBinding.attributes` drop `[<return: X>]`, so tools reading the untyped tree silently deleted attributes the source visibly had.

docs/postmortems/regression-parse-tree-fidelity-return-attributes.md

@@ -0,0 +1,78 @@
+# Regression: `[<return: X>]` Attributes Disappeared From the Untyped Syntax Tree
+
+## Summary
+
+A semantic lowering that had always run in the type checker was moved into the parser, so `SynBinding.attributes` stopped reporting `[<return: X>]` attributes that were visibly present in the source. Tools that read the untyped tree — formatters, analyzers, source generators — were handed a tree that no longer matched the file it came from. Fantomas silently deleted every `[<return: Struct>]` partial active pattern it formatted.
+
+## Error Manifestation
+
+No error. No warning. No diagnostic anywhere.
+
+Given source that visibly carries an attribute:
+
+```fsharp
+[<return: Struct>]
+let (|Foo|_|) x = ValueNone
+```
+
+`SynBinding.attributes` was `[]`. A round-tripping tool read the binding, found nothing to print, and wrote the file back without the attribute:
+
+```fsharp
+let (|Foo|_|) x = ValueNone     // attribute gone, file still compiles, meaning changed
+```
+
+The failure is silent by construction: the consumer cannot detect an absence it was never told about. Fantomas' own code base contains 34 such active patterns, and self-formatting would have stripped all of them.
+
+## Root Cause
+
+`[<return: X>]` on a binding is written in front of the binding but targets the method's return value. Routing it to `SynValInfo.retInfo` is correct for the type checker, IL emit and the Symbols API. The mistake was *where* the routing happened.
+
+[PR #19738](https://github.com/dotnet/fsharp/pull/19738) moved the rotation into `mkSynBinding` in `SyntaxTreeOps.fs`, a parser-stage constructor. Before that, the rotation lived in `TcNormalizedBinding` and patched a *local* `valSynData`; the `SynBinding` itself was never touched, so the parse tree stayed faithful to the source.
+
+The violated invariant:
+
+> **The untyped syntax tree describes where the user wrote things. Semantic relocation belongs downstream of it.**
+
+`SynBinding` has exactly one contract — report the source. It is not a type checker input in disguise; it is the public output of `FSharpParseFileResults`, and it is the *only* view some consumers have. Once the parser rewrites a node, no consumer can recover the original, because the parser is the last stage that saw the source.
+
+The rotation was lossy in two ways that made recovery impossible even for a consumer that knew about it:
+
+- The attribute list's range narrowed from the `[< >]` span to the attribute alone, so the brackets the user typed were no longer represented anywhere in the tree.
+- Every return attribute was collected into one synthesized `SynAttributeList`, so `[<return: A; return: B>]` and `[<return: A>][<return: B>]` produced identical trees. Neither can be printed back to its original form.
+
+## Why It Escaped
+
+The change was reviewed as a type-checker fix, and as a type-checker fix it was correct — both reported bugs (#17904, #19020) were genuinely fixed, and the tests added with it all passed:
+
+- `AttributeCheckingTests.fs` — diagnostics
+- `Symbols.fs` — `mfv.ReturnParameter.Attributes` via the FCS Symbols API
+
+All of them observe the *typed* tree. None observes the parse tree. The blast radius of editing `mkSynBinding` — every untyped-tree consumer in the ecosystem — was never in view.
+
+The `tests/service/data/SyntaxTree` baseline corpus is exactly the mechanism that catches this: it pretty-prints the parse tree to a `.bsl` file, so any change to tree shape shows up as a baseline diff a reviewer must accept. At the time of #19738, **not one file in that corpus contained a `return:` attribute**. The corpus was silent because the case did not exist in it, and a silent corpus reads the same as a passing one.
+
+It shipped to nuget.org in `FSharp.Compiler.Service 43.13.101-preview7.26381.103` (2026-08-11). It was found downstream by [fsprojects/fantomas#3400](https://github.com/fsprojects/fantomas/pull/3400) while bumping vendored compiler sources — caught before any Fantomas release carried it, but only because that bump walked one upstream commit at a time. Fantomas' own suite stayed green throughout: its tests covered the return *type annotation* form (`let f x : [<return: A>] int = x`), which never went through this rotation, and not the prefix form, which did.
+
+## Fix
+
+[PR #20356](https://github.com/dotnet/fsharp/pull/20356) moves the rotation to `BindingNormalization.NormalizeBinding` in `CheckExpressions.fs` — the single funnel from `SynBinding` to `NormalizedBinding`, and already a lowering step. Every consumer of the rotated form (`TcNormalizedBinding`, `AnalyzeAndMakeAndPublishRecursiveValue`, the object-expression paths) reads `NormalizeBinding`'s output, so both fixes from #19738 are unchanged and `retInfo` remains the single source of truth for the checker.
+
+`NormalizedBinding` holds a flat `SynAttribute list`, so `RotateReturnAttributes` now takes and returns that instead of `SynAttributes`. The list-splicing that flattened attribute grouping is gone with it — there is no grouping left to destroy at that layer.
+
+`SynBinding` again carries the attribute with its full `[< >]` range, and `retInfo` is empty at parse time.
+
+## Timeline
+
+| Date | PR | Change |
+|---|---|---|
+| 2026-05-20 | [#19738](https://github.com/dotnet/fsharp/pull/19738) | Rotation moved from `TcNormalizedBinding` (local `valSynData` patch) into `mkSynBinding`. Fixes #17904 and #19020; parse tree starts diverging from source. |
+| 2026-08-11 | — | Ships to nuget.org in `FSharp.Compiler.Service 43.13.101-preview7.26381.103`. |
+| 2026-08-21 | [fantomas#3400](https://github.com/fsprojects/fantomas/pull/3400) | Fantomas bumps vendored compiler sources, finds `[<return: Struct>]` silently deleted, works around it with `restoreRotatedReturnAttributes`, and raises the layering question upstream. |
+| 2026-08-25 | [#20356](https://github.com/dotnet/fsharp/pull/20356) | Rotation moved to `BindingNormalization.NormalizeBinding`. Parse tree faithful again; grouping and ranges preserved. |
+
+## Prevention
+
+- **Rule encoded** in [`.github/instructions/SyntaxTree.instructions.md`](../../.github/instructions/SyntaxTree.instructions.md): the parser must not perform semantic relocation, and any change to parse-tree shape needs `tests/service/data/SyntaxTree` coverage.
+- **Baseline coverage added** in `tests/service/data/SyntaxTree/Attribute/`: `ReturnTargetedAttributeStaysOnBinding.fs` pins the attribute to `SynBinding.attributes` with its `[< >]` range, and `ReturnTargetedAttributeGroupingIsPreserved.fs` pins `[<return: A>][<return: A>]` and `[<return: A; return: A>]` to distinct trees.
+
+The generalizable lesson is about *which* tests a change needs, not about attributes. A fix that is correct in the type checker can still be wrong in the parser, and only a parse-tree baseline will say so.

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -200,6 +200,7 @@
 * Stabilized several `preview` language features into F# 11.0 (`--langversion:11.0`, enabled by default with a .NET 11 SDK): `MethodOverloadsCache`, `ErrorOnMissingSignatureAttribute`, `DirectDelegateConstruction`, `AccessProtectedBaseFieldFromClosure`, and `RecordSpreads`. `FromEndSlicing` intentionally remains in `preview`. ([PR #20199](https://github.com/dotnet/fsharp/pull/20199))
 * Interpolated string holes (e.g. `$"{x}"`) are now formatted with invariant culture (via the `string` operator) instead of the current thread culture. ([PR #19971](https://github.com/dotnet/fsharp/pull/19971))
 * Lines starting with `#:` are now ignored ([Language suggestion 1440](https://github.com/fsharp/fslang-suggestions/issues/1440), [RFC FS-1337](https://github.com/fsharp/fslang-design/pull/830), [PR #20212](https://github.com/dotnet/fsharp/pull/20212))
+* `[<return: X>]` attributes written in front of a binding are again reported by `SynBinding.attributes` in the untyped syntax tree, with their original grouping and `[< >]` ranges. The rotation into `SynValInfo.retInfo` added by [PR #19738](https://github.com/dotnet/fsharp/pull/19738) now happens while normalizing a binding for checking instead of in the parser, so both fixes from that PR are unchanged while tools reading the parse tree (formatters, analyzers, source generators) again see what was written. ([PR #20356](https://github.com/dotnet/fsharp/pull/20356))
 * Calculate Entity.PublicPath instead of storing ([PR #20285](https://github.com/dotnet/fsharp/pull/20285))
 
 ### Breaking Changes

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -2629,6 +2629,9 @@ module BindingNormalization =
             let paramNames = Some valSynData.SynValInfo.ArgNames
             let checkXmlDocs = cenv.diagnosticOptions.CheckXmlDocs
             let xmlDoc = xmlDoc.ToXmlDoc(checkXmlDocs, paramNames)
+            // Rotate [<return:...>] from the binding to the return value. This is done here rather than in
+            // the parser so that SynBinding.attributes keeps reporting the attributes where they were written.
+            let attrs, valSynData = SynInfo.RotateReturnAttributes attrs valSynData
             NormalizedBinding(vis, kind, isInline, isMutable, attrs, xmlDoc, typars, valSynData, pat, rhsExpr, mBinding, debugPoint)
 
 //-------------------------------------------------------------------------
@@ -11507,7 +11510,7 @@ and TcNormalizedBinding declKind (cenv: cenv) env tpenv overallTy safeThisValOpt
             attrs
 
         // [<return: X>] attributes are moved out of the binding's prefix and into
-        // SynValData.SynValInfo.retInfo by SynInfo.RotateReturnAttributes in mkSynBinding,
+        // SynValData.SynValInfo.retInfo by SynInfo.RotateReturnAttributes in BindingNormalization,
         // alongside any attributes on the return type annotation populated by InferSynReturnData.
         // Use that as the single source of truth.
         let valAttribs = TcAttrs attrTgt false attrs

src/Compiler/SyntaxTree/SyntaxTreeOps.fs

@@ -765,27 +765,17 @@ module SynInfo =
     /// arity-info return position (`SynValInfo.retInfo`). Without this, downstream code that
     /// reads `Val.Attribs` would incorrectly see them alongside method-targeted attributes
     /// (see issues #17904 and #19020).
-    let RotateReturnAttributes (attrs: SynAttributes) (valSynData: SynValData) : SynAttributes * SynValData =
+    ///
+    /// This is a lowering step, applied while normalizing a binding for checking rather than in
+    /// the parser, so `SynBinding.attributes` keeps reporting the attributes where they were
+    /// written. Tools reading the untyped tree (formatters, analyzers, source generators) depend
+    /// on that.
+    let RotateReturnAttributes (attrs: SynAttribute list) (valSynData: SynValData) : SynAttribute list * SynValData =
         // Fast path: avoid all allocation when there's nothing to rotate (the common case).
-        let hasReturn =
-            attrs
-            |> List.exists (fun lst -> lst.Attributes |> List.exists isReturnTargetedAttribute)
-
-        if not hasReturn then
+        if not (List.exists isReturnTargetedAttribute attrs) then
             attrs, valSynData
         else
-            let mutable returnTargeted = []
-
-            let newAttrs =
-                attrs
-                |> List.choose (fun lst ->
-                    let ret, kept = lst.Attributes |> List.partition isReturnTargetedAttribute
-                    returnTargeted <- returnTargeted @ ret
-
-                    if List.isEmpty kept then
-                        None
-                    else
-                        Some { lst with Attributes = kept })
+            let returnTargeted, kept = attrs |> List.partition isReturnTargetedAttribute
 
             let (SynValData(memFlags, SynValInfo(args, SynArgInfo(retAttrs, opt, retId)), thisIdOpt)) =
                 valSynData
@@ -796,7 +786,7 @@ module SynInfo =
                     Range = (List.head returnTargeted).Range
                 }
 
-            newAttrs, SynValData(memFlags, SynValInfo(args, SynArgInfo(retList :: retAttrs, opt, retId)), thisIdOpt)
+            kept, SynValData(memFlags, SynValInfo(args, SynArgInfo(retList :: retAttrs, opt, retId)), thisIdOpt)
 
 let mkSynBindingRhs staticOptimizations rhsExpr mRhs retInfo =
     let rhsExpr =
@@ -817,8 +807,6 @@ let mkSynBinding
     let info =
         SynInfo.InferSynValData(memberFlagsOpt, Some headPat, Option.map snd retInfo, origRhsExpr)
 
-    let attrs, info = SynInfo.RotateReturnAttributes attrs info
-
     let rhsExpr, retTyOpt = mkSynBindingRhs staticOptimizations origRhsExpr mRhs retInfo
     let mBind = unionRangeWithXmlDoc xmlDoc mBind
     SynBinding(vis, SynBindingKind.Normal, isInline, isMutable, attrs, xmlDoc, info, headPat, retTyOpt, rhsExpr, mBind, spBind, trivia)

src/Compiler/SyntaxTree/SyntaxTreeOps.fsi

@@ -275,6 +275,15 @@ module SynInfo =
 
     val emptySynArgInfo: SynArgInfo
 
+    /// Rotate any `[<return: X>]` attributes from a binding's prefix attribute list into the
+    /// arity-info return position (`SynValInfo.retInfo`), so that the attributes reach the
+    /// return-value metadata slot rather than `Val.Attribs`.
+    ///
+    /// This is a lowering step, applied while normalizing a binding for checking rather than in
+    /// the parser, so `SynBinding.attributes` keeps reporting the attributes where they were
+    /// written.
+    val RotateReturnAttributes: attrs: SynAttribute list -> valSynData: SynValData -> SynAttribute list * SynValData
+
     /// Infer the syntactic information for a 'let' or 'member' definition, based on the argument pattern,
     /// any declared return information (e.g. .NET attributes on the return element), and the r.h.s. expression
     /// in the case of 'let' definitions.

tests/service/data/SyntaxTree/Attribute/ReturnTargetedAttributeGroupingIsPreserved.fs

@@ -0,0 +1,13 @@
+module M
+
+open System
+
+[<AttributeUsage(AttributeTargets.ReturnValue, AllowMultiple = true)>]
+type AAttribute() =
+    inherit Attribute()
+
+[<return: A>][<return: A>]
+let f () = ()
+
+[<return: A; return: A>]
+let g () = ()