search

fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
345 stars 21 forks source link

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

Open baronfel opened 8 years ago

baronfel commented 8 years ago

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 commented 1 year ago

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 commented 1 year ago

@dsyme, I'd be interested in tackling this if you could spare a moment to provide some pointers https://github.com/fsharp/fslang-suggestions/issues/338#issuecomment-311199794.

dsyme commented 1 year ago

@kerams I would suggest

I think that's it really

dsyme commented 1 year ago

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

kerams commented 1 year ago

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 commented 1 year ago

Sorry if this feels tangential:

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 commented 1 year ago

@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 commented 1 year ago

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

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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.