Allow type providers to report diagnostics for static parameters #14483

kerams · 2022-12-17T12:52:04Z

Implements fsharp/fslang-suggestions#155.

@baronfel raised this question:

Can the TPs define their own warning codes instead of piggybacking on the FS ones? And/or provide URLs as well. This is all allowed in existing MSBuild diagnostics and can help out users to find detailed docs

Currently all diagnostics are implicitly FS-prefixed. Maybe we could allow TPs to send custom codes and URLs to the compiler, which would just ignore those and use FS3033 and FS3088 until custom codes are supported? Just so that the TP interface won't have to keep changing.

Analysis of issues and open questions, XML docs (once the API is ironed out), tests incoming.

kerams · 2022-12-17T12:56:30Z

src/Compiler/Checking/CheckExpressions.fs

+                    match kind with
+                    | SynStringKind.Regular -> 1, 1
+                    | SynStringKind.Verbatim -> 2, 1
+                    | SynStringKind.TripleQuote -> 3, 3


Triple quoted verbatim strings do not have their own kind, so the diagnostic range becomes off by one. Not sure it's worth adding, they are fairly rare.

@abonie : I assume you plan to touch the parsing and tree representation of string kinds anyway, could you account for this as well please?
i.e. the ability to cover the range(s) involved precisely, and perharps even Trivia(s) to give ranges per line in case of multiline strings.

kerams · 2022-12-17T13:00:19Z

src/FSharp.Core/fslib-extra-pervasives.fsi

+
+    type ITypeProvider3 =
+        abstract ApplyStaticArguments : context:ITypeProviderDiagnosticsContext * typeWithoutArguments:Type * typePathWithArguments:string[] * staticArguments:obj[] -> Type 
+
+        abstract ApplyStaticArgumentsForMethod:
+            context: ITypeProviderDiagnosticsContext * methodWithoutArguments: MethodBase * methodNameWithArguments: string * staticArguments: obj[] -> MethodBase


@dsyme, I couldn't extend ITypeProvider as you had suggested, because TPs that rely on the old version of the interface in their ProvidedTypes.fs would fail to load. There's ITypeProvider2 that allows to tack on additional functionality, so I opted for the same approach.

Now that I test backwards compatibility with VS which includes FSharp.Core 7.0 without these additions, ITypeProvider3 can't be loaded and the TP doesn't work. Even if I ditch the interface and use reflection to access the methods, there are 2 new types we now depend on. I'm at a loss here.

Can ITypeProvider3 extend ITypeProvider2 ?

Yes, but I don't see how that would solve the problem (i.e. the interface missing in FSharp.Core that an older compiler will use, causing a type load exception).

kerams · 2022-12-17T13:11:39Z

tests/service/data/TestTP/ProvidedTypes.fsi

    member DefineStaticParameters: parameters: ProvidedStaticParameter list * instantiationFunction: (string -> obj[] -> ProvidedMethod) -> unit

+    member DefineStaticParameters: parameters: ProvidedStaticParameter list * instantiationFunction: (ProvidedMethod -> string -> obj[] -> ProvidedMethod) -> unit
+
+    member ReportWarning: staticParameterName: string * rangeInParameterIfString: (int * int) option * message: string * throwIfNotSupported: bool -> unit
+
+    member ReportError: staticParameterName: string * rangeInParameterIfString: (int * int) option * message: string -> unit
+


I'm really not happy with adding these 3 methods (and with the reference to self in instantiationFunction in particular), but I could not think of a way to report diagnostics for static method arguments by calling ReportWarning/Error through the provider type itself, as is the case when applying type-level static arguments.

kerams · 2022-12-17T13:16:34Z

src/Compiler/TypedTree/TypeProviders.fs

+            | TypeProviderDiagnosticSeverity.Warning ->
+                Error (FSComp.SR.etProviderWarning (tpDesignation, message), m) |> warning
+            | TypeProviderDiagnosticSeverity.Error ->
+                stopProcessingRecovery (Error (FSComp.SR.etProviderError (tpDesignation, message), m)) m


stopProcessingRecovery allows the TP to continue, but when I tried to use Error, which does take the control away, the diagnostic gets wrapped in additional exception, and the entire TP range gets a squiggly line, hiding what was meant to be highlighted. Not sure how to avoid that.

kerams · 2022-12-17T13:19:29Z

src/FSharp.Core/fslib-extra-pervasives.fsi

+
+    [<AllowNullLiteral>]
+    type ITypeProviderDiagnosticsContext =
+        abstract ReportDiagnostic: staticParameterName: string * rangeInParameterIfString: (int * int) option * message: string * severity: TypeProviderDiagnosticSeverity -> unit


I think it might be a good idea to have ReportDiagnostics, allowing the TP to report multiple errors at once before the control is taken away. For example, multiple problems in a query string could be highlighted at the same time, not one by one as they are fixed.

src/Compiler/TypedTree/TypeProviders.fs

