Warn FS3888 when consumer-visible attributes are missing from .fsi

.github/instructions/NoBloat.instructions.md

@@ -0,0 +1,55 @@
+---
+applyTo:
+  - "src/Compiler/**/*.{fs,fsi}"
+  - "vsintegration/src/**/*.{fs,fsi}"
+  - "tests/FSharp.Compiler.ComponentTests/**/*.fs"
+  - "tests/FSharp.Compiler.Service.Tests/**/*.fs"
+  - "vsintegration/tests/**/*.fs"
+---
+
+# Code Style: No Bloat
+
+Reviewers read code, not prose. Add bytes only when they pay for themselves.
+
+## Comments
+
+Good names beat comments **24/7**. Before writing a comment, ask: *can I rename a value, extract a function, or use an active pattern so the comment becomes unnecessary?* If yes, do that instead.
+
+- **Do not** restate what the code says (variable name, type name, attribute name, function signature).
+- **Do not** narrate the algorithm step-by-step. The diff is the algorithm.
+- **Do not** justify design decisions inline ("we chose X over Y because…"). Put rationale in the commit message or PR body.
+- **Do not** leave war-story comments ("previously we did Z, but…", "counter-example: …"). The history is in `git log`.
+- **Do not** write multi-line `///` doc comments for internal helpers whose body is one expression.
+
+Acceptable comments answer **why**, not **what**, and only when the *why* is non-obvious and cannot be expressed by renaming:
+- Workarounds for compiler/runtime bugs (link the bug).
+- Performance constraints invisible from the code shape ("inner loop runs 50M times per typecheck").
+- Cross-file invariants the code itself can't enforce.
+
+If you are tempted to write `// This is intentional`, change the code so the intent is structural, not decorative.
+
+## Code shape
+
+- Compact, idiomatic F#: pattern matching over `if`/`elif` ladders; active patterns where they remove duplication.
+- Low cyclomatic complexity per function. Extract helpers — even one-line ones — when a name clarifies a step.
+- Prefer module-level `let` over big bodies with nested locals.
+- New file > bloating an existing 5000-line file when adding a self-contained concept.
+
+## Test code
+
+Tests get a touch more leeway for explanation, but the same rules largely apply:
+
+- One parametrized test (`[<Theory>]`, `[<InlineData>]`, or a `for/yield` over inputs) > five copy-pasted tests.
+- Module-level constants for shared paths (`Path.Combine` for OS neutrality), shared source strings, and shared expected outputs.
+- Helpers like `parseAndCheck code` over reinventing the setup per test.
+- Don't reinvent an entire `.fs` file inside each test when one shared module-level binding will do.
+
+## PR scope
+
+**Not paid by LOC.** Large PRs are typically shitty PRs. If the diff has 1000+ lines, split it.
+- Cleanup commits separate from feature commits.
+- No "phase tag" / "transitional measure" / "follow-up" comments left behind — either do it now in a follow-up commit, or file an issue. Don't leave breadcrumbs in the code.
+
+## When in doubt
+
+Delete the comment, rename the value, and re-read. If the code is still unclear, *that* is what needs fixing — not the comment.

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

