Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intellisense Gets Broken in vscode if we have some derived properties from TFM #1738

Closed
Anipik opened this issue Mar 17, 2020 · 4 comments · Fixed by #1846
Closed

Intellisense Gets Broken in vscode if we have some derived properties from TFM #1738

Anipik opened this issue Mar 17, 2020 · 4 comments · Fixed by #1846

Comments

@Anipik
Copy link

Anipik commented Mar 17, 2020

In case of multple TargetFrameworks, omnisharp chooses the first target framework and do a reevaluation.

Before Re-evaluation, omnisharp uses setproperty function to set the TargetFramework property.

evaluatedProject.SetProperty(PropertyNames.TargetFramework, targetFramework);

So this updates a property if it exists, otherwise it searches for the first unconditioned PropertyGroup in the project file to add the property to. This is always going to be after any implicit imports as well as explicitly leading imports. This appears to be the long-standing behavior of SetProperty. Contrast that to SetGlobalProperty, which will treat the property as global.

Hence any property derived from targetFramework in implicit imports and leading imports will not be evaluated correctly.

There is a detail repro here dotnet/runtime#33427 (comment)

@ericstj
Copy link

ericstj commented Apr 28, 2020

This makes design-time build inconsistent with the actual build, which sets TargetFramework as a global property. The result is that Omnisharp's design time build is broken when actual build works fine. Omnisharp should use SetGlobalProperty instead to avoid this problem. /cc @DustinCampbell

@filipw
Copy link
Member

filipw commented Jun 23, 2020

@ericstj thanks a lot. are there any side effects of this change that you are aware of?

@ericstj
Copy link

ericstj commented Jun 23, 2020

Not that I am aware of. Global properties have two different behaviors than the current code. 1: they apply to the entire project evaluation (the fix we want) 2: the flow to any child invocations of MSBuild from the project. 2 could potentially change things but given MSBuild already behaves this way on the command line I don’t expect it to be an issue. To test you can make sure to try a case where a project references another project and that project has different but compatible TargetFrameworks. Cc @rainersigwald

@filipw
Copy link
Member

filipw commented Jun 29, 2020

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants