Skip to content

Integrated XML documentation reading in FCS #11454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Apr 27, 2021
Merged

Integrated XML documentation reading in FCS #11454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1e0a2d7
Initial work for proper xml doc info
TIHan Apr 20, 2021
1b51919
Implementing IXmlDocumentationInfoLoader
TIHan Apr 20, 2021
4d78a2f
Added TryFindXmlDocumentationInfo to AssemblyLoader
TIHan Apr 20, 2021
f548501
Able to get xml comments
TIHan Apr 20, 2021
c0cc45e
Minor refactor
TIHan Apr 20, 2021
e90ca48
More refactoring
TIHan Apr 21, 2021
eedd83e
Fixing build
TIHan Apr 21, 2021
456327a
Plumbing infoReader and updating surface area
TIHan Apr 21, 2021
0a6943e
Do not create a local entity ref here
TIHan Apr 21, 2021
92245d0
Removed extracting XML from resources as that is not correct
TIHan Apr 21, 2021
dc68e93
Ignore parameter
TIHan Apr 21, 2021
125613e
More cleanup
TIHan Apr 21, 2021
cf27f51
layoutXmlDoc formats well with new-lines
TIHan Apr 21, 2021
7f18105
layout xml doc for values
TIHan Apr 22, 2021
c667bfc
Setting up more layout functions
TIHan Apr 22, 2021
c56b34f
Using new layouts
TIHan Apr 22, 2021
b185d27
All xml is being emitted
TIHan Apr 22, 2021
4dbe2ce
fixing a few tests
TIHan Apr 23, 2021
832e2d6
Fixing test
TIHan Apr 27, 2021
0452aa8
Fixing a test
TIHan Apr 27, 2021
d86f111
fixing tests
TIHan Apr 27, 2021
9fee21c
Fixing tests
TIHan Apr 27, 2021
63ece44
Better caching for xml documentation
TIHan Apr 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/fsharp/AugmentWithHashCompare.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ open FSharp.Compiler.AbstractIL.IL
open FSharp.Compiler.ErrorLogger
open FSharp.Compiler.Infos
open FSharp.Compiler.Syntax
open FSharp.Compiler.Xml
open FSharp.Compiler.TcGlobals
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeBasics
Expand Down
13 changes: 8 additions & 5 deletions src/fsharp/CheckDeclarations.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ open FSharp.Compiler.Syntax.PrettyNaming
open FSharp.Compiler.SyntaxTreeOps
open FSharp.Compiler.Text
open FSharp.Compiler.Text.Range
open FSharp.Compiler.Xml
Copy link
Contributor

@dsyme dsyme May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice the addition of this namespace. I don't think we should be using it as it's not about XML as a thing, but about F# documentation

In any case I'd like to make sure all FCS API changes get carefully reviewed - please make sure I sign off on them for now - we need this to be iterating towards being fully stable

Copy link
Contributor

@dsyme dsyme May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the logic in moving out of "Syntax" since it's not purely syntax but also additional logic.

Perhaps either of these:

namespace FSharp.Compiler.XmlDocumentation
namespace FSharp.Compiler.XmlDocs

though we could also look forward to if we one day support Markdown docs, in which case

namespace FSharp.Compiler.CodeDocumentation

may be best (to also have corresponding types like MarkdownDoc etc.)

open FSharp.Compiler.TcGlobals
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeBasics
Expand Down Expand Up @@ -5717,7 +5718,7 @@ let ApplyDefaults (cenv: cenv) g denvAtEnd m mexpr extraAttribs =
ConstraintSolver.ChooseTyparSolutionAndSolve cenv.css denvAtEnd tp)
with e -> errorRecovery e m