@@ -73,6 +73,7 @@
 * Reference assembly MVIDs are now deterministic across compiler invocations. Previously, `--refout` / `<ProduceReferenceAssembly>true</ProduceReferenceAssembly>` produced a different MVID every build because the implied signature hash used .NET's randomized `String.GetHashCode()`. ([Issue #19751](https://github.com/dotnet/fsharp/issues/19751), [PR #19801](https://github.com/dotnet/fsharp/pull/19801))
 * Parser: recover on unfinished if and binary expressions
 ([PR #19724](https://github.com/dotnet/fsharp/pull/19724))
+* Warn FS3888 when a compiler-semantic attribute on a value/member or type/module is present in the `.fs` but missing from the `.fsi`. Such attributes were previously ignored at the consumer side. Under the `ErrorOnMissingSignatureAttribute` preview language feature, FS3888 is an error. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880))
 * Emit debug points at a stack-empty position ([PR #19877](https://github.com/dotnet/fsharp/pull/19877))
 * Fix spurious XmlDoc warnings (unknown parameter / no documentation for parameter) under `--warnon:3390` when a get/set property documents the full parameter set across both accessors. ([Issue #13684](https://github.com/dotnet/fsharp/issues/13684), [PR #19884](https://github.com/dotnet/fsharp/pull/19884))
 

docs/release-notes/.Language/preview.md

@@ -2,6 +2,7 @@
 
 * Warn (FS3884) when a function or delegate value is used as an interpolated string argument, since it will be formatted via `ToString` rather than being applied. ([PR #19289](https://github.com/dotnet/fsharp/pull/19289))
 * Added `MethodOverloadsCache` language feature (preview) that caches overload resolution results for repeated method calls, significantly improving compilation performance. ([PR #19072](https://github.com/dotnet/fsharp/pull/19072))
+* Added `ErrorOnMissingSignatureAttribute` preview language feature: makes FS3888 (compiler-semantic attribute on the `.fs` but not on the `.fsi`) an error instead of a warning. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880))
 
 ### Fixed
 

docs/release-notes/.VisualStudio/18.vNext.md

@@ -1,3 +1,7 @@
+### Added
+
+* Code-fixes for FS3888 (compiler-semantic attribute on the `.fs` but not the `.fsi`): copy the attribute into the `.fsi`, or remove it from the `.fs`. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880))
+
 ### Fixed
 
 * Fixed Rename incorrectly renaming `get` and `set` keywords for properties with explicit accessors. ([Issue #18270](https://github.com/dotnet/fsharp/issues/18270), [PR #19252](https://github.com/dotnet/fsharp/pull/19252))

src/Compiler/AbstractIL/il.fsi

@@ -854,6 +854,7 @@ type ILAttribElem =
 type ILAttributeNamedArg = string * ILType * bool * ILAttribElem
 
 /// Custom attribute.
+[<RequireQualifiedAccess>]
 type ILAttribute =
     /// Attribute with args encoded to a binary blob according to ECMA-335 II.21 and II.23.3.
     /// 'decodeILAttribData' is used to parse the byte[] blob to ILAttribElem's as best as possible.
@@ -973,6 +974,7 @@ type internal ILSecurityAction =
     | InheritanceDemandChoice
     | DemandChoice
 
+[<RequireQualifiedAccess>]
 type internal ILSecurityDecl = ILSecurityDecl of ILSecurityAction * byte[]
 
 /// Abstract type equivalent to ILSecurityDecl list - use helpers
@@ -1041,6 +1043,7 @@ type MethodBody =
     | NotAvailable
 
 /// Generic parameters.  Formal generic parameter declarations may include the bounds, if any, on the generic parameter.
+[<NoEquality; NoComparison>]
 type ILGenericParameterDef =
     {
         Name: string

src/Compiler/AbstractIL/ilprint.fs

@@ -333,7 +333,7 @@ and goutput_permission _env os p =
              | ILSecurityAction.DemandChoice -> "demandchoice")
 
     match p with
-    | ILSecurityDecl(sa, b) ->
+    | ILSecurityDecl.ILSecurityDecl(sa, b) ->
         output_string os " .permissionset "
         output_security_action os sa
         output_string os " = ("

src/Compiler/AbstractIL/ilread.fs

@@ -3358,7 +3358,7 @@ and securityDeclsReader ctxtH tag =
         |> List.toArray)
 
 and seekReadSecurityDecl ctxt (act, ty) =
-    ILSecurityDecl(
+    ILSecurityDecl.ILSecurityDecl(
         (if List.memAssoc (int act) (Lazy.force ILSecurityActionRevMap) then
              List.assoc (int act) (Lazy.force ILSecurityActionRevMap)
          else

src/Compiler/AbstractIL/ilwrite.fs

@@ -1506,7 +1506,7 @@ and GenCustomAttrsPass3Or4 cenv hca (attrs: ILAttributes) =
 // ILSecurityDecl --> DeclSecurity rows
 // -------------------------------------------------------------------- *)
 
-let rec GetSecurityDeclRow cenv hds (ILSecurityDecl (action, s)) =
+let rec GetSecurityDeclRow cenv hds (ILSecurityDecl.ILSecurityDecl (action, s)) =
     UnsharedRow
         [| UShort (uint16 (List.assoc action (Lazy.force ILSecurityActionMap)))
            HasDeclSecurity (fst hds, snd hds)

src/Compiler/Checking/NameResolution.fs

@@ -1990,7 +1990,7 @@ let ItemsAreEffectivelyEqual g orig other =
          | TType_var (tp1, _), TType_var (tp2, _) ->
             not tp1.IsCompilerGenerated && not tp1.IsFromError &&
             not tp2.IsCompilerGenerated && not tp2.IsFromError &&
-            equals tp1.Range tp2.Range
+            Range.equals tp1.Range tp2.Range
          | AbbrevOrAppTy(tcref1, _), AbbrevOrAppTy(tcref2, _) ->
             tyconRefDefnEq g tcref1 tcref2
          | _ -> false)

src/Compiler/Checking/SignatureConformance.fs

@@ -16,6 +16,7 @@ open FSharp.Compiler.InfoReader
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.SyntaxTreeOps
 open FSharp.Compiler.Text
+open FSharp.Compiler.TcGlobals
 open FSharp.Compiler.TypedTree
 open FSharp.Compiler.TypedTreeBasics
 open FSharp.Compiler.TypedTreeOps
@@ -41,6 +42,131 @@ exception InterfaceNotRevealed of DisplayEnv * TType * range
 
 exception ArgumentsInSigAndImplMismatch of sigArg: Ident * implArg: Ident
 
+type private V = WellKnownValAttributes
+type private E = WellKnownEntityAttributes
+
+module private AttributeConformance =
+
+    // Each row: a well-known attribute that is read off the .fsi by the
+    // typechecker / IDE. Runtime-only attributes (DllImport, ReflectedDefinition,
+    // SkipLocalsInit, ...) deliberately not listed.
+    let private enforcedVals : V list = [
+        V.NoDynamicInvocationAttribute_True ||| V.NoDynamicInvocationAttribute_False
+        V.RequiresExplicitTypeArgumentsAttribute
+        V.ConditionalAttribute
+        V.NoEagerConstraintApplicationAttribute
+        V.GeneralizableValueAttribute
+        V.WarnOnWithoutNullArgumentAttribute
+        V.CLIEventAttribute
+        V.ExtensionAttribute
+        V.ParamArrayAttribute
+        V.LiteralAttribute
+    ]
+
+    let private enforcedEntities : E list = [
+        E.RequireQualifiedAccessAttribute
+        // AutoOpen on .fs alone is a no-op for F# consumers (.fsi wins).
+        E.NoComparisonAttribute
+        E.NoEqualityAttribute
+        // StructuralEquality / StructuralComparison are documentary for consumers
+        // (F# default already generates them); load-bearing only as impl-side FS1176/FS1177.
+        E.CustomEqualityAttribute
+        E.CustomComparisonAttribute
+        E.ReferenceEqualityAttribute
+        E.AbstractClassAttribute
+        E.SealedAttribute_True ||| E.SealedAttribute_False
+        E.CLIMutableAttribute
+        E.AllowNullLiteralAttribute_True ||| E.AllowNullLiteralAttribute_False
+        E.DefaultAugmentationAttribute_True ||| E.DefaultAugmentationAttribute_False
+        E.ObsoleteAttribute
+        E.CompilerMessageAttribute
+        E.ExperimentalAttribute
+        E.UnverifiableAttribute
+        E.EditorBrowsableAttribute
+        E.AttributeUsageAttribute
+        E.IsByRefLikeAttribute
+        E.IsReadOnlyAttribute
+        E.ExtensionAttribute
+        E.MeasureAttribute
+        E.StructAttribute
+        E.ClassAttribute
+        E.InterfaceAttribute
+    ]
+
+    let private enforcedValsMask     : V = List.reduce Flags.union enforcedVals
+    let private enforcedEntitiesMask : E = List.reduce Flags.union enforcedEntities
+
+    // `AutoOpenAttribute` -> `"AutoOpen"`, `SealedAttribute_True ||| _False` -> `"Sealed"`.
+    let inline private displayName< ^T when ^T : enum<uint64> > (flag: ^T) : string =
+        let v = LanguagePrimitives.EnumToValue flag
+        let lsb : ^T = LanguagePrimitives.EnumOfValue (v &&& (0uL - v))
+        let s = string lsb
+        let s = if s.EndsWith "_True"  then s.Substring(0, s.Length - 5)
+                elif s.EndsWith "_False" then s.Substring(0, s.Length - 6)
+                else s
+        if s.EndsWith "Attribute" then s.Substring(0, s.Length - 9) else s
+
+    // Point the squiggle at the offending impl attribute, not the value/type identifier.
+    let inline private rangeOfMissing
+        (classify: Attrib -> 'F)
+        (attribs: Attrib list)
+        (bits: 'F)
+        (fallback: range)
+        : range =
+        match attribs |> List.tryFind (fun a -> classify a |> Flags.intersects bits) with
+        | Some a -> a.Range
+        | None -> fallback
+
+    let inline private checkEnforced
+        (emit: exn -> unit)
+        (enforcedFlagsOn: 'Subject -> 'F)
+        (policy: 'F list)
+        (attribsOf: 'Subject -> Attrib list)
+        (classify: Attrib -> 'F)
+        (displayNameOf: 'Subject -> string)
+        (impl: 'Subject) (sig': 'Subject) (fallback: range) =
+        let presentOnImplAndRequiredFromSig = enforcedFlagsOn impl
+        if not (Flags.isEmpty presentOnImplAndRequiredFromSig) then
+            let actuallyPresentInSig = enforcedFlagsOn sig'
+            if not (presentOnImplAndRequiredFromSig |> Flags.isSubsetOf actuallyPresentInSig) then
+                let missing = presentOnImplAndRequiredFromSig |> Flags.except actuallyPresentInSig
+                let implAttribs = attribsOf impl
+                for flag in policy do
+                    if flag |> Flags.intersects missing then
+                        let m = rangeOfMissing classify implAttribs flag fallback
+                        emit(Error (FSComp.SR.implAttributeMissingFromSignature(displayName flag, displayNameOf impl), m))
+
+    let private emitter (g: TcGlobals) : exn -> unit =
+        if g.langVersion.SupportsFeature LanguageFeature.ErrorOnMissingSignatureAttribute then
+            errorR
+        else
+            warning
+
+    let checkVal (g: TcGlobals) (implVal: Val) (sigVal: Val) (fallback: range) =
+        let enforcedFlagsOnVal (v: Val) =
+            ValHasWellKnownAttribute g enforcedValsMask v |> ignore // forceload
+            v.ValAttribs.Flags |> Flags.intersect enforcedValsMask
+        checkEnforced (emitter g) enforcedFlagsOnVal enforcedVals
+            (fun (v: Val)    -> v.Attribs)
+            (classifyValAttrib g)
+            (fun (v: Val)    -> v.DisplayName)
+            implVal sigVal fallback
+
+    let checkEntity (g: TcGlobals) (implEntity: Entity) (sigEntity: Entity) (fallback: range) =
+        // Skip when the sig declares a non-module entity with hidden representation
+        // (e.g. `type internal T` opaque in .fsi vs DU in .fs): structural-equality
+        // / NoEquality attributes can't be applied to opaque type declarations.
+        // Modules report IsHiddenReprTycon=true but accept RequireQualifiedAccess.
+        if sigEntity.IsModuleOrNamespace || not sigEntity.IsHiddenReprTycon then
+            let enforcedFlagsOnEntity (e: Entity) =
+                EntityHasWellKnownAttribute g enforcedEntitiesMask e |> ignore // forceload
+                e.EntityAttribs.Flags |> Flags.intersect enforcedEntitiesMask
+            checkEnforced (emitter g) enforcedFlagsOnEntity enforcedEntities
+                (fun (e: Entity) -> e.Attribs)
+                (classifyEntityAttrib g)
+                (fun (e: Entity) -> e.DisplayName)
+                implEntity sigEntity fallback
+
 exception DefinitionsInSigAndImplNotCompatibleAbbreviationsDiffer of
     denv: DisplayEnv *
     implTycon:Tycon *
@@ -126,6 +252,14 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) =
             fixup (sigAttribs @ keptImplAttribs)
             true
 
+        // Pull the enforcement-relevant subset of an Entity's / Val's flags,
+        // forcing the cached `Flags` to be populated as a side-effect.
+        let checkEnforcedEntityAttribs implEntity sigEntity m =
+            AttributeConformance.checkEntity g implEntity sigEntity m
+
+        let checkEnforcedValAttribs implVal sigVal m =
+            AttributeConformance.checkVal g implVal sigVal m
+
         let rec checkTypars m (aenv: TypeEquivEnv) (implTypars: Typars) (sigTypars: Typars) = 
             if implTypars.Length <> sigTypars.Length then 
                 errorR (Error(FSComp.SR.typrelSigImplNotCompatibleParamCountsDiffer(), m)) 
@@ -185,6 +319,12 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) =
             // Propagate defn location information from implementation to signature . 
             sigTycon.SetOtherRange (implTycon.Range, true)
             implTycon.SetOtherRange (sigTycon.Range, false)
+
+            // Enforce that compiler-semantic entity attributes present on the
+            // implementation are also present on the signature. See
+            // `signatureEnforcedEntityAttribs` for the list and rationale.
+            // Emitted as a warning (will become an error in a future F# version).
+            checkEnforcedEntityAttribs implTycon sigTycon m
 
             if implTycon.LogicalName <> sigTycon.LogicalName then 
                 errorR (Error (FSComp.SR.DefinitionsInSigAndImplNotCompatibleNamesDiffer(implTycon.TypeOrMeasureKind.ToString(), sigTycon.LogicalName, implTycon.LogicalName), m))
@@ -368,6 +508,8 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) =
             let mk_err kind denv f = ValueNotContained(kind,denv, infoReader, implModRef, implVal, sigVal, f)
             let err denv f = errorR(mk_err RegularMismatch denv f); false
             let m = implVal.Range
+
+            checkEnforcedValAttribs implVal sigVal m
             if implVal.IsMutable <> sigVal.IsMutable then (err denv FSComp.SR.ValueNotContainedMutabilityAttributesDiffer)
             elif implVal.LogicalName <> sigVal.LogicalName then (err denv FSComp.SR.ValueNotContainedMutabilityNamesDiffer)
             elif (implVal.CompiledName g.CompilerGlobalState) <> (sigVal.CompiledName g.CompilerGlobalState) then (err denv FSComp.SR.ValueNotContainedMutabilityCompiledNamesDiffer)
@@ -753,6 +895,10 @@ type Checker(g, amap, denv, remapInfo: SignatureRepackageInfo, checkingSig) =
             // Propagate defn location information from implementation to signature . 
             sigModRef.SetOtherRange (implModRef.Range, true)
             implModRef.Deref.SetOtherRange (sigModRef.Range, false)
+            // Enforce consumer-visible compiler-semantic attributes on the module
+            // itself (e.g. [<AutoOpen>] on a nested module). Same rationale as
+            // checkTypeDef. Emitted as a warning (will become an error later).
+            checkEnforcedEntityAttribs implModRef.Deref sigModRef implModRef.Range
             checkModuleOrNamespaceContents implModRef.Range aenv infoReader implModRef sigModRef.ModuleOrNamespaceType &&
             checkAttribs aenv implModRef.Attribs sigModRef.Attribs implModRef.Deref.SetAttribs
 

src/Compiler/CodeGen/EraseUnions.fs

@@ -694,9 +694,9 @@ let private rewriteNullableAttrForFlattenedField (g: TcGlobals) (existingAttrs:
         let replacementAttr =
             match existingAttrs[idx] with
             // Single byte: change non-nullable (1) to WithNull (2); leave nullable (2) and ambivalent (0) as-is
-            | Encoded(method, _data, [ ILAttribElem.Byte 1uy ]) -> mkILCustomAttribMethRef (method, [ ILAttribElem.Byte 2uy ], [])
+            | ILAttribute.Encoded(method, _data, [ ILAttribElem.Byte 1uy ]) -> mkILCustomAttribMethRef (method, [ ILAttribElem.Byte 2uy ], [])
             // Array of bytes: change first element only (field itself); leave generic type arg nullability unchanged
-            | Encoded(method, _data, [ ILAttribElem.Array(elemType, ILAttribElem.Byte 1uy :: otherElems) ]) ->
+            | ILAttribute.Encoded(method, _data, [ ILAttribElem.Array(elemType, ILAttribElem.Byte 1uy :: otherElems) ]) ->
                 mkILCustomAttribMethRef (method, [ ILAttribElem.Array(elemType, (ILAttribElem.Byte 2uy) :: otherElems) ], [])
             | attrAsBefore -> attrAsBefore