Mark logging source generator file as generated

[martincostello]

Give the output file for the M.E.Logging logging classes a .g suffix so that it is treated as a generated file to prevent it being flagged by source analyzers for violations the application developer cannot fix for themselves.

This is essentially the same kind of issue as I found with the Razor generator in dotnet/sdk#16777 with .NET 6 preview 3.

The use case for finding this was that I was playing around with using the Logging source generator announced in the .NET 6 preview 4 blog post and initially had the Log class I was using defined as public.

The project I was trying it out with has TreatWarningsAsErrors set to true and as it is a library requires public documentation on its members. I temporarily suppressed this with a #pragma warning disable CS1591 in the Log class, but the compilation still failed with the following error:

1>C:\Code\MyLib\src\MyLib\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessageGenerator.cs(13,36,13,51): error CS1591: Missing XML comment for publicly visible type or member 'Log.LogSomething(ILogger, string)'

In my case it's easily fixed by making the Log class be internal instead, but it highlights the issue that source analyzers may still cause user applications using the feature to fail to compile if the generated source does not conform to a rule required by the non-generated code.

[ghost]

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

Issue Details

---

Give the output file for the M.E.Logging logging classes a .g suffix so that it is treated as a generated file to prevent it being flagged by source analyzers for violations the application developer cannot fix for themselves.

This is essentially the same kind of issue as I found with the Razor generator in dotnet/sdk#16777 with .NET 6 preview 3.

The use case for finding this was that I was playing around with using the Logging source generator announced in the .NET 6 preview 4 blog post and initially had the Log class I was using defined as public.

The project I was trying it out with has TreatWarningsAsErrors set to true and as it is a library requires public documentation on its members. I temporarily suppressed this with a #pragma warning disable CS1591 in the Log class, but the compilation still failed with the following error:

1>C:\Code\MyLib\src\MyLib\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessageGenerator.cs(13,36,13,51): error CS1591: Missing XML comment for publicly visible type or member 'Log.LogSomething(ILogger, string)'

In my case it's easily fixed by making the Log class be internal instead, but it highlights the issue that source analyzers may still cause user applications using the feature to fail to compile if the generated source does not conform to a rule required by the non-generated code.

Author:	martincostello
Assignees:	-
Labels:	area-Extensions-Logging
Milestone:	-

[eerhardt]

Give the output file for the M.E.Logging logging classes a .g suffix so that it is treated as a generated file to prevent it being flagged by source analyzers for violations the application developer cannot fix for themselves.

@chsienki - Why isn’t the output of a source generator automatically treated as a generated file? It was obviously generated if it was outputted by a source generator.

[chsienki]

@eerhardt There was a bit of back and forth about whether source generated files should be considered generated to analyzers but there is an issue tracking it here dotnet/roslyn#49326

[eerhardt]

@martincostello -

1>C:\Code\MyLib\src\MyLib\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessageGenerator.cs(13,36,13,51): error CS1591: Missing XML comment for publicly visible type or member 'Log.LogSomething(ILogger, string)'

If you want your Log class to be public, and you want all public members to have XML documentation, shouldn't you be adding the XML documentation to your side of the partial method?

[martincostello]

Well in this specific case I was working through trying it out and started with public, which I then silenced with a #pragma as I was working through things, which exposed this as it didn't flow through to the generated code.

For my use case, the correct thing to do was to use internal, but as I was trying things out and was getting various compiler errors as I got my head around what was needed, I started with the exact access modifiers in the examples.

The code I was replacing used nested private classes, but nested classes aren't supported so I had to hoist the code out into a new Log class, which is where the messing about with modifiers came from.

[eerhardt]

The code I was replacing used nested private classes, but nested classes aren't supported

Feel free to upvote #52301. 😉

[chsienki]

Just looping back here, .g.cs is a supported way to express that the file is generated, and it seems in these cases that you don't want analyzers to look at the generated code, so I think this change makes sense.

[eerhardt]

.g.cs is a supported way to express that the file is generated

@chsienki - What are all the ways we need to use to express that the file is generated? Are these listed somewhere so we can be sure we got them all?

[chsienki]

@eerhardt it's technically a heuristic, so I don't think we have it published anywhere, but the code that does the detection is here https://sourceroslyn.io/#Microsoft.CodeAnalysis/InternalUtilities/GeneratedCodeUtilities.cs,1a8366e77d732c39

Currently we recognize:

• Filename starts with TemporaryGeneratedFile_
• Filename ends with:
  • .designer.cs
  • .generated.cs
  • .g.cs
  • .g.i.cs
• File starts with a comment beginning:
  • <autogenerated
  • <auto-generated

[eerhardt]

The logging generator already spits out // <auto-generated/> as the first line of the file:

runtime/src/libraries/Microsoft.Extensions.Logging.Abstractions/gen/LoggerMessageGenerator.Emitter.cs

Lines 26 to 30 in 17481fe

	public string Emit(IReadOnlyList<LoggerClass> logClasses, CancellationToken cancellationToken)
	{
	_builder.Clear();
	_builder.AppendLine("// <auto-generated/>");
	_builder.AppendLine("#nullable enable");

So I'm wondering why we need to also change the file name to end with .g.cs. Do source generators need to do both in order to tell analyzers to not look at the generated code?

[chsienki]

@eerhardt hmm, that is not expected. let me take a look and get back to you.

[chsienki]

@eerhardt Hmm, I checked and // <auto-generated/> does indeed mark the file as generated and hidden from analyzers unless they opt-in.

However, looking at the description in the PR, I tried to repro and realized the error isn't coming from an analyzer but is actually a compiler warning about the xml doc being missing. Using .g.cs doesn't fix that either, so I think this PR can be closed and I'll open an issue in Roslyn about fixing the underlying cause.

[eerhardt]

@chsienki - thanks for the great investigation! Good to see the underlying issue getting understood here.

so I think this PR can be closed

I actually think the proposed changes in this PR are correct - we should be generating the file with a .g.cs suffix. We do that for the JSON source generator, so we should do that here to be consistent.

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs

Lines 99 to 106 in b21ac01

	// Add root context implementation.
	AddSource($"{contextName}.g.cs", GetRootJsonContextImplementation(), isRootContextDef: true);
	// Add GetJsonTypeInfo override implementation.
	AddSource($"{contextName}.GetJsonTypeInfo.g.cs", GetGetTypeInfoImplementation());
	// Add property name initialization.
	AddSource($"{contextName}.PropertyNames.g.cs", GetPropertyNameInitialization());

I just wanted to fully understand what the rules were for marking a file as "generated", which you addressed above.

I believe this PR can be merged now that we fully understand the situation.

[eerhardt]

LGTM - thanks @martincostello for the contribution here, and forcing the discussion.

[iSazonov]

Nobody think about writing/updating documentation for source code generators? :-) It is very tricky to write correct SCG.