let CheckValueRestriction denvAtEnd rootSigOpt implFileTypePriorToSig m =
let CheckValueRestriction denvAtEnd infoReader rootSigOpt implFileTypePriorToSig m =
if Option.isNone rootSigOpt then
let rec check (mty: ModuleOrNamespaceType) =
for v in mty.AllValsAndMembers do
Expand All @@ -5731,7 +5732,7 @@ let CheckValueRestriction denvAtEnd rootSigOpt implFileTypePriorToSig m =
// for example FSharp 1.0 3661.
(match v.ValReprInfo with None -> true | Some tvi -> tvi.HasNoArgs)) then
match ftyvs with
| tp :: _ -> errorR (ValueRestriction(denvAtEnd, false, v, tp, v.Range))
| tp :: _ -> errorR (ValueRestriction(denvAtEnd, infoReader, false, v, tp, v.Range))
| _ -> ()
mty.ModuleAndNamespaceDefinitions |> List.iter (fun v -> check v.ModuleOrNamespaceType)
try check implFileTypePriorToSig with e -> errorRecovery e m
Expand Down Expand Up @@ -5762,15 +5763,15 @@ let CheckModuleSignature g (cenv: cenv) m denvAtEnd rootSigOpt implFileTypePrior
// As typechecked the signature and implementation use different tycons etc.
// Here we (a) check there are enough names, (b) match them up to build a renaming and
// (c) check signature conformance up to this renaming.
if not (SignatureConformance.CheckNamesOfModuleOrNamespace denv (mkLocalTyconRef implFileSpecPriorToSig) sigFileType) then
if not (SignatureConformance.CheckNamesOfModuleOrNamespace denv cenv.infoReader (mkLocalTyconRef implFileSpecPriorToSig) sigFileType) then
raise (ReportedError None)

// Compute the remapping from implementation to signature
let remapInfo, _ = ComputeRemappingFromInferredSignatureToExplicitSignature cenv.g implFileTypePriorToSig sigFileType

let aenv = { TypeEquivEnv.Empty with EquivTycons = TyconRefMap.OfList remapInfo.RepackagedEntities }

