Address signature generation bugs

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

@@ -27,6 +27,12 @@
 * Fix box instruction for literal upcasts. (Issue [#18319](https://github.com/dotnet/fsharp/issues/18319), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
 * Fix Decimal Literal causes InvalidProgramException in Debug builds. (Issue [#18956](https://github.com/dotnet/fsharp/issues/18956), [PR #19338](https://github.com/dotnet/fsharp/pull/19338))
 * Fix `AttributeUsage.AllowMultiple` not being inherited for attributes subclassed in C#. ([Issue #17107](https://github.com/dotnet/fsharp/issues/17107), [PR #19315](https://github.com/dotnet/fsharp/pull/19315))
+* Fix signature generation: recursive module `do` binding leaking compiler-generated val. ([Issue #13832](https://github.com/dotnet/fsharp/issues/13832), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
+* Fix signature generation: literal values in attribute arguments now preserve literal identifier name. ([Issue #13810](https://github.com/dotnet/fsharp/issues/13810), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
+* Fix signature generation: struct types with non-comparable/non-equatable fields now include `[<NoComparison>]`/`[<NoEquality>]`. ([Issue #15339](https://github.com/dotnet/fsharp/issues/15339), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
+* Fix signature generation: backtick escaping for identifiers containing backticks. ([Issue #15389](https://github.com/dotnet/fsharp/issues/15389), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
+* Fix signature generation: `private` keyword placement for prefix-style type abbreviations. ([Issue #15560](https://github.com/dotnet/fsharp/issues/15560), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
+* Fix signature generation: missing `[<Class>]` attribute for types without visible constructors. ([Issue #16531](https://github.com/dotnet/fsharp/issues/16531), [PR #19586](https://github.com/dotnet/fsharp/pull/19586))
 
 ### Added
 

src/Compiler/Checking/CheckDeclarations.fs

@@ -5900,7 +5900,7 @@ let CheckOneImplFile
 
         // Warn on version attributes.
         topAttrs.assemblyAttrs |> List.iter (function
-           | Attrib(tref, _, [ AttribExpr(Expr.Const (Const.String version, range, _), _) ], _, _, _, _) ->
+           | Attrib(tref, _, [ AttribExpr(_, Expr.Const (Const.String version, range, _)) ], _, _, _, _) ->
                 let attrName = tref.CompiledRepresentationForNamedType.FullName
                 let isValid() =
                     try parseILVersion version |> ignore; true

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -11628,8 +11628,37 @@ and TcAttributeEx canFail (cenv: cenv) (env: TcEnv) attrTgt attrEx (synAttr: Syn
 
                 UnifyTypes cenv env mAttr ty (tyOfExpr g expr)
 
+                // Collect literal val references from the syntax expression to recover original names.
+                // TcVal inlines literal vals to Expr.Const, losing the original val reference.
+                // We recover it here by matching ranges from the syntax tree against the checked expression.
+                let literalIdents =
+                    let rec collect (synExpr: SynExpr) acc =
+                        match synExpr with
+                        | SynExpr.Ident ident ->
+                            match env.NameEnv.eUnqualifiedItems |> Map.tryFind ident.idText with
+                            | Some(Item.Value vref) when vref.LiteralValue.IsSome ->
+                                (ident.idRange, vref) :: acc
+                            | _ -> acc
+                        | SynExpr.Paren(expr = inner) -> collect inner acc
+                        | SynExpr.Tuple(exprs = exprs) -> List.fold (fun a e -> collect e a) acc exprs
+                        | SynExpr.App(_, _, funcExpr, argExpr, _) -> collect funcExpr (collect argExpr acc)
+                        | _ -> acc
+
+                    collect arg []
+
                 let mkAttribExpr e =
-                    AttribExpr(e, EvalLiteralExprOrAttribArg g e)
+                    let sourceExpr =
+                        match e with
+                        | Expr.Const(_, m, _) ->
+                            match literalIdents |> List.tryFind (fun (r, _) -> Range.equals r m) with
+                            // Only use Expr.Val for local refs to avoid creating transitive assembly
+                            // dependencies in pickled metadata (VRefNonLocal would require the
+                            // external assembly to be available when importing this DLL).
+                            | Some(_, vref) when vref.IsLocalRef -> Expr.Val(vref, NormalValUse, m)
+                            | _ -> e
+                        | _ -> e
+
+                    AttribExpr(sourceExpr, EvalLiteralExprOrAttribArg g e)
 
                 let checkPropSetterAttribAccess m (pinfo: PropInfo) =
                     let setterMeth = pinfo.SetterMethod

src/Compiler/Checking/NicePrint.fs

@@ -561,8 +561,10 @@ module PrintTypes =
     /// Layout a single attribute arg, following the cases of 'gen_attr_arg' in ilxgen.fs
     /// This is the subset of expressions we display in the NicePrint pretty printer 
     /// See also dataExprL - there is overlap between these that should be removed 
-    let rec layoutAttribArg denv arg = 
-        match arg with 
+    let rec layoutAttribArg denv arg =
+        match arg with
+        | Expr.Val (vref, _, _) when vref.LiteralValue.IsSome -> wordL (tagLocal vref.DisplayName)
+
         | Expr.Const (c, _, ty) -> 
             if isEnumTy denv.g ty then 
                 WordL.keywordEnum ^^ angleL (layoutType denv ty) ^^ bracketL (layoutConst denv.g ty c)
@@ -1986,6 +1988,47 @@ module TastDefinitionPrinting =
         let isMeasure = (tycon.TypeOrMeasureKind = TyparKind.Measure)
         let ty = generalizedTyconRef g tcref 
 
+        // Augment tycon.Attribs with synthetic [<NoComparison>] / [<NoEquality>] when the type
+        // is a candidate for comparison/equality augmentation but augmentation was not generated
+        // (e.g. struct with non-comparable fields). This ensures generated signatures compile. (#15339)
+        let augmentedAttribs =
+            let isTrueFSharpStruct =
+                tycon.IsFSharpStructOrEnumTycon && not tycon.IsFSharpEnumTycon
+
+            // Only structs need synthetic NoComparison/NoEquality in signatures.
+            // Reference types (records, unions) compile fine without them.
+            let canBeAugmentedWithCompare = isTrueFSharpStruct
+            let canBeAugmentedWithEquals = isTrueFSharpStruct
+
+            let mkSyntheticCoreAttrib (attrName: string) =
+                let fsharpCorePath = [| "Microsoft"; "FSharp"; "Core" |]
+                let attrTcref = mkNonLocalTyconRef (mkNonLocalEntityRef g.fslibCcu fsharpCorePath) attrName
+
+                let ilTypeRef =
+                    ILTypeRef.Create(g.ilg.fsharpCoreAssemblyScopeRef, [], "Microsoft.FSharp.Core." + attrName)
+
+                let ilMethodRef =
+                    ILMethodRef.Create(ilTypeRef, ILCallingConv.Instance, ".ctor", 0, [], ILType.Void)
+
+                Attrib(attrTcref, ILAttrib ilMethodRef, [], [], false, None, Range.range0)
+
+            let mutable attribs = tycon.Attribs
+
+            if canBeAugmentedWithCompare
+               && not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.NoComparisonAttribute tycon)
+               && not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.CustomComparisonAttribute tycon)
+               && tycon.GeneratedCompareToValues.IsNone then
+                attribs <- mkSyntheticCoreAttrib "NoComparisonAttribute" :: attribs
+
+            if canBeAugmentedWithEquals
+               && not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.NoEqualityAttribute tycon)
+               && not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.CustomEqualityAttribute tycon)
+               && not (EntityHasWellKnownAttribute g WellKnownEntityAttributes.ReferenceEqualityAttribute tycon)
+               && tycon.GeneratedHashAndEqualsValues.IsNone then
+                attribs <- mkSyntheticCoreAttrib "NoEqualityAttribute" :: attribs
+
+            attribs
+
         let start, tagger =
             if isStructTy g ty && not tycon.TypeAbbrev.IsSome then
                 // Always show [<Struct>] whether verbose or not
@@ -1998,7 +2041,7 @@ module TastDefinitionPrinting =
             elif isMeasure then
                 None, tagClass
             elif isClassTy g ty then
-                if denv.printVerboseSignatures then
+                if denv.showAttributes then
                     (if simplified then None else Some "class"), tagClass
                 else
                     None, tagClass
@@ -2009,18 +2052,18 @@ module TastDefinitionPrinting =
             if isFirstType then
                 WordL.keywordType
             else
-                wordL (tagKeyword "and") ^^ layoutAttribs denv start false tycon.TypeOrMeasureKind tycon.Attribs emptyL
+                wordL (tagKeyword "and") ^^ layoutAttribs denv start false tycon.TypeOrMeasureKind augmentedAttribs emptyL
 
         let nameL = ConvertLogicalNameToDisplayLayout (tagger >> mkNav tycon.DefinitionRange >> wordL) tycon.DisplayNameCore
 
-        let nameL = layoutAccessibility denv tycon.Accessibility nameL
-        let denv = denv.AddAccessibility tycon.Accessibility 
-
         let lhsL =
             let tps = tycon.TyparsNoRange
             let tpsL = layoutTyparDecls denv nameL tycon.IsPrefixDisplay tps
+            let tpsL = layoutAccessibility denv tycon.Accessibility tpsL
             typewordL ^^ tpsL
 
+        let denv = denv.AddAccessibility tycon.Accessibility
+
 
         let sortKey (minfo: MethInfo) = 
             (not minfo.IsConstructor,
@@ -2197,15 +2240,15 @@ module TastDefinitionPrinting =
         let needsStartEnd =
             match start with 
             | Some "class" ->
+                // When allDecls is empty, the repr layout produces 'class end' which is sufficient
+                not (isNil allDecls) &&
                 // 'inherits' is not enough for F# type kind inference to infer a class
                 // inherits.IsEmpty &&
                 ilFields.IsEmpty &&
                 // 'abstract' is not enough for F# type kind inference to infer a class by default in signatures
                 // 'static member' is surprisingly not enough for F# type kind inference to infer a class by default in signatures
                 // 'overrides' is surprisingly not enough for F# type kind inference to infer a class by default in signatures
-                //(meths |> List.forall (fun m -> m.IsAbstract || m.IsDefiniteFSharpOverride || not m.IsInstance)) &&
-                //(props |> List.forall (fun m -> (not m.HasGetter || m.GetterMethod.IsAbstract))) &&
-                //(props |> List.forall (fun m -> (not m.HasSetter || m.SetterMethod.IsAbstract))) &&
+                // Concrete instance methods and properties are also not enough (Error 938)
                 ctors.IsEmpty &&
                 instanceVals.IsEmpty &&
                 staticVals.IsEmpty
@@ -2384,7 +2427,7 @@ module TastDefinitionPrinting =
                 |> addLhs
 
         typeDeclL 
-        |> fun tdl -> if isFirstType then layoutAttribs denv start false tycon.TypeOrMeasureKind tycon.Attribs tdl else tdl
+        |> fun tdl -> if isFirstType then layoutAttribs denv start false tycon.TypeOrMeasureKind augmentedAttribs tdl else tdl
         |> layoutXmlDocOfEntity denv infoReader tcref 
 
     // Layout: exception definition
@@ -2551,7 +2594,7 @@ module InferredSigPrinting =
         let rec imdefsL denv x = aboveListL (x |> List.map (imdefL denv))
 
         and imdefL denv x = 
-            let filterVal (v: Val) = not v.IsCompilerGenerated && Option.isNone v.MemberInfo
+            let filterVal (v: Val) = not v.IsCompilerGenerated && Option.isNone v.MemberInfo && not (v.LogicalName.StartsWithOrdinal("doval@"))
             let filterExtMem (v: Val) = v.IsExtensionMember
 
             match x with 

src/Compiler/SyntaxTree/PrettyNaming.fs

@@ -515,8 +515,8 @@ let DoesIdentifierNeedBackticks (name: string) : bool =
 let AddBackticksToIdentifierIfNeeded (name: string) : string =
     if
         DoesIdentifierNeedBackticks name
-        && not (name.StartsWithOrdinal("`"))
-        && not (name.EndsWithOrdinal("`"))
+        && not (name.StartsWithOrdinal("``"))
+        && not (name.EndsWithOrdinal("``"))
     then
         "``" + name + "``"
     else

src/Compiler/TypedTree/TypedTreePickle.fs

@@ -2875,7 +2875,18 @@ and p_attribkind x st =
 and p_attrib (Attrib(a, b, c, d, e, _targets, f)) st = // AttributeTargets are not preserved
     p_tup6 (p_tcref "attrib") p_attribkind (p_list p_attrib_expr) (p_list p_attrib_arg) p_bool p_dummy_range (a, b, c, d, e, f) st
 
-and p_attrib_expr (AttribExpr(e1, e2)) st = p_tup2 p_expr p_expr (e1, e2) st
+and p_attrib_expr (AttribExpr(e1, e2)) st =
+    // Normalize Expr.Val back to Expr.Const before pickling.
+    // The literal name recovery (Expr.Val in source field) is an in-memory optimization
+    // for signature generation display. We must not change the pickle format, because
+    // old compilers reading Expr.Val in this position would show degraded attribute display.
+    let e1 =
+        match e1 with
+        | Expr.Val(vref, _, m) when vref.LiteralValue.IsSome ->
+            Expr.Const(vref.LiteralValue.Value, m, vref.Type)
+        | _ -> e1
+
+    p_tup2 p_expr p_expr (e1, e2) st
 
 and p_attrib_arg (AttribNamedArg(a, b, c, d)) st =
     p_tup4 p_string p_ty p_bool p_attrib_expr (a, b, c, d) st

tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/misc/misc.fs

@@ -11,12 +11,14 @@ open System.IO
 module help_options =
 
     // ReqENU	SOURCE=dummy.fsx COMPILE_ONLY=1  PRECMD="\$FSC_PIPE >help40.txt -?     2>&1" POSTCMD="\$FSI_PIPE --nologo --quiet --exec ..\\..\\..\\comparer.fsx help40.txt help40.437.1033.bsl"	# -?
+    // Reset enableConsoleColoring before help tests to avoid global mutable state
+    // pollution from concurrent tests (e.g., ``fsc --consolecolors switch``).
     [<Fact>]
     let ``Help - variant 1``() =
         FSharp ""
         |> asExe
         |> withBufferWidth 120
-        |> withOptions ["-?"]
+        |> withOptions ["--consolecolors+"; "-?"]
         |> compile
         |> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
         |> shouldSucceed
@@ -27,7 +29,7 @@ module help_options =
         FSharp ""
         |> asExe
         |> withBufferWidth 120
-        |> withOptions ["/?"]
+        |> withOptions ["--consolecolors+"; "/?"]
         |> compile
         |> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
         |> shouldSucceed
@@ -38,7 +40,7 @@ module help_options =
         FSharp ""
         |> asExe
         |> withBufferWidth 120
-        |> withOptions ["--help"]
+        |> withOptions ["--consolecolors+"; "--help"]
         |> compile
         |> verifyOutputWithBaseline (Path.Combine(__SOURCE_DIRECTORY__, "compiler_help_output.bsl"))
         |> shouldSucceed