T-Gro · 2022-12-19T08:56:53Z

Could the built-in message template also include the static parameter name?
The more info we can put (as placeholders) into the template, the more unified the experience will be.

Also, the template itself will get localized, whereas the content provided by the TP will not - that is why I would staticParameterName into a sentence in the localized template already.

kerams · 2022-12-19T10:52:10Z

Sure it could, though I'm not sure how useful it would be.

The type provider '{0}' reported a warning: {1}</target>
vs
The type provider '{0}' reported a warning for static parameter '{2}': {1}</target>

The static parameter will already be underlined, so this does not add any new information (outside of an IDE perhaps it does, but it's still of no help without the message from the TP).

src/FSharp.Core/fslib-extra-pervasives.fsi

dsyme · 2023-01-10T14:18:17Z

@kerams Do you know what's causing the current build failures?

 Check Diagnostics (TestProject): [|D:\a\_work\1\s\tests\service/data/TestProject/TestProject.fs (48,11)-(48,66) typecheck error The static parameter 'Query' of the provided type or method 'GenerativeProvider' requires a value. Static parameters to type providers may be optionally specified using named arguments, e.g. 'GenerativeProvider<Query=...>'.;
   D:\a\_work\1\s\tests\service/data/TestProject/TestProject.fs (48,11)-(48,66) typecheck error The static parameter 'Query' of the provided type or method 'GenerativeProvider' requires a value. Static parameters to type providers may be optionally specified using named arguments, e.g. 'GenerativeProvider<Query=...>'.;
   D:\a\_work\1\s\tests\service/data/TestProject/TestProject.fs (51,22)-(51,24) typecheck error The value or constructor 'T2' is not defined.|]

kerams · 2023-01-10T14:25:41Z

@dsyme, yeah, I've made changes to tests/service/data/TestTP/ProvidedTypes.fs (which should go into the TPSDK instead) and the test TP just so that I could try it out in a different solution (that's where the screenshot is from). TestProject.fs, which uses the test TP, remains unchanged, hence the failures caused by missing parameters.

I didn't know how you'd want to test all this (especially the compatibility of FSC vis-a-vis ProvidedTypes.fs versions) and whether the changes to tests/service/data/TestTP/ProvidedTypes.fs would get reverted later on.

dsyme · 2023-01-10T14:28:33Z

I didn't know how you'd want to test all this and whether the changes to tests/service/data/TestTP/ProvidedTypes.fs would get reverted later on.

There are tests for type providers under tests/fsharp/typeProviders, it might be possible to add a simple test there.

It's also good you have prototyped additions to the TPSDK.

kerams · 2023-01-10T14:33:54Z

Will do. It would be great if you could help me sort out the open conversations above in the meantime. Also, as you've mentioned elsewhere, there are several copies of ProvidedTypes lying around the compiler. Should I add the diagnostics support to all of them?

Allow type providers to report diagnostics for static parameters

d8297b2

kerams requested a review from a team as a code owner December 17, 2022 12:52

kerams commented Dec 17, 2022

View reviewed changes

src/Compiler/TypedTree/TypeProviders.fs Show resolved Hide resolved

dsyme reviewed Jan 3, 2023

View reviewed changes

src/FSharp.Core/fslib-extra-pervasives.fsi Show resolved Hide resolved

dsyme reviewed Jan 3, 2023

View reviewed changes

src/FSharp.Core/fslib-extra-pervasives.fsi Outdated Show resolved Hide resolved

Add support for informational diagnostics

b7bc75d

kerams closed this Jul 20, 2023

kerams deleted the tp branch July 20, 2023 13:34

kerams mentioned this pull request May 14, 2025

Scoped nowarn #18049

Merged

4 tasks

Allow type providers to report diagnostics for static parameters #14483

Allow type providers to report diagnostics for static parameters #14483

Uh oh!

Conversation

kerams commented Dec 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kerams Dec 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

T-Gro Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

kerams Dec 17, 2022

Choose a reason for hiding this comment

Uh oh!

kerams Dec 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

T-Gro Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

kerams Dec 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kerams Dec 17, 2022

Choose a reason for hiding this comment

Uh oh!

kerams Dec 17, 2022

Choose a reason for hiding this comment

Uh oh!

kerams Dec 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

T-Gro commented Dec 19, 2022

Uh oh!

kerams commented Dec 19, 2022

Uh oh!

Uh oh!

Uh oh!

dsyme commented Jan 10, 2023

Uh oh!

kerams commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dsyme commented Jan 10, 2023

Uh oh!

kerams commented Jan 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kerams commented Dec 17, 2022 •

edited

Loading

kerams Dec 17, 2022 •

edited

Loading

kerams Dec 18, 2022 •

edited

Loading

kerams Dec 19, 2022 •

edited

Loading

kerams commented Jan 10, 2023 •

edited

Loading

kerams commented Jan 10, 2023 •

edited

Loading