dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.83k stars 773 forks source link

Make FS0078 a warning instead of an error. #4362

Open matthid opened 6 years ago

matthid commented 6 years ago

If you have #r "<something>" and <something> is invalid then the result is platform dependent and not consistent:

You either:

depending on whether <something> is a valid file name, which itself is platform dependent. Usually I'd suggest making this an error always. However, considering FST-1027 or future extensions and https://github.com/fsharp/FAKE/issues/1780. I think it is reasonable to suggest to change FS0078 into a warning message.

KevinRansom commented 6 years ago

Warning should be fine. If the #r "assembly" is actually necessary, not finding it will yield a later error. The one issue with making it a warning, is that I'm sure enterprising developers will write hunt and peck resolution using it:

r "path1\assembly.dll"

r "path2\assembly.dll"

r "path3\assembly.dll"

I would hate a developer to write the above ... but hey ... people will usually try the wrong thing and it's not as if fsi itself doesn't do exactly similar stuff internally.

Kevin

dsyme commented 6 years ago

@KevinRansom Yes, I'm concerned by that too - people already do that mess with #I "...", e.g. look at this: https://github.com/BlueMountainCapital/FSharpRProvider/blob/master/src/RProvider/RProvider.fsx

KevinRansom commented 6 years ago

@dsyme

yukk!!! but ...

when compiling, if msbuild fails to resolve a dll it doesn't fail the build. The build only fails if the compiler tries and fails to find the type that should be found in the missing dll.

The warning really just allows for "consistency" with that. Of course I held my nose when using the consistency argument, since generally I don't like it as an argument. Often it seems to me that it is used as an argument to continue doing the wrong thing.

Kevin

dsyme commented 6 years ago

@KevinRansom That file alone makes me desperate to have #r "paket: ... " and #r "nuget: ..." in the scripting model. Paket's load script generation capability would also have helped

matthid commented 6 years ago

Of course I held my nose when using the consistency argument, since generally I don't like it as an argument.

Yes I agree. I only really suggested this because it would help with FAKE 5 until we have #r "paket: ..."...

There is also another inconsistency by the way: FSI and FSC will behave differently in these situations. FSC will yield warnings where FSI will emit errors. So it is inconsistent between platforms and between tools on the same platform.

matthid commented 6 years ago

While trying to figure out how to fix this I saw:

error FS0082: Could not resolve this reference. Could not locate the assembly "not_exists.dll". Check to make sure the assembly exists on disk. If this reference is required by your code, you may get compilation errors. (Code=MSB3245)
matthid commented 6 years ago

So a lot of warnings and error numbers are associated with the "simple" error scenario. And it is not obvious when the compiler emits what.

matthid commented 5 years ago

As now even @dsyme opened an issue over at FAKE (https://github.com/fsharp/FAKE/issues/2289). I'd really like this edge case to be sorted out so we can remove the recommendation to add // to every #r "paket: " string in FAKE. Would a PR be accepted anytime soon for this?