if not (SignatureConformance.Checker(cenv.g, cenv.amap, denv, remapInfo, true).CheckSignature aenv (mkLocalModRef implFileSpecPriorToSig) sigFileType) then (
if not (SignatureConformance.Checker(cenv.g, cenv.amap, denv, remapInfo, true).CheckSignature aenv cenv.infoReader (mkLocalModRef implFileSpecPriorToSig) sigFileType) then (
// We can just raise 'ReportedError' since CheckModuleOrNamespace raises its own error
raise (ReportedError None)
)
Expand All @@ -5795,6 +5796,8 @@ let TypeCheckOneImplFile
(rootSigOpt: ModuleOrNamespaceType option)
(ParsedImplFileInput (_, isScript, qualNameOfFile, scopedPragmas, _, implFileFrags, isLastCompiland)) =

let infoReader = InfoReader(g, amap)

eventually {
let cenv =
cenv.Create (g, isScript, niceNameGen, amap, topCcu, false, Option.isSome rootSigOpt,
Expand Down Expand Up @@ -5838,7 +5841,7 @@ let TypeCheckOneImplFile

// Check the value restriction. Only checked if there is no signature.
conditionallySuppressErrorReporting (checkForErrors()) (fun () ->
CheckValueRestriction denvAtEnd rootSigOpt implFileTypePriorToSig m)
CheckValueRestriction denvAtEnd infoReader rootSigOpt implFileTypePriorToSig m)

// Solve unsolved internal type variables
conditionallySuppressErrorReporting (checkForErrors()) (fun () ->
Expand Down
11 changes: 6 additions & 5 deletions src/fsharp/CheckExpressions.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ open FSharp.Compiler.TcGlobals
open FSharp.Compiler.Text
open FSharp.Compiler.Text.Position
open FSharp.Compiler.Text.Range
open FSharp.Compiler.Xml
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeBasics
open FSharp.Compiler.TypedTreeOps
Expand Down Expand Up @@ -78,7 +79,7 @@ exception UnitTypeExpectedWithPossibleAssignment of DisplayEnv * TType * bool *
exception UnitTypeExpectedWithPossiblePropertySetter of DisplayEnv * TType * string * string * range
exception UnionPatternsBindDifferentNames of range
exception VarBoundTwice of Ident
exception ValueRestriction of DisplayEnv * bool * Val * Typar * range
exception ValueRestriction of DisplayEnv * InfoReader * bool * Val * Typar * range
exception ValNotMutable of DisplayEnv * ValRef * range
exception ValNotLocal of DisplayEnv * ValRef * range
exception InvalidRuntimeCoercion of DisplayEnv * TType * TType * range
Expand Down Expand Up @@ -2573,7 +2574,7 @@ let TcValEarlyGeneralizationConsistencyCheck cenv (env: TcEnv) (v: Val, vrec, ti
let tau3 = instType (mkTyparInst tpsorig tinst) tau2
//printfn "tau3 = '%s'" (DebugPrint.showType tau3)
if not (AddCxTypeEqualsTypeUndoIfFailed env.DisplayEnv cenv.css m tau tau3) then
let txt = bufs (fun buf -> NicePrint.outputQualifiedValSpec env.DisplayEnv buf v)
let txt = bufs (fun buf -> NicePrint.outputQualifiedValSpec env.DisplayEnv cenv.infoReader buf (mkLocalValRef v))
error(Error(FSComp.SR.tcInferredGenericTypeGivesRiseToInconsistency(v.DisplayName, txt), m)))
| _ -> ()

Expand Down Expand Up @@ -6230,7 +6231,7 @@ and FreshenObjExprAbstractSlot cenv (env: TcEnv) (implty: TType) virtNameAndArit
match absSlots with
| [] when not (CompileAsEvent cenv.g bindAttribs) ->
let absSlotsByName = List.filter (fst >> fst >> (=) bindName) virtNameAndArityPairs
let getSignature absSlot = (NicePrint.stringOfMethInfo cenv.amap mBinding env.DisplayEnv absSlot).Replace("abstract ", "")
let getSignature absSlot = (NicePrint.stringOfMethInfo cenv.infoReader mBinding env.DisplayEnv absSlot).Replace("abstract ", "")
let getDetails (absSlot: MethInfo) =
if absSlot.GetParamTypes(cenv.amap, mBinding, []) |> List.existsSquared (isAnyTupleTy cenv.g) then
FSComp.SR.tupleRequiredInAbstractMethod()
Expand Down Expand Up @@ -6506,7 +6507,7 @@ and TcObjectExpr cenv overallTy env tpenv (synObjTy, argopt, binds, extraImpls,
overridesAndVirts |> List.iter (fun (m, implty, dispatchSlots, dispatchSlotsKeyed, availPriorOverrides, overrides) ->
let overrideSpecs = overrides |> List.map fst

DispatchSlotChecking.CheckOverridesAreAllUsedOnce (env.DisplayEnv, cenv.g, cenv.amap, true, implty, dispatchSlotsKeyed, availPriorOverrides, overrideSpecs)
DispatchSlotChecking.CheckOverridesAreAllUsedOnce (env.DisplayEnv, cenv.g, cenv.infoReader, true, implty, dispatchSlotsKeyed, availPriorOverrides, overrideSpecs)

DispatchSlotChecking.CheckDispatchSlotsAreImplemented (env.DisplayEnv, cenv.infoReader, m, env.NameEnv, cenv.tcSink, false, implty, dispatchSlots, availPriorOverrides, overrideSpecs) |> ignore)

Expand Down Expand Up @@ -9908,7 +9909,7 @@ and ApplyAbstractSlotInference (cenv: cenv) (envinner: TcEnv) (bindingTy, m, syn
| [] ->
let details =
slots
|> Seq.map (NicePrint.stringOfMethInfo cenv.amap m envinner.DisplayEnv)
|> Seq.map (NicePrint.stringOfMethInfo cenv.infoReader m envinner.DisplayEnv)
|> Seq.map (sprintf "%s %s" System.Environment.NewLine)
|> String.concat ""

Expand Down
3 changes: 2 additions & 1 deletion src/fsharp/CheckExpressions.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ open FSharp.Compiler.Syntax
open FSharp.Compiler.SyntaxTreeOps
open FSharp.Compiler.TcGlobals
open FSharp.Compiler.Text
open FSharp.Compiler.Xml
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeOps

Expand Down Expand Up @@ -130,7 +131,7 @@ exception UnitTypeExpectedWithPossibleAssignment of DisplayEnv * TType * bool *
exception FunctionValueUnexpected of DisplayEnv * TType * range
exception UnionPatternsBindDifferentNames of range
exception VarBoundTwice of Ident
exception ValueRestriction of DisplayEnv * bool * Val * Typar * range
exception ValueRestriction of DisplayEnv * InfoReader * bool * Val * Typar * range
exception ValNotMutable of DisplayEnv * ValRef * range
exception ValNotLocal of DisplayEnv * ValRef * range
exception InvalidRuntimeCoercion of DisplayEnv * TType * TType * range
Expand Down
5 changes: 5 additions & 0 deletions src/fsharp/CompilerConfig.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ open FSharp.Compiler.IO
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Text
open FSharp.Compiler.Text.Range
open FSharp.Compiler.Xml
open FSharp.Compiler.TypedTree

#if !NO_EXTENSIONTYPING
Expand Down Expand Up @@ -487,6 +488,8 @@ type TcConfigBuilder =
mutable pathMap: PathMap

mutable langVersion: LanguageVersion

mutable xmlDocInfoLoader: IXmlDocumentationInfoLoader option
}


Expand Down Expand Up @@ -662,6 +665,7 @@ type TcConfigBuilder =
useFsiAuxLib = isInteractive
rangeForErrors = rangeForErrors
sdkDirOverride = sdkDirOverride
xmlDocInfoLoader = None
}

member tcConfigB.FxResolver =
Expand Down Expand Up @@ -1033,6 +1037,7 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) =
member x.tryGetMetadataSnapshot = data.tryGetMetadataSnapshot
member x.internalTestSpanStackReferring = data.internalTestSpanStackReferring
member x.noConditionalErasure = data.noConditionalErasure
member x.xmlDocInfoLoader = data.xmlDocInfoLoader

static member Create(builder, validate) =
use unwindBuildPhase = PushThreadBuildPhaseUntilUnwind BuildPhase.Parameter
Expand Down
5 changes: 5 additions & 0 deletions src/fsharp/CompilerConfig.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ open System
open Internal.Utilities
open Internal.Utilities.Library
open FSharp.Compiler
open FSharp.Compiler.Xml
open FSharp.Compiler.AbstractIL
open FSharp.Compiler.AbstractIL.IL
open FSharp.Compiler.AbstractIL.ILBinaryReader
Expand Down Expand Up @@ -273,6 +274,8 @@ type TcConfigBuilder =
mutable pathMap : PathMap

mutable langVersion : LanguageVersion

mutable xmlDocInfoLoader : IXmlDocumentationInfoLoader option
}

static member CreateNew:
Expand Down Expand Up @@ -440,6 +443,8 @@ type TcConfig =
member isInteractive: bool
member isInvalidationSupported: bool

member xmlDocInfoLoader: IXmlDocumentationInfoLoader option

member FxResolver: FxResolver

member ComputeLightSyntaxInitialStatus: string -> bool
Expand Down
42 changes: 22 additions & 20 deletions src/fsharp/CompilerDiagnostics.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ let GetRangeOfDiagnostic(err: PhasedDiagnostic) =
| IntfImplInIntrinsicAugmentation m
| OverrideInExtrinsicAugmentation m
| IntfImplInExtrinsicAugmentation m
| ValueRestriction(_, _, _, _, m)
| ValueRestriction(_, _, _, _, _, m)
| LetRecUnsound (_, _, m)
| ObsoleteError (_, m)
| ObsoleteWarning (_, m)
Expand All @@ -156,10 +156,10 @@ let GetRangeOfDiagnostic(err: PhasedDiagnostic) =
| TyconBadArgs(_, _, _, m) ->
Some m

| FieldNotContained(_, arf, _, _) -> Some arf.Range
| ValueNotContained(_, _, aval, _, _) -> Some aval.Range
| ConstrNotContained(_, aval, _, _) -> Some aval.Id.idRange
| ExnconstrNotContained(_, aexnc, _, _) -> Some aexnc.Range
| FieldNotContained(_, _, _, arf, _, _) -> Some arf.Range
| ValueNotContained(_, _, _, aval, _, _) -> Some aval.Range
| ConstrNotContained(_, _, _, aval, _, _) -> Some aval.Id.idRange
| ExnconstrNotContained(_, _, aexnc, _, _) -> Some aexnc.Range

| VarBoundTwice id
| UndefinedName(_, _, id, _) ->
Expand Down Expand Up @@ -803,7 +803,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa
sprintf " // %s" nameOrOneBasedIndexMessage
| _ -> ""

(NicePrint.stringOfMethInfo x.amap m displayEnv x.methodSlot.Method) + paramInfo
(NicePrint.stringOfMethInfo x.infoReader m displayEnv x.methodSlot.Method) + paramInfo

let nl = System.Environment.NewLine
let formatOverloads (overloads: OverloadInformation list) =
Expand Down Expand Up @@ -1348,18 +1348,20 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa
| UnionPatternsBindDifferentNames _ ->
os.Append(UnionPatternsBindDifferentNamesE().Format) |> ignore

| ValueNotContained (denv, mref, implVal, sigVal, f) ->
let text1, text2 = NicePrint.minimalStringsOfTwoValues denv implVal sigVal
| ValueNotContained (denv, infoReader, mref, implVal, sigVal, f) ->
let text1, text2 = NicePrint.minimalStringsOfTwoValues denv infoReader (mkLocalValRef implVal) (mkLocalValRef sigVal)
os.Append(f((fullDisplayTextOfModRef mref), text1, text2)) |> ignore

| ConstrNotContained (denv, v1, v2, f) ->
os.Append(f((NicePrint.stringOfUnionCase denv v1), (NicePrint.stringOfUnionCase denv v2))) |> ignore
| ConstrNotContained (denv, infoReader, enclosingTycon, v1, v2, f) ->
let enclosingTcref = mkLocalEntityRef enclosingTycon
os.Append(f((NicePrint.stringOfUnionCase denv infoReader enclosingTcref v1), (NicePrint.stringOfUnionCase denv infoReader enclosingTcref v2))) |> ignore

| ExnconstrNotContained (denv, v1, v2, f) ->
os.Append(f((NicePrint.stringOfExnDef denv v1), (NicePrint.stringOfExnDef denv v2))) |> ignore
| ExnconstrNotContained (denv, infoReader, v1, v2, f) ->
os.Append(f((NicePrint.stringOfExnDef denv infoReader (mkLocalEntityRef v1)), (NicePrint.stringOfExnDef denv infoReader (mkLocalEntityRef v2)))) |> ignore

| FieldNotContained (denv, v1, v2, f) ->
os.Append(f((NicePrint.stringOfRecdField denv v1), (NicePrint.stringOfRecdField denv v2))) |> ignore
| FieldNotContained (denv, infoReader, enclosingTycon, v1, v2, f) ->
let enclosingTcref = mkLocalEntityRef enclosingTycon
os.Append(f((NicePrint.stringOfRecdField denv infoReader enclosingTcref v1), (NicePrint.stringOfRecdField denv infoReader enclosingTcref v2))) |> ignore

| RequiredButNotSpecified (_, mref, k, name, _) ->
let nsb = new System.Text.StringBuilder()
Expand Down Expand Up @@ -1510,19 +1512,19 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa

| MissingFields(sl, _) -> os.Append(MissingFieldsE().Format (String.concat "," sl + ".")) |> ignore

| ValueRestriction(denv, hassig, v, _, _) ->
| ValueRestriction(denv, infoReader, hassig, v, _, _) ->
let denv = { denv with showImperativeTyparAnnotations=true }
let tau = v.TauType
if hassig then
if isFunTy denv.g tau && (arityOfVal v).HasNoArgs then
os.Append(ValueRestriction1E().Format
v.DisplayName
(NicePrint.stringOfQualifiedValOrMember denv v)
(NicePrint.stringOfQualifiedValOrMember denv infoReader (mkLocalValRef v))
v.DisplayName) |> ignore
else
os.Append(ValueRestriction2E().Format
v.DisplayName
(NicePrint.stringOfQualifiedValOrMember denv v)
(NicePrint.stringOfQualifiedValOrMember denv infoReader (mkLocalValRef v))
v.DisplayName) |> ignore
else
match v.MemberInfo with
Expand All @@ -1533,17 +1535,17 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa
| SynMemberKind.Constructor -> true (* can't infer extra polymorphism *)
| _ -> false (* can infer extra polymorphism *)
end ->
os.Append(ValueRestriction3E().Format (NicePrint.stringOfQualifiedValOrMember denv v)) |> ignore
os.Append(ValueRestriction3E().Format (NicePrint.stringOfQualifiedValOrMember denv infoReader (mkLocalValRef v))) |> ignore
| _ ->
if isFunTy denv.g tau && (arityOfVal v).HasNoArgs then
os.Append(ValueRestriction4E().Format
v.DisplayName
(NicePrint.stringOfQualifiedValOrMember denv v)
(NicePrint.stringOfQualifiedValOrMember denv infoReader (mkLocalValRef v))
v.DisplayName) |> ignore
else
os.Append(ValueRestriction5E().Format
v.DisplayName
(NicePrint.stringOfQualifiedValOrMember denv v)
(NicePrint.stringOfQualifiedValOrMember denv infoReader (mkLocalValRef v))
v.DisplayName) |> ignore


Expand Down
Loading