Teach the compiler how to discover more kinds of parameter names

[baronfel]

These commits enable the following script to compile using the locally-built fsi and the --warnon:3900 flag without warning about missing parameters, fixing #10809:

type Thing = Inner of string

/// <summary>does the thing</summary>
/// <param name="s">the thing to do!</param>
let doer (Thing.Inner s) = ()

/// <summary>gives you a friendly greeting</summary>
/// <param name="pal">your good mate</param>
type Buddy(pal: string) = 
    do
        printfn $"hi, {pal}"

The first commit expands the set of type constructors that TcTyconDefnCore_Phase1A_BuildInitialTycon knows how to extract pattern names for to include TyconUnspecified, because that's what came through when debugging the above script. I am unsure if this is the correct Tycon to be looking for here, but the overall pattern traversal to extract names seems easy enough.

The second commit is a bit more ambiguous to me. I've changed the way SynValData is inferred for bindings (specifically the constructor of a SynSimplePat from a SynPat) so that a Paren(LongIdent(pat)) can be constructed. That is, applications like (Thing.Inner s) will return a simple pat with the name of the inner application, in this case s. I am unsure if this is a) correct, and b) safe because I recognize that applying a LongIdent to the incoming value in this case is like saying let (Thing.Inner s) = anon_incoming_value and returning the inner s, but I'm trying to strike a balance between the usability of the doc-checking feature and the correctness of my change.

Finally, I'm unsure where tests for these sorts of changes should go and would appreciate any pointers in that direction.

[baronfel]

I pushed a different approach to discovering argument names for LongIdent patterns that doesn't seem to change as much of the processing logic. Instead of changing how the simple pat laterF is created, I instead taught SimplePatOfPat how to find the name of a single inner pattern when part of an LongIdent pattern. This seemed safer because we still delegate to the generated continuation function for actually accessing/generating the value.

[NinoFloris]

Nice! closely related to an issue I opened a month ago #10514

[baronfel]

@NinoFloris excellent! Glad to get more pointers as to where I should be looking.

As we can see from the sea of red x's for the PR checks, it's clear the compiler isn't quite happy with this kind of change as of yet. I think it's more reasonable to focus my efforts on the as <name> kinds of patterns instead of trying to coerce the name of a pattern that could be incredibly nested (think in Active Pattern usages), and so I think I'll begin to touch on your issue more as I start looking that way.

[cartermp]

@baronfel you can add a few tests here: https://github.com/dotnet/fsharp/blob/main/tests/FSharp.Compiler.ComponentTests/Language/XmlComments.fs

there should be an option to explicitly expect no diagnostics, which would accomplish the goal

[baronfel]

Thanks, done!

[baronfel]

I'm wondering if my constructor fix should even go in, because from this preexisting test it seems like the intended design was to require explicit documentation of the implicit constructor, which was a thing I didn't know was possible.

[cartermp]

Hmmm. Yeah, that just sounds like a weird quirk to me. The large majority of class declarations use primary constructors on the same line and they should just also be checked as-is.