dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.23k stars 1.35k forks source link

CS2002 warning for .resx code generation in .NET Core / VS Code #8086

Closed tillig closed 2 years ago

tillig commented 2 years ago

Issue Description

I'm trying to work out how to use the strongly-typed resource (resx) generation across both VS Code and Visual Studio. I found that if you put this in your .csproj things mostly work:

<Project Sdk="Microsoft.NET.Sdk">
  <ItemGroup>
    <!--
      Magic embedded resource incantation based on https://github.com/dotnet/msbuild/issues/4751

      The EmbeddedResource entry seems to add the Designer files to Compile, so
      if you don't first remove them from Compile you get warnings about
      double-including source.
    -->
    <Compile Remove="**/*.Designer.cs" />
    <EmbeddedResource Update="MyResources.resx">
      <Generator>ResXFileCodeGenerator</Generator>
      <LastGenOutput>MyResources.Designer.cs</LastGenOutput>
      <StronglyTypedFileName>MyResources.Designer.cs</StronglyTypedFileName>
      <StronglyTypedLanguage>CSharp</StronglyTypedLanguage>
      <StronglyTypedNamespace>MyNamespace</StronglyTypedNamespace>
      <StronglyTypedClassName>MyResources</StronglyTypedClassName>
    </EmbeddedResource>
  </ItemGroup>
</Project>

When you build this it works great. No warnings, no errors, all the strongly-typed resources are available to your code. If the .Designer.cs file doesn't exist, it will get generated. No problem.

What I've found is that the <Compile Remove="**/*.Designer.cs" /> is a problem for OmniSharp code analysis because it follows MSBuild, but MSBuild somehow is including the .Designer.cs files magically even though they're listed as removed. Whenever you use the class (MyNamespace.MyResources.SomeResource) the OmniSharp code analysis says there's no such class.

However, if you remove that line, then when you build you get a warning:

CSC : warning CS2002: Source file '/path/to/MyNamespace/MyResources.Designer.cs' specified multiple times

I tried filing this as https://github.com/OmniSharp/omnisharp-vscode/issues/5396 but they noted it's an MSBuild issue.

Related issues here: #4751, #7609

Steps to Reproduce

I have provided a repro project here with a README that explains how to see the behavior.

Expected Behavior

