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

Enable IDE0005 analyzer in the build #47912

Open
eerhardt opened this issue Apr 26, 2023 · 24 comments · May be fixed by #49080
Open

Enable IDE0005 analyzer in the build #47912

eerhardt opened this issue Apr 26, 2023 · 24 comments · May be fixed by #49080
Assignees
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Comments

@eerhardt
Copy link
Member

eerhardt commented Apr 26, 2023

See the conversation in #47540 (comment). In order to unblock updating the SDK, I needed to add dotnet_diagnostic.EnableGenerateDocumentationFile.severity = none to the .globalconfig file. The new SDK was breaking the build for projects that don't have GenerateDocumentationFile=true.

We should figure out how to enable GenerateDocumentationFile=true for non product assemblies.

@sharwell suggested adding:

  <PropertyGroup>
    <!--
      Make sure any documentation comments which are included in code get checked for syntax during the build, but do
      not report warnings for missing comments.
      CS1573: Parameter 'parameter' has no matching param tag in the XML comment for 'parameter' (but other parameters do)
      CS1591: Missing XML comment for publicly visible type or member 'Type_or_Member'
      CS1712: Type parameter 'type_parameter' has no matching typeparam tag in the XML comment on 'type_or_member' (but other type parameters do)
    -->
    <GenerateDocumentationFile>True</GenerateDocumentationFile>
    <NoWarn>$(NoWarn),1573,1591,1712</NoWarn>
  </PropertyGroup>

I think we should only set those NoWarn for non-product level assemblies. So we need to do some build logic to ensure those doc warnings still occur for product projects that have public API.

More info at:

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Apr 26, 2023
@mkArtakMSFT
Copy link
Member

@eerhardt given that the SDK update was unblocked already, how urgent is this?
Also, @wtgodbe mentioned that the next SDK update may enable this analyzer by default. Is there more for us to do here then?
/cc @captainsafia

@eerhardt
Copy link
Member Author

eerhardt commented May 2, 2023

how urgent is this?

Not urgent. Just know that we aren't getting the IDE0005 diagnostic working in all projects because of this.

Is there more for us to do here then?

Yes. The work is as I describe above.

@wtgodbe wtgodbe added this to the 8.0-preview6 milestone May 10, 2023
@wtgodbe wtgodbe self-assigned this May 10, 2023
@wtgodbe wtgodbe linked a pull request Jun 28, 2023 that will close this issue
@wtgodbe wtgodbe modified the milestones: 8.0-preview6, 8.0-preview7 Jun 28, 2023
@wtgodbe wtgodbe modified the milestones: 8.0-preview7, 8.0-rc1 Aug 3, 2023
@ryanbuening
Copy link

I just updated my Visual Studio to 17.7 GA and am now seeing this issue in my .NET 7 project when building.

image

Removing dotnet_diagnostic.IDE0005.severity = error from the .editorconfig fixes it.

@madhusameena
Copy link

I just updated my Visual Studio to 17.7 GA and am now seeing this issue in my .NET 7 project when building.

image

Removing dotnet_diagnostic.IDE0005.severity = error from the .editorconfig fixes it.

Same with me, any workaround for this?
I cant disable IDE0005 completely

@scottkuhl
Copy link

I'm seeing the same thing after updating Visual Studio for Mac as well.

@sharwell
Copy link
Member

sharwell commented Aug 14, 2023

Also, @wtgodbe mentioned that the next SDK update may enable this analyzer by default.

What do you mean "this analyzer"? It would be unfortunate to enable IDE0005 at warning level by default. In team use, it has been observed to increase the frequency of merge conflicts. Eventually we just turned it off in Roslyn and there appears to be no downside to leaving it that way.

@sharwell
Copy link
Member

sharwell commented Aug 14, 2023

Same with me, any workaround for this?
I cant disable IDE0005 completely

The compiler requires one of the following two states:

  1. IDE0005 is set to suggestion (or lower) severity
  2. GenerateDocumentationFile is set to true

