Redesign XmlDoc collecting mechanism (#11973)

* wip * let bindings and 'extern' support * fix SurfaceArea * fix for val & member val * tests + fixes * yet another tests * _ * simplify * test for implicit ctor * fix * improve test * move grabXmlDoc to the ParseHelpers * diagnostic * initial impl # Conflicts: # src/fsharp/SyntaxTreeOps.fs # src/fsharp/SyntaxTreeOps.fsi # src/fsharp/fsi/fsi.fs # src/fsharp/pars.fsy * add in test that verifies the SyntaxTree representation # Conflicts: # tests/service/Symbols.fs * generate sigs and write docs for union case properties # Conflicts: # src/fsharp/XmlDocFileWriter.fs * implement xmldocs for anon fields as well # Conflicts: # src/fsharp/SyntaxTreeOps.fs # src/fsharp/SyntaxTreeOps.fsi # src/fsharp/XmlDocFileWriter.fs # src/fsharp/pars.fsy # tests/service/Symbols.fs * minor fixes * fix xml-doc in FSharp.Core * fix tests * fix docs in FSharp.Compiler.Service * yet another comment fix * try to fix tests * yet another test fix * compilation fix Co-authored-by: Chet Husk <chusk3@gmail.com>
dotnet · Sep 17, 2021 · 85fe280 · 85fe280
1 parent 780c91c
commit 85fe280
Show file tree

Hide file tree

Showing 100 changed files with 1,488 additions and 489 deletions.
diff --git a/src/fsharp/CheckDeclarations.fs b/src/fsharp/CheckDeclarations.fs
@@ -483,9 +483,9 @@ module TcRecdUnionAndEnumDeclarations =
                     | Some fieldId, Parent tcref ->
                         let item = Item.UnionCaseField (UnionCaseInfo (thisTyInst, UnionCaseRef (tcref, id.idText)), i)
                         CallNameResolutionSink cenv.tcSink (fieldId.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurence.Binding, env.AccessRights)
-                    | _ -> ()
-
-                    TcAnonFieldDecl cenv env parent tpenv (mkUnionCaseFieldName nFields i) fld)
+                        TcNamedFieldDecl cenv env parent false tpenv fld
+                    | _ ->
+                        TcAnonFieldDecl cenv env parent tpenv (mkUnionCaseFieldName nFields i) fld)
                 ValidateFieldNames(flds, rfields)
 
                 rfields, thisTy
@@ -878,12 +878,21 @@ module IncrClassChecking =
                 not arity.HasNoArgs && not v.IsMutable
 
 
+        /// <summary>
         /// Choose how a binding is represented