I expect the analysis to see the class just like the build does - I should have consistent behavior between the build (only include the .Designer.cs file once) and the OmniSharp analysis (.Designer.cs still somehow included even though it's excluded from <Compile>).

OmniSharp is following the MSBuild here, so they insist this is an MSBuild problem, and I tend to agree.

Actual Behavior

There's a difference between the build and the OmniSharp analysis. Either the build works, or I get no-red-squiggly, but I can't have both.

OmniSharp is following the MSBuild here, so they insist this is an MSBuild problem, and I tend to agree. (I apologize for saying this multiple times; in cases like this where I mention "I can see the problem illustrated via XYZ solution" like OmniSharp, there has been a tendency in my experience to want to close the issue and redirect to that solution. It's not an OmniSharp bug. It's an MSBuild bug.)

Analysis

I have not been able to figure out where it's happening, just that it is. Sorry.

Versions & Configurations

Therzok commented 2 years ago

Not a maintainer of MSBuild, but you can theoretically set EnableDefaultCompileItems to false, and include the files you want explicitly.

I think what's happening here is that the Compile Remove target is evaluated before the actual files are flushed to disk, and that causes some weird timing issue.

ResGen is the target handling the generation of those .resources.cs files. The files theoretically should be injected by the target. A bit lower in CoreResGen it includes the generated .resources.cs into the build.

I'm not sure how this would be an issue in msbuild itself.

Therzok commented 2 years ago

If omnisharp evaluates those targets before resgen is actually run, those files won't exist on disk, so there would be no typesystem information.

tillig commented 2 years ago

Even if you generate the files, close out VS Code, then start it up again - with the files already there - if the <Compile Remove> is there, OmniSharp won't find them.

I don't think having to switch to all-manual-control is a valid workaround here. I appreciate that it's technologically possible, but it defeats the purpose of not having to do that. Seems like this should, as they say, "just work."

It seems to be an MSBuild issue because of the CS2002 warning - I shouldn't have to exclude the .Designer.cs files in order to get rid of a build warning. That warning happens whether the file already exists or whether it's just being created. There shouldn't be a warning.

If there wasn't that warning, I could drop the <Compile Remove> part and things would "just work." OmniSharp would work, MSBuild would work, all would be well.

rainersigwald commented 2 years ago

There are a few issues here. #4751 still tracks making it less awful.

First, generated files should go in the obj/ directory, so you should change this:

-      <StronglyTypedFileName>MyResources.Designer.cs</StronglyTypedFileName>
+      <StronglyTypedFileName>$(IntermediateOutputPath)\MyResources.Designer.cs</StronglyTypedFileName>

That will remove the need to have <Compile Remove="**/*.Designer.cs" />, because the default globs don't look into obj. Fixing that will resolve the CS2002 warnings.

Second, you have two conflicting definitions for "what part of the build should generate source for this resx?". <Generator>ResXFileCodeGenerator</Generator> says "Visual Studio should generate the file, into a file next to the original .resx", while specifying StronglyTyped* indicates that the GenerateResource task in CoreResGen should do that. That causes Visual Studio to create/update src/ResxRepro/MyResources.Designer.cs. You can remove the Generator, or switch to <Generator>MSBuild:Compile</Generator> (per https://github.com/dotnet/msbuild/issues/4751#issuecomment-1268408929 by @arthri) to tell Visual Studio to stop doing that (or run a build when it thinks it needs to).

This unfortunately doesn't fix OmniSharp, because it's calling the build in an unusual way that avoids the dependencies that usually cause CoreResGen to run before CoreCompile. I'll comment and see if they can reactivate the bug you filed there. https://github.com/dotnet/msbuild/issues/4751#issuecomment-1268408929 by @arthri works around that by adding a dependency that is respected: <CoreCompileDependsOn>PrepareResources;$(CompileDependsOn)</CoreCompileDependsOn>.

With those changes, I think things work in both VSCode and VS:

diff --git a/src/ResxRepro/ResxRepro.csproj b/src/ResxRepro/ResxRepro.csproj
index 4723193..05e1851 100644
--- a/src/ResxRepro/ResxRepro.csproj
+++ b/src/ResxRepro/ResxRepro.csproj
@@ -3,27 +3,14 @@
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
+
+    <CoreCompileDependsOn>PrepareResources;$(CompileDependsOn)</CoreCompileDependsOn>
   </PropertyGroup>
   <ItemGroup>
-    <!--
-      This 'Compile Remove' is the solution to removing the CS2002 warning.
-
-      If you leave it here, it seems like .Designer.cs files should not be
-      included in compilation, but it is. With it here, there is no CS2002
-      warning but tools like OmniSharp exclude the .Designer.cs files so
-      analysis is wrong.
-
-      If you comment it out, you get the CS2002 warning that the .Designer.cs
-      file is included multiple times but OmniSharp analysis starts working.
-
-      This is an inconsistency in how resource generation is addressed with
-      respect to compilation.
-    -->
-    <Compile Remove="**/*.Designer.cs" />
     <EmbeddedResource Update="MyResources.resx">
-      <Generator>ResXFileCodeGenerator</Generator>
+      <Generator>MSBuild:Compile</Generator>
       <LastGenOutput>MyResources.Designer.cs</LastGenOutput>
-      <StronglyTypedFileName>MyResources.Designer.cs</StronglyTypedFileName>
+      <StronglyTypedFileName>$(IntermediateOutputPath)\MyResources.Designer.cs</StronglyTypedFileName>
       <StronglyTypedLanguage>CSharp</StronglyTypedLanguage>
       <StronglyTypedNamespace>ResxRepro</StronglyTypedNamespace>
       <StronglyTypedClassName>MyResources</StronglyTypedClassName>
tillig commented 2 years ago

I created a new branch in my repro - issuecomment-1290568321 - that contains the above fixes. It does seem to get things to work well together, but having read through #4751 many times, I still never did put all of that together.

I guess this issue can be closed since, basically, it boils down to #4751 all over again. Thanks for the help, @rainersigwald !

tillig commented 2 years ago

Related: I have a blog article that tries to summarize all this without folks having to troll through #4751 and I've updated that article with this info.