A non-existing target gets called for F# project during design-time

[vzarytovskii]

Visual Studio Version

17.6 and 17.7

Summary

When loading a semingly normal ASP.NET WebAPI solution with 2 F# projects, during design-time build, there's an issue:

Error    MSB4057  

The target "ResolveCodeAnalysisRuleSet" does not exist in the project.  

C:\Program Files\Microsoft Visual Studio\2022\NonIntPreview\MSBuild\Microsoft\VisualStudio\Managed\Microsoft.Managed.DesignTime.targets    

It is normal to not have this target for F#, VS msbuild design time target defines CollectUpToDateCheckInputDesignTimeDependsOn as following:

<PropertyGroup>
	<CollectUpToDateCheckInputDesignTimeDependsOn>CompileDesignTime</CollectUpToDateCheckInputDesignTimeDependsOn>
	<!-- F# projects do not have the ResolveCodeAnalysisRuleSet target. -->
	<CollectUpToDateCheckInputDesignTimeDependsOn Condition="'$(Language)' == 'C#' or '$(Language)' == 'VB'">$(CollectUpToDateCheckInputDesignTimeDependsOn);ResolveCodeAnalysisRuleSet</CollectUpToDateCheckInputDesignTimeDependsOn>
</PropertyGroup>

However, in the binlog I see the following:

Property reassignment: $(CollectUpToDateCheckInputDesignTimeDependsOn)="CompileDesignTime;ResolveCodeAnalysisRuleSet" (previous value: "CompileDesignTime") at C:\Program Files\Microsoft Visual Studio\2022\NonIntPreview\MSBuild\Microsoft\VisualStudio\Managed\Microsoft.Managed.DesignTime.targets (443,5)
Property reassignment: $(Language)="F#" (previous value: "C#") at C:\Program Files\Microsoft Visual Studio\2022\NonIntPreview\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\Microsoft.FSharp.Targets (43,9)

It seems that language gets assigned to C# and then reassigned to F# after the CollectUpToDateCheckInputDesignTimeDependsOn is set.

To summarize: non-existing target ResolveCodeAnalysisRuleSet gets assigned to CollectUpToDateCheckInputDesignTimeDependsOn and later on becomes a source of break in design-time build, making F# intellisense unusable for such projects, because of F# gets set as a Language only after it.

Steps to Reproduce

Clone the following repository, open in VS 17.6 or 17.7: https://github.com/vzarytovskii/webapi-proj-system-issue
Issue doesn't reproduce if GitVersion.MsBuild package is removed, which leads me to believe that it resets the language or something, however it was not an issue in 17.5.

Expected Behavior

Language is set before assigning targets, or non-existed target is not getting invoked.

Actual Behavior

Language is set after assigning targets and non-existed target gets invoked.

User Impact

Project builds, but majority intellisense and language features do not work (workaround exists).

[vzarytovskii]

cc @dotnet/fsharp-team-msft and @baronfel

[tmeschter]

The issue is indeed related to GitVersion.MsBuild. The package explicitly sets the <Language> to C# in a .props file that will be imported early in the project, whereas the SDK sets it in a .targets file imported much later.

In the case of F#, the <CollectUpToDateCheckInputDesignTimeDependsOn> property is set in between the two, which is why we get the wrong behavior:

Note that C# is unaffected for both the obvious reason--in both cases <Language> is getting set to the same value--and a subtle but more important reason: the .NET SDK sets <Language> to "C#" before the <CollectUpToDateCheckInputDesignTimeDependsOn> property is set. So in a .csproj it wouldn't matter what GitVersion.MsBuild did, at least as far as this issue is concerned.

I can see three potential fixes:

1. Fix GitVersion.MsBuild to stop setting the <Language> property. I don't fully understand why it is being set, but it seems to be more of a workaround for some other bad behavior rather than a proper fix.
2. Modify the F# targets files so that Microsoft.FSharp.targets gets imported and sets <Language> to the correct value before Microsoft.FSharp.DesignTime.Targets is imported. This will prevent <CollectUpToDateCheckInputDesignTimeDependsOn> being updated to include "ResolveCodeAnalysisRuleset". Note that this is effectively the import order for C# and VB projects. The way F# imports its .targets files is more complicated, so this may not be as simple as it appears to be.
3. Move the logic that sets <CollectUpToDateCheckInputDesignTimeDependsOn> out of Microsoft.Managed.DesignTime.targets and into the C# and VB-specific design-time targets files. This will prevent F# from ever encountering that logic, regardless of how and when <Language> is set. On the downside we will then have two copies.

[baronfel]

Personally I'd love number 2 - there are a couple other flags (like project capabilities?) that I think are also set for C#/VB in this ordering, and the template engine for example would appreciate that pattern being used by F# :)

But we should probably also push number 1 independently.

[T-Gro]

Both 1 and 2 have IMO a risk of unintended consequences and therefore possible breaks.
No 3 should be safer in that regard.

[vzarytovskii]

Both 1 and 2 have IMO a risk of unintended consequences and therefore possible breaks. No 3 should be safer in that regard.

Setting the language (2) in, say, shim, should be fine in this case, given that this targets file is only imported for F# projects, given it's empty by the time when gitversion sets it.

[drewnoakes]

Another option:

4. Modify the project system to set Language as a global property so that it can never be changed.

I'm not advocating for this. Just pointing out the possibility. As an approach it begs the question: which other properties would we want to do this for? How would we decide which to do this for and which to do in props?

it was not an issue in 17.5

The change that caused this was b6b9f80. However I don't think we want to back that out over this. We need that change in order to make VSSDK projects behave correctly with the FUTDC.

[vzarytovskii]

Our current "fix" went in to 17.7, we are now setting language in our props:
https://github.com/dotnet/fsharp/pull/15228/files

[drewnoakes]

That fix seems like a reasonable approach overall. Is it worth keeping this issue open?

[vzarytovskii]

That fix seems like a reasonable approach overall. Is it worth keeping this issue open?

Probably not, thanks everyone for investigating, we have a feasible fix for 17.7 and a workaround for 17.6. Will backport if needed.

[abatishchev]

I'm getting a similar error for my msbuildproject and updating VS to 17.7.1 didn't fix it.

[vzarytovskii]

I'm getting a similar error for my msbuildproject and updating VS to 17.7.1 didn't fix it.

Gitversion should stop setting the language. That's probably the most proper fix.

[abatishchev]

I have 2 msbuildproject project, only one has this issue. And it's the one which has <Language>C#</Language> specified, because TypeSpec requires it.

What would be a workaround in my case?