Allow type providers to report location, warnings and informationals to the compiler

[baronfel]

Submitted by Gustavo Guerra on 3/21/2014 12:00:00 AM
11 votes on UserVoice prior to migration

Response

** by fslang-admin on 8/3/2015 12:00:00 AM **

This is approved-in-principle for F# 4.x+
A detailed design is needed. I would prefer one that is idiom-based and doesn’t force type providers to use a later FSharp.Core.dll
Implementations of approved language design items can now be submitted as pull requests to the appropriate branch of http://github.com/Microsoft/visualfsharp. See http://fsharp.github.io/2014/06/18/fsharp-contributions.html for information on contributing to the F# language and core library.
Don Syme, F# Language and Core Library Evolution

Original UserVoice Submission
Archived Uservoice Comments

[kerams]

What is meant by idiom-based? Like some structural convention, e.g. the design-time component throwing an exception with certain properties (static parameter index, range if the parameter is a string, etc.) that type checking could look for? On second thought, this approach would not be viable for warnings.

[kerams]

@dsyme, I'd be interested in tackling this if you could spare a moment to provide some pointers #338 (comment).

[dsyme]

@kerams I would suggest

• In fslib-extra-pervasives.fsi, add a new method to TypeProviderConfig that allows a type provider to report a diagnostic. The data delivered should be essentially the same as PhasedDiagnostic * FSharpDiagnosticSeverity in the F# compiler though since this is in FSharp.Core we shouldn't take a direct dependency on those types.

• On the compiler side, the implementation should convert the delivered information to PhasedDiagnostic * FSharpDiagnosticSeverity and deliver to DiagnosticsLogger.diagnosticSink.

• Once available update the TPSDK

I think that's it really

[dsyme]

BTW it's possible we should use this as a chance to move the TPSDK into dotnet/fsharp, where it really belongs.

[kerams]

Thanks. Doesn't it go against what you said previously though?

I would prefer one that is idiom-based and doesn’t force type providers to use a later FSharp.Core.dll

Will this approach allow me to put a squiggly line below titl with an error saying there is no such column, given that TPs don't internally deal with source code ranges?

type DvdRental = NpgsqlConnection<dvdRental, ReuseProvidedTypes = true>
let getAllFilmsWithRatingsCommand = DvdRental.CreateCommand<"select titl, rating from film">

PhasedDiagnostic comprises Exception and Phase. The latter seems irrelevant in the context of TPs, and exceptions basically carry a message and stack trace, which would be insufficient for more detailed reporting I think.

[smoothdeveloper]

Sorry if this feels tangential:

• type resolution in script file doesn't work for type provider without qualified names dotnet/fsharp#13887 (there is no way to reopen this issue?)
• [WIP] fix for #4956 (TP error reporting) dotnet/fsharp#4978

Troubleshooting type providers, from user surfaced issues, in the specifics (like this issue, assuming it is about error reporting related to static type parameters) or in the overall (when an exception in a TP is raised during a build or in the tooling) would definitely help.

I'm suspecting the later is a bigger hurdle in the way to inspire more people to author and maintain type providers and an easier fix.

[smoothdeveloper]

@dsyme: In general we don't want to show stack traces in error messages. But we do need to improve the diagnostics form TP developers and TP early adopters so they can see stack traces if they want.

[dsyme]

Doesn't it go against what you said previously though?

Yes but I don't think it matters too much. These days F# tooling is now much better at moving to latest FSharp.Core fairly promptly and I'd expect the iteration time for TPs to adopt to be enough for FSharp.Core to appear in latest tooling.

Will this approach allow me to put a squiggly line below titl with an error saying there is no such column, given that TPs don't internally deal with source code ranges?

Ah yes, you're right - what we're primarily after is that TPs should report diagnostic information within static arguments. Indeed arguably the TP should only be allowed to report

• diagnostics within static arguments
• diagnostics for the overall TP static invocation (ApplyStaticArguments, ApplyStaticArgumentsForMethod)
• adhoc failures on use of particular provided methods, properties etc.

It doesn't even really make much sense for TPs to report diagnostics in non-source-code files. In general these would be ignored by IDE tooling anyway (when analyzing a specific document IDEs tend to discard any diagnostics relating to other documents). So I guess it's not worth allowing that.

Note it's not entirely trivial to report failures for TP argument as even a "string" or "int" argument is quite a complex thing syntactically - there may be escapes, unicode characters, triple quotes, literals etc. Even an integer static argument may be a compile-time Literal1 + Literal2 etc. but all the TP sees is the resulting integer. For strings perhaps best is that the TP should report the location range in the logical string argument, and it's up to the compiler to map that location back to a source range.

From an implementation point of view it's hard to arrange such reverse-mapping

logical string range --> source range

if the ReportDiagnostic method is on TypeProviderConfig - how does the implementation of this "know" the context of the invocation - that is which type provider instantiation the thing relates to? But equally it's a little hard to deliver a callback to each and every operation in a TP that may raise a diagnostic - these operations include calls like GetMethods etc.

One alternative would be to deliver the callback to ApplyStaticArguments and ApplyStaticArgumentsForMethod, e.g. the former is today

type ITypeProvider =
    abstract ApplyStaticArguments : typeWithoutArguments:Type * typePathWithArguments:string[] * staticArguments:obj[] -> Type 

but perhaps should be

type ITypeProvider =
    abstract ApplyStaticArguments : context: TypeProviderDiagnosticsContext * typeWithoutArguments:Type * typePathWithArguments:string[] * staticArguments:obj[] -> Type 

type TypeProviderDiagnosticsContext =
    member ReportDiagnostic(...) = ...

At this point it's much easier in the compiler to capture enough information about the static arguments and their original syntax.

[dsyme]

@smoothdeveloper I agree the hurdles for TP developers need to be reduced. I think moving the TPSDK into dotnet/fsharp may be a good start.

[NatElkins]

Not sure if this is a language suggestion, but it sure would be cool if syntax highlighting, autocomplete, etc. could be delegated to the type provider in some way as well. To achieve functionality similar to https://marketplace.visualstudio.com/items?itemName=alfonsogarciacaro.vscode-template-fsharp-highlight, but in a more first-class kind of way. Would such a thing ever be possible?

[baronfel]

There's some precedence here with StringSyntaxAttribute, which was introduced in .NET 7. The idea here is to 'tag' a parameter (which could be a static parameter in this case?) with a language, and then tooling could use that knowledge to automatically defer checking/highlighting/etc of that string using the language specified in the attribute. Roslyn does this currently with things like dates and regexes, but I haven't yet figured out the right mechanism for creating these 'virtual documents' in FsAutoComplete, for example.

[T-Gro]

@baronfel : I think that this is still "too static", as in the tooling has to be tought about the available languages.

The way I read @NatElkins proposal is the TP itself being responsible for it.
E.g. having the ability to turn an incoming static argument into a decorated Tagged-Text with colors, autocomplete options and tooltips.

[kerams]

@NatElkins, that sounds very intriguing, but not really a language suggestion as such. Would you mind creating an issue/discussion in dotnet/fsharp? I have some ideas about this "tooling provider" (and questions whether analyzers aren't a better solution) that I want to bounce off others.