Suppress compilation errors until there has been a successful restore

[dsplaisted]

• Create a new C# Console app
• Observe C# editor window and Error List

Expected: No errors show up, either in the Error List or as squigglies in the editor Actual: Errors may be displayed temporarily until the NuGet restore operation completes

Suggested fix: When we open a project, disable errors from the compiler until a restore operation has completed for the project. (We should still probably show the errors if the restore operation completed unsuccessfully, because that might be because one of the dependencies wasn't found, but NuGet will still write a partial assets file with what it was able to restore.)

[davkean]

I don't think we should suppress all "errors", we should suppress the same ones that we suppress in Misc.

[davidfowl]

We used to do this in xproj, it was one of the big things that we had to do because you could get errors in the 100K range (for larger solutions) on open. We should also block build until the first successful restore is complete.

/cc @BillHiebert @abpiskunov

[davkean]

The design of background restore was supposed to block build @natidea ?

[davkean]

We should basically get a mix of Misc Files with project context (source files, raw references, project references, options, etc) - Roslyn doesn't have this concept yet, so we'd need to plumb it through.

[natidea]

Yes, background restore is registered as a critical task in CPS. That means operations like build, save, project rename and project reload block while restore is happening.

[srivatsn]

We should set the language service's LastDesignTimeBuildSucceeded property to false until the first restore is complete. Today we set that flag if a design time build fails. However in the unrestored state, design time build succeed (since it's just collecting the parameters to pass to the compiler and not actually compiling). Setting the flag turns off semantic errors for that project and we get the misc file experience.

[mavasani]

We should set the language service's LastDesignTimeBuildSucceeded property to false until the first restore is complete

@natidea @srivatsn Is there any existing service in our repo that can be queried to find out if the first restore is complete? If not, is the correct approach to set a flag here in the NugetRestorer and expose it on that type?

[davkean]

I disagree that this has anything to do with restore, that's a symptom sure but what if you have no NuGet packages.

Sent from Outlook

---