-        member localRep.ChooseRepresentation (cenv: cenv, env: TcEnv, isStatic, isCtorArg, 
-                                              ctorInfo: IncrClassCtorLhs, 
-                                              /// The vars forced to be fields due to static member bindings, instance initialization expressions or instance member bindings
-                                              staticForcedFieldVars: FreeLocals, 
-                                              /// The vars forced to be fields due to instance member bindings
+        /// </summary>
+        /// <param name='cenv'></param>
+        /// <param name='env'></param>
+        /// <param name='isStatic'></param>
+        /// <param name='isCtorArg'></param>
+        /// <param name='ctorInfo'></param>
+        /// <param name='staticForcedFieldVars'>The vars forced to be fields due to static member bindings, instance initialization expressions or instance member bindings</param>
+        /// <param name='instanceForcedFieldVars'>The vars forced to be fields due to instance member bindings</param>
+        /// <param name='takenFieldNames'></param>
+        /// <param name='bind'></param>
+        member localRep.ChooseRepresentation (cenv: cenv, env: TcEnv, isStatic, isCtorArg,
+                                              ctorInfo: IncrClassCtorLhs,
+                                              staticForcedFieldVars: FreeLocals,
                                               instanceForcedFieldVars: FreeLocals, 
                                               takenFieldNames: Set<string>, 
                                               bind: Binding) = 
@@ -1091,9 +1100,9 @@ module IncrClassChecking =
         /// Given localRep saying how locals have been represented, e.g. as fields.
         /// Given an expr under a given thisVal context.
         //
-        /// Fix up the references to the locals, e.g. 
-        ///     v -> this.fieldv
-        ///     f x -> this.method x
+        // Fix up the references to the locals, e.g.
+        //     v -> this.fieldv
+        //     f x -> this.method x
         member localRep.FixupIncrClassExprPhase2C cenv thisValOpt safeStaticInitInfo (thisTyInst: TypeInst) expr = 
             // fixup: intercept and expr rewrite
             let FixupExprNode rw e =
@@ -1143,21 +1152,27 @@ module IncrClassChecking =
       | Phase2CCtorJustAfterSuperInit     
       | Phase2CCtorJustAfterLastLet    
 
-    /// Given a set of 'let' bindings (static or not, recursive or not) that make up a class, 
-    /// generate their initialization expression(s).  
-    let MakeCtorForIncrClassConstructionPhase2C 
+    /// <summary>
+    /// Given a set of 'let' bindings (static or not, recursive or not) that make up a class,
+    /// generate their initialization expression(s).
+    /// </summary>
+    /// <param name='cenv'></param>
+    /// <param name='env'></param>
+    /// <param name='ctorInfo'>The lhs information about the implicit constructor</param>
+    /// <param name='inheritsExpr'>The call to the super class constructor</param>
+    /// <param name='inheritsIsVisible'>Should we place a sequence point at the 'inheritedTys call?</param>
+    /// <param name='decs'>The declarations</param>
+    /// <param name='memberBinds'></param>
+    /// <param name='generalizedTyparsForRecursiveBlock'>Record any unconstrained type parameters generalized for the outer members as "free choices" in the let bindings</param>
+    /// <param name='safeStaticInitInfo'></param>
+    let MakeCtorForIncrClassConstructionPhase2C
                (cenv: cenv, 
-                env: TcEnv, 
-                /// The lhs information about the implicit constructor
-                ctorInfo: IncrClassCtorLhs, 
-                /// The call to the super class constructor
-                inheritsExpr, 
-                /// Should we place a sequence point at the 'inheritedTys call?
-                inheritsIsVisible, 
-                /// The declarations
+                env: TcEnv,
+                ctorInfo: IncrClassCtorLhs,
+                inheritsExpr,
+                inheritsIsVisible,
                 decs: IncrClassConstructionBindingsPhase2C list, 
-                memberBinds: Binding list, 
-                /// Record any unconstrained type parameters generalized for the outer members as "free choices" in the let bindings 
+                memberBinds: Binding list,
                 generalizedTyparsForRecursiveBlock, 
                 safeStaticInitInfo: SafeInitData) = 
 
@@ -1459,7 +1474,7 @@ module IncrClassChecking =
             ctorBody
 
         let cctorBodyOpt =
-            /// Omit the .cctor if it's empty 
+            // Omit the .cctor if it's empty
             match cctorInitActions with
             | [] -> None 
             | _ -> 
@@ -3586,7 +3601,7 @@ module EstablishTypeDefinitionCores =
     /// Note that for 
     ///              type PairOfInts = int * int
     /// then after running this phase and checking for cycles, operations 
-    // such as 'isRefTupleTy' will return reliable results, e.g. isRefTupleTy on the 
+    /// such as 'isRefTupleTy' will return reliable results, e.g. isRefTupleTy on the
     /// TAST type for 'PairOfInts' will report 'true' 
     //
     let private TcTyconDefnCore_Phase1C_Phase1E_EstablishAbbreviations (cenv: cenv) envinner inSig tpenv pass (MutRecDefnsPhase1DataForTycon(_, synTyconRepr, _, _, _, _)) (tycon: Tycon) (attrs: Attribs) =

diff --git a/src/fsharp/CheckExpressions.fs b/src/fsharp/CheckExpressions.fs
@@ -1031,8 +1031,8 @@ let noArgOrRetAttribs = ArgAndRetAttribs ([], [])
 /// shares the same code paths (e.g. TcLetBinding and TcLetrec) as processing expression bindings (such as "let x = 1 in ...")
 /// Member bindings also use this path.
 //
-/// However there are differences in how different bindings get processed,
-/// i.e. module bindings get published to the implicitly accumulated module type, but expression 'let' bindings don't.
+// However there are differences in how different bindings get processed,
+// i.e. module bindings get published to the implicitly accumulated module type, but expression 'let' bindings don't.
 type DeclKind =
     | ModuleOrMemberBinding
 
@@ -5971,7 +5971,7 @@ and TcExprUndelayed cenv (overallTy: OverallTy) env tpenv (synExpr: SynExpr) =
         let e3', tpenv = TcExpr cenv overallTy env tpenv e3
         Expr.StaticOptimization (constraints', e2', e3', m), tpenv
 
-    /// e1.longId <- e2
+    // e1.longId <- e2
     | SynExpr.DotSet (e1, (LongIdentWithDots(longId, _) as lidwd), e2, mStmt) ->
         if lidwd.ThereIsAnExtraDotAtTheEnd then
             // just drop rhs on the floor
@@ -5981,11 +5981,11 @@ and TcExprUndelayed cenv (overallTy: OverallTy) env tpenv (synExpr: SynExpr) =
             let mExprAndDotLookup = unionRanges e1.Range (rangeOfLid longId)
             TcExprThen cenv overallTy env tpenv false e1 [DelayedDotLookup(longId, mExprAndDotLookup); MakeDelayedSet(e2, mStmt)]
 
-    /// e1 <- e2
+    // e1 <- e2
     | SynExpr.Set (e1, e2, mStmt) ->
         TcExprThen cenv overallTy env tpenv false e1 [MakeDelayedSet(e2, mStmt)]
 
-    /// e1.longId(e2) <- e3, very rarely used named property setters
+    // e1.longId(e2) <- e3, very rarely used named property setters
     | SynExpr.DotNamedIndexedPropertySet (e1, (LongIdentWithDots(longId, _) as lidwd), e2, e3, mStmt) ->
         if lidwd.ThereIsAnExtraDotAtTheEnd then
             // just drop rhs on the floor

diff --git a/src/fsharp/CheckExpressions.fsi b/src/fsharp/CheckExpressions.fsi
@@ -46,10 +46,10 @@ val InitialImplicitCtorInfo: unit -> CtorInfo
 [<NoEquality; NoComparison>]
 type UngeneralizableItem
 
-[<NoEquality; NoComparison>]
 /// Represents the type environment at a particular scope. Includes the name
 /// resolution environment, the ungeneralizable items from earlier in the scope
 /// and other information about the scope.
+[<NoEquality; NoComparison>]
 type TcEnv =
     { /// Name resolution information 
       eNameResEnv: NameResolutionEnv 

diff --git a/src/fsharp/CompilerConfig.fs b/src/fsharp/CompilerConfig.fs
@@ -860,8 +860,8 @@ type TcConfigBuilder =
 // TcConfig
 //--------------------------------------------------------------------------
 
-[<Sealed>]
 /// This type is immutable and must be kept as such. Do not extract or mutate the underlying data except by cloning it.
+[<Sealed>]
 type TcConfig private (data: TcConfigBuilder, validate: bool) =
 
     // Validate the inputs - this helps ensure errors in options are shown in visual studio rather than only when built

diff --git a/src/fsharp/CompilerImports.fsi b/src/fsharp/CompilerImports.fsi
@@ -117,8 +117,8 @@ type ImportedAssembly =
     }
 
 
-[<Sealed>] 
 /// Tables of assembly resolutions
+[<Sealed>]
 type TcAssemblyResolutions = 
 
     member GetAssemblyResolutions: unit -> AssemblyResolution list

diff --git a/src/fsharp/ConstraintSolver.fs b/src/fsharp/ConstraintSolver.fs
@@ -143,8 +143,8 @@ let FreshenMethInfo m (minfo: MethInfo) =
 // Subsumption of types: solve/record subtyping constraints
 //------------------------------------------------------------------------- 
 
-[<RequireQualifiedAccess>] 
 /// Information about the context of a type equation.
+[<RequireQualifiedAccess>]
 type ContextInfo =
 
     /// No context was given.

diff --git a/src/fsharp/ConstraintSolver.fsi b/src/fsharp/ConstraintSolver.fsi
@@ -43,8 +43,8 @@ val FreshenTypars: range -> Typars -> TType list
 
 val FreshenMethInfo: range -> MethInfo -> TType list
 
-[<RequireQualifiedAccess>] 
 /// Information about the context of a type equation.
+[<RequireQualifiedAccess>]
 type ContextInfo =
 
     /// No context was given.

diff --git a/src/fsharp/FSComp.txt b/src/fsharp/FSComp.txt
@@ -1629,3 +1629,4 @@ reprStateMachineInvalidForm,"The state machine has an unexpected form"
 3517,optFailedToInlineSuggestedValue,"The value '%s' was marked 'InlineIfLambda' but was not determined to have a lambda value. This warning is for informational purposes only."
 3518,implMissingInlineIfLambda,"The 'InlineIfLambda' attribute is present in the signature but not the implementation."
 3519,tcInlineIfLambdaUsedOnNonInlineFunctionOrMethod,"The 'InlineIfLambda' attribute may only be used on parameters of inlined functions of methods whose type is a function or F# delegate type."
+3520,invalidXmlDocPosition,"XML comment is not placed on a valid language element."
diff --git a/src/fsharp/FSharp.Core/FSharp.Core.fsproj b/src/fsharp/FSharp.Core/FSharp.Core.fsproj
@@ -17,6 +17,8 @@
 	<OtherFlags>$(OtherFlags) --warnon:1182</OtherFlags>
 	<!-- 3390: xmlDocBadlyFormed -->
 	<OtherFlags>$(OtherFlags) --warnon:3390</OtherFlags>
+    <!-- 3520: invalidXmlDocPosition -->
+    <OtherFlags>$(OtherFlags) --warnon:3520</OtherFlags>
     <!-- Turn off 57: Use of construct with Experimental attribute -->
 	<OtherFlags>$(OtherFlags) --nowarn:57</OtherFlags>
     <!-- Turn off 3511: state machine not compilable - expected for inlined functions defining state machine generators -->

diff --git a/src/fsharp/FSharp.Core/Linq.fs b/src/fsharp/FSharp.Core/Linq.fs
@@ -497,7 +497,7 @@ module LeafExpressionConverter =
             // or type T to an argument expecting Expression<T>.
             | ImplicitExpressionConversionHelperQ (_, [_], [x1]) -> ConvExprToLinqInContext env x1
 
-            /// Use witnesses if they are available
+            // Use witnesses if they are available
             | CallWithWitnesses (objArgOpt, _, minfo2, witnessArgs, args) -> 
                 let fullArgs = witnessArgs @ args
                 let replacementExpr =

diff --git a/src/fsharp/FSharp.Core/Query.fsi b/src/fsharp/FSharp.Core/Query.fsi
@@ -10,15 +10,15 @@ namespace Microsoft.FSharp.Linq
     open System.Collections
     open System.Collections.Generic
 
-    [<NoComparison; NoEquality; Sealed>]
     /// <summary>
     /// A partial input or result in an F# query. This type is used to support the F# query syntax.
     /// </summary>
     ///
     /// <namespacedoc><summary>
-    ///   Library functionality for F# query syntax and interoperability with .NET LINQ Expressions. See 
+    ///   Library functionality for F# query syntax and interoperability with .NET LINQ Expressions. See
     ///   also <a href="https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/query-expressions">F# Query Expressions</a> in the F# Language Guide.
     /// </summary></namespacedoc>
+    [<NoComparison; NoEquality; Sealed>]
     type QuerySource<'T, 'Q> =
         /// <summary>
         /// A method used to support the F# query syntax.  
@@ -30,9 +30,9 @@ namespace Microsoft.FSharp.Linq
         /// </summary>
         member Source: seq<'T>
 
-    [<Class>]
-    /// The type used to support the F# query syntax. Use 'query { ... }' to use the query syntax. See 
+    /// The type used to support the F# query syntax. Use 'query { ... }' to use the query syntax. See
     /// also <a href="https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/query-expressions">F# Query Expressions</a> in the F# Language Guide.
+    [<Class>]
     type QueryBuilder =
         /// <summary>Create an instance of this builder. Use 'query { ... }' to use the query syntax.</summary>
         new: unit -> QueryBuilder