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

Generated code should include [GeneratedCodeAttribute] and // <auto-generated/> #64541

Open
danmoseley opened this issue Jan 31, 2022 · 13 comments
Labels
area-Meta source-generator Indicates an issue with a source generator feature
Milestone

Comments

@danmoseley
Copy link
Member

Logging, Interop, Regex and JSON generators include // <auto-generated/> at the top of their emitted .g.cs.
EventSource generator does not.

Logging, Regex and JSON generators include something like [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "42.42.42.42")]
EventSource and Interop generators do not.

After #64534, all 5 of them use .g.cs extensions when writing to disk.

@jaredpar is it reasonable that all three of these should be required by convention in generators? I'm not sure what effect each one has. For example, we discovered that coverlet looks for .g.cs.

@danmoseley danmoseley added area-Meta source-generator Indicates an issue with a source generator feature labels Jan 31, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jan 31, 2022
@ghost
Copy link

ghost commented Jan 31, 2022

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

Logging, Interop, Regex and JSON generators include // <auto-generated/> at the top of their emitted .g.cs.
EventSource generator does not.

Logging, Regex and JSON generators include something like [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "42.42.42.42")]
EventSource and Interop generators do not.

After #64534, all 5 of them use .g.cs extensions when writing to disk.

@jaredpar is it reasonable that all three of these should be required by convention in generators? I'm not sure what effect each one has. For example, we discovered that coverlet looks for .g.cs.

Author: danmoseley
Assignees: -
Labels:

area-Meta, source-generator

Milestone: -

@jaredpar
Copy link
Member

I'm not sure what effect each one has. For example, we discovered that coverlet looks for .g.cs.

That seems like a bug in coverlet, not something we should be designing our generators around.

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "42.42.42.42")]

This has no impact on code that I am aware of.

@stephentoub
Copy link
Member

That seems like a bug in coverlet, not something we should be designing our generators around.

I agree. In the case of #64534 I was fine "fixing" it because we already use ".g.cs" everywhere else and this was the one outlier, with no good reason, so changing it made it more consistent with little downside. But if coverlet actually breaks if someone uses a different suffix (or no suffix) with a source generator, that certainly seems like a coverlet bug.

is it reasonable that all three of these should be required by convention in generators?

From my perspective, the <auto-generated/> is valuable in source to highlight to someone reading the source that trying to change it will be futile as it will be regenerated, the .g.cs suffix is valuable for someone looking at files on disk to know what's been created by a tool rather than by hand, and [GeneratedCode] is useful for tools that inspect metadata.

@danmoseley
Copy link
Member Author

Does VS respect both <auto-generated/> and [GeneratedCode]?

Coverlet aside, consistency is good; we should try to be consistent across our own generators, and if these two annotations have value, we should likely recommend them as well.

@dotnet/interop-contrib do you want me to open a specific issue for your generator to include GeneratedCodeAttribute, like the others?

@jaredpar
Copy link
Member

Does VS respect both and [GeneratedCode]?

Can you be more specefic in what you expect from VS here? There are many layers of VS and, unfortunately, there are differing heuristics for detecting and different responses to generated code

@danmoseley
Copy link
Member Author

I meant "are there components of VS that look for one but not the other, such that it is necessary to apply both for all components to identify generated code"?

For example, MSBuild writes XX.AssemblyInfo.cs with

//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------

but no [GeneratedCode] attribute. WPF's XAML code behind files have that plus [System.CodeDom.Compiler.GeneratedCodeAttribute("PresentationBuildTasks", "7.0.0.0")] (and end in '.g.cs' incidentally)

Yes, I wasn't intending to suggest we should cater to Coverlet's bugs. The extension would purely have the value of consistency for humans.

@jaredpar
Copy link
Member

jaredpar commented Jan 31, 2022

I meant "are there components of VS that look for one but not the other, such that it is necessary to apply both for all components to identify generated code"?

Yes. Across VS there are different heuristics for identifying generated code. The one we are trying to standardize on is the compiler heuristic around generated code.

https://sourceroslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/GeneratedCodeUtilities.cs,1c5dbec99946c19e,references