From: Srivatsn Narayanan notifications@github.com Sent: Thursday, March 23, 2017 12:02:12 PM To: dotnet/roslyn-project-system Cc: David Kean; Mention Subject: Re: [dotnet/roslyn-project-system] Suppress compilation errors until there has been a successful restore (#928)

Assigned #928https://github.com/dotnet/roslyn-project-system/issues/928 to @davkeanhttps://github.com/davkean.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/roslyn-project-system/issues/928#event-1013048183, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABDYIpDp3MPc_tBmlWWJJdO5maTN-Z_-ks5rosG0gaJpZM4LLRJ7.

[srivatsn]

I don't understand - if there are no NuGet packages why are there missing type errors? In that case we should show them.

[davkean]

Because we haven't done a design-time build, or the TFM is missing, or whatever reason...

[srivatsn]

If we haven't done a design time build I think LastDesignTimeBuildSucceeded flag is already false and hence there won't be any squiggles. If a TFM is missing then that's a valid error and there should be squiggles. Restore is a transient case, are there other transient cases?

[davkean]

No if TFM is missing, showing thousands of errors is not helpful, just the TFM is missing should show.

The lack of Restore is a symptom, the real cause is my required dependencies are not present - that's the case we should check.

[davkean]

I'm seeing thosands of errors regardless before the first design-time build also, this isn't working. If I've restored before and reopen the project - I shouldn't need restore to get references, these should be pulled from the assets file that was generated in the previous open.

[srivatsn]

How would we know if the dependencies are missing because restore didn't happen versus it's a broken reference. If we always don't show semantic errors because one reference was missing, we'll end up with missing features in the IDE (like codefixes) for that entire project and possibly for all downstream projects. That seems to be the other extreme.

[davkean]

I'm not suggesting that we don't show semantic errors if one reference is missing, but if no references are present there it's a pretty good indication that something is wrong.

[srivatsn]

That would work for the File->New experience for netcore but if a project has assembly refs\project refs and package refs and someone clones the repo and opens in VS, first run will show a bunch of squiggles. I disagree that restore not happening is a symptom - it's the cause for the missing dependencies. Dependencies could be missing for other valid reasons when we should show errors whereas a restore that hasn't happened is a transient phase and so we shouldn't show errors.

Now it's probably simpler to do the no dependencies check - we could start with that'll improve the file-new experience but in my opinion that's only part of the fix.

[davkean]

I see your point, so I'll avoid suggesting a design until I've thought about this, what I'm suggesting is that we come up with a design that works regardless of where you get your core dependencies from. Restore is just one location, TFMs reference assemblies are another.

[davkean]

We should factor in the XAML Designer and other components that need a similar experience //cc @lutzroeder @debonte

[jzabroski]

Any update on this issue?

I landed here by jumping from https://www.google.com/search?q=error+cs0518+predefined+type+%27system.object%27 to https://developercommunity.visualstudio.com/content/problem/35650/predefined-type-systemobject-is-not-defined-or-imp.html to https://developercommunity.visualstudio.com/content/problem/35650/predefined-type-systemobject-is-not-defined-or-imp.html to here.

Thank God for Google.

Background Noise

If helpful, in my scenario, I see the following:

Visual Studio Solution Explorer (25 of 21 projects??)

Closing and re-opening the sln fixes this issue.

Build output log:

2>C:\Program Files\dotnet\sdk\3.1.201\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(234,5): error NETSDK1005: Assets file 'D:\source\GitHub\fluentmigrator\src\FluentMigrator.Runner.Core\obj\project.assets.json' doesn't have a target for '.NETStandard,Version=v2.1'. Ensure that restore has run and that you have included 'netstandard2.1' in the TargetFrameworks for your project.

(I'm still not sure why this error pops up - but it was in the course of adding support for .NET Core 3.0 to FluentMigrator.DotNet.Cli.)

[NickCraver]

This stemmed from yet another Twitter conversation today. Asking for the build to fail on the first dependency failure (maybe the more general ask here) has been a thing since VS 2010.

If I may add, I think maybe the ask is more general here: instead of failing only on a restore, allow stopping the build on the first dependency failure. The restore step itself being treated as a dependency in the build and failing before the first project.

Right now this manifests in ways I see users confused about multiple times a week, both experienced and inexperienced devs internal and out in OSS. It's even more confusing because of the nature of dependencies and references. If one dev has built and is just changing say a reference in a base base library (e.g. dependency of dependency or a dependency project in the solution), they likely only see a delta of errors. If the base library didn't have any changes to the API surface they may see only the SDK errors (I'm not sure what to call the category here - restore, bad TFM, etc.). That's because they built it earlier, before that project level change (e.g. a new NuGet package version), and the downstream projects that Visual Studio continued to build are satisfied with that older DLL it attempts to use and carry on with.

There are 2 main problems with that:

1. It's not using the actual DLL/version it should be to compile with downstream
2. It presents an inconsistent experience between 2 developers

If another developer, let's say player B enters the game, and they haven't built in some time: branch change, new to project, did a git reset...whatever. The won't see just the restore/TFM/whatever error that dev A was fortunate enough to encounter, they'll get many, many errors - it can easily be tens of thousands in any decent-sized solution, from the base dependency not building. Usually errors such as class missing, etc. And all that noise prevents them from seeing the actual problem amongst the fog of war we have just presented them.

The reason I say this is wider is because that experience could be a botched restore, or it could be a missing semi-colon in that base dependency project. The experience for a user is very much the same, and a difficult thing to debug.

Right now people are installing extensions such as StopOnFirstBuildError, BuildVision or even (and I'm not kidding) VSColorOutput today. All of these have the functionality that should be in the base Visual Studio experience, I think, given that's how the command line by default behaves.

[jbogard]

Twitter conversation in question:

https://twitter.com/jbogard/status/1324003632128397317

😬

Comment in question:

https://twitter.com/Nick_Craver/status/1324010753997836292

[jzabroski]

If I may add, I think maybe the ask is more general here: instead of failing only on a restore, allow stopping the build on the first dependency failure. The restore step itself being treated as a dependency in the build and failing before the first project.

Please see the previously linked issue for this: https://github.com/dotnet/project-system/issues/1846 - The issue does not directly describe your feature request, but it is more appropriate to discuss it there, since your observations boil down to "how can we make batch compilation from CLI and design time compilation from Visual Studio behave more consistently while not sacrificing interactivity of a GUI?" And the simple answer is, there needs to be a state machine/API for that, and all the Visual Studio Designers need to hook into that state machine/API. Right now, the state machine exists, it is simply too fine grained and there is no way to organize high level events. Once it exists, VS Engineering can build regression tests to support that state machine. (It would probably be awesome to publish a diagram with that state machine, as well as some activity diagrams that explain how designers are supposed to work in the modern tooling environment, because it sounds like nobody is coordinating on that.)

Related: One simple workaround I have is whenever I can't make sense of compiler errors, I just delete all bin and obj directories. I've implemented CleanFull target in my solution (see: https://github.com/dotnet/sdk/issues/3067), caveat lector that if you misconfigure recursive wholesale delete you may end up deleting a bunch of stuff not in source control including outside your repository working directory. But CleanFull is the only thing I've found that can solve things like TieredCompilation not working when you turn it on, or upgrading a "Prefer 32-bit" app from pre-.NET4.5 ToolsVersion csproj to .NET Core SDK csproj, or switch TFM (as @davkean notes), or... so on and so forth. There are lots of footguns that can create corrupt bin and obj directories.