Note that it is trivial to meet the second requirement by using the XML <PropertyGroup> syntax shown in the first post. With those three specific documentation warnings disabled, GenerateDocumentationFile will only produce new warnings in the build if you have invalid XML comments already contained in source code.

If you attempt to "work around" this issue by trying to disable the EnableGenerateDocumentationFile, then it will not be successful because that approach is effectively the same as option (1) - it causes IDE0005 to be silently disabled.

I cant disable IDE0005 completely

If EnableGenerateDocumentationFile is showing up in your build, then IDE0005 is already disabled, even though the project asked for it to be enabled. The warning is alerting developers to the fact that the compiler cannot provide the functionality that the project has explicitly requested because the project is incorrectly configured.

@scottkuhl
Copy link

If you set GenerateDocumentationFile to true, that will bring back this issue, Warning when referencing a project with GenerateDocumentationFile true, on MAUI projects.

@sharwell
Copy link
Member

That issue was marked as fixed. If you are still using an older version of the SDK, it should be possible to update the XML file item in the build to include the expected metadata, though I'm not exactly sure where that update would need to be placed (it would have to be after the location where the SDK converts GenerateDocumentationFile to the actual build item that gets copied). The other option would be if there is some property that allows the file to be generated by the compiler, but not copied to the output.

@scottkuhl
Copy link

It may have been marked as fixed, but I’m on the latest version of 7 and it still exists.

@LiviuD
Copy link

LiviuD commented Aug 23, 2023

Still present in 17.7.2

@VictorioBerra
Copy link
Contributor

Setting EnableGenerateDocumentationFile to true for me lit up all sorts of errors for the .editorconfig I am using: https://github.com/Dotnet-Boxed/Templates/blob/main/.editorconfig

@sharwell
Copy link
Member

@VictorioBerra If those warnings were CS1573, CS1591, and/or CS1712, the solution is given in the very first post of this thread. If it was other warnings starting with CS, it means the project contained documentation comments containing some sort of syntax error which is almost certainly not intended. If it was another error, I would need more information to better understand.

@LiviuD
Copy link

LiviuD commented Aug 26, 2023

That's how Microsoft sees the issue: https://developercommunity.visualstudio.com/t/EnableGenerateDocumentationFile-Error/10448023?viewtype=solutions
This cannot be "by design" when this happens all of a sudden once you update to 17.7 version and while before all was nice and working fine.

@EGUSAXO
Copy link

EGUSAXO commented Aug 28, 2023

It is very annoying - all our projects started showing this error after updating .net sdk. I would like to have IDE0005 enabled, but I don't want to enable "GenerateDocumentationFile", as it starts generating errors in old code, plus I think generating this file will make builds slower.

I don't see how IDE0005 and "GenerateDocumentationFile" are related, it should be perfectly fine to enable just one and not other.

@patricksadowski
Copy link

It is very annoying - all our projects started showing this error after updating .net sdk. I would like to have IDE0005 enabled, but I don't want to enable "GenerateDocumentationFile", as it starts generating errors in old code, plus I think generating this file will make builds slower.

I don't see how IDE0005 and "GenerateDocumentationFile" are related, it should be perfectly fine to enable just one and not other.

To keep the previous behavior we use <NoWarn>$(NoWarn);EnableGenerateDocumentationFile</NoWarn>.

@LiviuD
Copy link

LiviuD commented Aug 29, 2023

It is very annoying - all our projects started showing this error after updating .net sdk. I would like to have IDE0005 enabled, but I don't want to enable "GenerateDocumentationFile", as it starts generating errors in old code, plus I think generating this file will make builds slower.
I don't see how IDE0005 and "GenerateDocumentationFile" are related, it should be perfectly fine to enable just one and not other.

To keep the previous behavior we use <NoWarn>$(NoWarn);EnableGenerateDocumentationFile</NoWarn>.

Hi Patrick,
Where do you put this: $(NoWarn);EnableGenerateDocumentationFile?

Thank you!

@sharwell
Copy link
Member

sharwell commented Aug 29, 2023

Hi @LiviuD,