This takes into account several popular existing hueristics, including extensions. That is genenerally not a great approach. The one we'd prefer is that people have the <auto-generated> header.

@AaronRobinsonMSFT
Copy link
Member

@dotnet/interop-contrib do you want me to open a specific issue for your generator to include GeneratedCodeAttribute, like the others?

We have been discussing this in our team meeting. Adding a tracking issue would be helpful and adding it to the productization issue - #60595.

@danmoseley
Copy link
Member Author

This takes into account several popular existing hueristics, including extensions. That is genenerally not a great approach. The one we'd prefer is that people have the <auto-generated> header.

@stephentoub are you aware of tools in the ecosystem that only respect [GeneratedCode] such that we can't realistically just use <auto-generated> ? (I have zero opinion just trying to record consensus here)

@stephentoub
Copy link
Member

stephentoub commented Jan 31, 2022

@stephentoub are you aware of tools in the ecosystem that only respect [GeneratedCode]

Anything that only has access to binaries / metadata / reflection and not source, e.g.
https://grep.app/search?q=%3CGeneratedCodeAttribute%3E
https://grep.app/search?q=typeof%28GeneratedCodeAttribute%29

@stephentoub
Copy link
Member

stephentoub commented Jan 31, 2022

(In particular, I think it's important for source generated code to include [GeneratedCode(version)] so if there are any discovered bugs in the generator, binaries can be more easily audited to understand which ones are likely impacted.)

@danmoseley
Copy link
Member Author

@jaredpar if you agree, where should guidance go - in https://docs.microsoft.com/en-us/dotnet/csharp/roslyn-sdk/source-generators-overview? and perhaps samples?

@jeffhandley jeffhandley added this to the 7.0.0 milestone Feb 3, 2022
@jeffhandley jeffhandley removed the untriaged New issue has not been triaged by the area owner label Feb 3, 2022
@danmoseley danmoseley modified the milestones: 7.0.0, Future Jul 6, 2022
@Guiorgy
Copy link
Contributor

Guiorgy commented Jan 3, 2024

I only came across the GeneratedCodeAttribute attribute while reviewing some generator sources.

onyxmaster added a commit to drivenet/Dnet.SourceGenerators that referenced this issue Apr 9, 2024
onyxmaster added a commit to drivenet/Dnet.SourceGenerators that referenced this issue Apr 10, 2024
nil4 added a commit to royalapplications/beyondnet that referenced this issue Oct 5, 2024
Mark C# file contents as auto-generated code.

ref. https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/configuration-options#exclude-generated-code

> .NET code analyzer warnings aren't useful on generated code files, such as designer-generated files, which users can't edit to fix any violations.
> In most cases, code analyzers skip generated code files and don't report violations on these files.

ref. dotnet/runtime#64541
ref. dotnet/runtime#64541 (comment)

> From my perspective, the `<auto-generated/>` is valuable in source to highlight to someone reading the source that trying to change it will be futile as it will be regenerated, > the `.g.cs` suffix is valuable for someone looking at files on disk to know what's been created by a tool rather than by hand, and `[GeneratedCode]` is useful for tools that  inspect metadata.

ref. dotnet/runtime#64541 (comment)

> The one we are trying to standardize on is the compiler heuristic around generated code.
>
> https://sourceroslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/GeneratedCodeUtilities.cs,1c5dbec99946c19e,references
>
> This takes into account several popular existing hueristics, including extensions. That is genenerally not a great approach.
> The one we'd prefer is that people have the `<auto-generated>` header.

Also explicitly enable nullable annotations, since auto-generated C# code is nullable-oblivious by default.

ref. https://learn.microsoft.com/en-us/dotnet/csharp/nullable-references#nullable-contexts

> **Important**
>
> The global nullable context does not apply for generated code files. Under either strategy,> the nullable context is disabled for any source file marked as generated.
> **This means any APIs in generated files are not annotated.**

Taken together, this should avoid spurious warnings in consuming apps, and help verify that nullable annotations are accurately generated:

```
src/generated/Generated_CS.cs(77,68): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
src/generated/Generated_CS.cs(153,22): warning CS8632: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
etc.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Meta source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

No branches or pull requests

6 participants