FCS Versions with DependencyManager break FAKE 5

[baronfel]

While testing FCS updates in Ionide I discovered that FAKE 5 scripts are now broken as a result of the dependencymanager work: ionide/FsAutoComplete#566. I think they would continue to work if the error generated by not finding a particular dependencymanager was a warning instead. In addition, I noted that the error code for this particular error is overlapping a prior one: 3216 exists in FSComp.txt already for the itemNotFoundDuringDynamicCodeGen and itemNotFoundInTypeDuringDynamicCodeGen errors.

Repro steps

1. Clone FSAC from that MR link
2. run dotnet fake build -t LspTest to see the test harness open the file and attempt to have FAKE 5 parse and check it, via the FCS APIs.

Expected behavior

Perhaps a warning about a missing dependencymanager implementation that FAKE could suppress or something.

Actual behavior

Errors are given for the missing dependency manager, and the FAKE runtime doesn't get its hook to do its own resolution as expected.

Known workarounds

None from a typechecking perspective. This all still works on latest FSI (presumably until the next FSI builds land in publicly-consumable forms).

Related information

Provide any related information (optional):

• Operating system: MacOS Catalina
• .NET Runtime kind: .Net Core 3.1.1xx
• Editing Tools: FCS 34.1.0 and 34.1.1 (from late February and two days ago, respectively)

[KevinRansom]

@baronfel, I will take a look shortly.

[matthid]

I'm pretty sure I mentioned this potential break in either the corresponding issue or PR. @baronfel That is why I was surprised that the build in FAKE was still green.
In worst case (from my side) we'd just upgrade to the new plugin system for #r paket (hopefully in a non-breaking way), which needs another fcs update, correct?

Generally, I'd say the compiler emitting an error here is the right thing to do (but strictly speaking that is a breaking change - as we can see)

[baronfel]

Upgrading to the new plugin system would involve developing and publishing a paket addin and distributing that alongside FSAC, which would probably be a good thing in general to start working on ahead of releases :D

[KevinRansom]

@baronfel, I don't really see that there is anything much we can do about that. It is one of the problems with creating a breaking change in how you interpret the string following paket:.

For sure it is a script error to type:

#r "paket: Some illegal value un-recognized by the dependency manager."

I don't think we would want to reduce the severity to a warning.

Sorry mate.

[matthid]

@KevinRansom Just note that this is not about an invalid string for the dependency manager. This is about unknown/missing dependency manager. The string is valid.

Logically that would be similar to a missing dll file (which has been a warning in the past). This is because the compiler cannot decide if the string is valid or not.

[baronfel]

@KevinRansom another thing I just thought of: I started getting errors on #r references without opting in to package management by adding the --langversion:preview flag. That's the part that worries me. I think the current logic should add a check for language version support before trying to evaluate the registered dependency managers.

If the current language version doesn't support package management, but a : is detected, a warning could be issued saying that in future versions of the compiler this #r directive will throw and error, etc, etc.