I would strongly discourage using $(NoWarn);EnableGenerateDocumentationFile. This will not fix the problem, but will instead directly instruct the build to produce an incorrect output. If you do not want to enable documentation output, the only currently-correct solution is to reduce the severity of IDE0005 to suggestion. If customers keep trying to work around this issue by disabling EnableGenerateDocumentationFile, we will likely be forced to rename the diagnostic or add a new diagnostic to force it to appear again.

I would like to have IDE0005 enabled, but I don't want to enable "GenerateDocumentationFile"

This is not possible today, but there is an open feature request to make it possible:
dotnet/roslyn#26938

I don't see how IDE0005 and "GenerateDocumentationFile" are related, it should be perfectly fine to enable just one and not other.

Note that this is literally the reason why the EnableGenerateDocumentationFile warning exists. The build is silently producing a constraint that almost no customers were aware of.

@LiviuD
Copy link

LiviuD commented Aug 29, 2023

Thank you very much @sharwell for your suggestion.
Can you please point me in the right direction on how can I reduce the severity of IDE0005 to suggestion, for an entire solution or for the entire machine?

Thank you!

piotrzajac added a commit to Accenture/AutoFixture.XUnit2.AutoMock that referenced this issue Aug 30, 2023
@glen-84
Copy link
Contributor

glen-84 commented Aug 30, 2023

@LiviuD,

Add this to a .editorconfig file:

[*.cs]
dotnet_diagnostic.IDE0005.severity = suggestion

The suggestions won't show up during a build/dotnet-format though. You can use dotnet format --verify-no-changes --severity info, but that may show additional suggestions.

piotrzajac added a commit to Accenture/AutoFixture.XUnit2.AutoMock that referenced this issue Aug 30, 2023
* Replace AppVeyor build definition with GitHub workflows
* Enable GitHub workflow manual trigger and introduce ability to select environment and package
* Use windows agents to support old .NET Framework
* Update expected dotnet version from 6 to 7
* Update analyzers
* Ignore S6608 rule in tests
* Address IDE0005 analyzer on the build issue: dotnet/aspnetcore#47912
* Use gitversion for automatic Versioning
* Automate pushing tags when publishing packages

---------

Co-authored-by: krzysztofkaczorowski <[email protected]>
Co-authored-by: Krzysztof Kaczorowski <[email protected]>
@sharwell
Copy link
Member

sharwell commented Aug 30, 2023

@LiviuD The default severity of IDE0005 is either suggestion or hidden. The correction would need to be made in the location where IDE0005 was customized in your build to have a non-default value. git grep IDE0005 may help locate the place where it was customized.

@LiviuD
Copy link

LiviuD commented Aug 31, 2023

Hi @sharwell,
Thank you for your suggestion, this seems to do the trick. I was able to find the customisation in a Warnings.props file, where IDE0005 warning was set to appear as an error.
Maybe this will help others, so it is not only in .editorconfigs files that you need to look/change.

@wtgodbe wtgodbe modified the milestones: 8.0-rc1, 8.0.0 Oct 3, 2023
@mkArtakMSFT mkArtakMSFT modified the milestones: 8.0.0, .NET 9 Planning Dec 12, 2023
@ghost
Copy link

ghost commented Dec 12, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
crystalninja5 added a commit to crystalninja5/scaling-barnacle that referenced this issue Aug 11, 2024
* Replace AppVeyor build definition with GitHub workflows
* Enable GitHub workflow manual trigger and introduce ability to select environment and package
* Use windows agents to support old .NET Framework
* Update expected dotnet version from 6 to 7
* Update analyzers
* Ignore S6608 rule in tests
* Address IDE0005 analyzer on the build issue: dotnet/aspnetcore#47912
* Use gitversion for automatic Versioning
* Automate pushing tags when publishing packages

---------

Co-authored-by: krzysztofkaczorowski <[email protected]>
Co-authored-by: Krzysztof Kaczorowski <[email protected]>
@pastrasigns
Copy link

I'm also having issues with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging a pull request may close this issue.