feat: annotate all emitted methods and classes with GeneratedCodeAttribute.

[skwasjer]

Add GeneratedCodeAttribute to all generated methods and classes (except root/partial class).

Description

• The attribute is added to generated methods, as well as all classes (except root/partial class).
• Refactored some related helper methods.
• Updated all snapshot tests. I did not deem it necessary to include unit tests because the coverage via the snapshots is extensive, but do let me know if you feel otherwise.
• Note: a lot of snapshot files had an UTF-8 BOM, but .editorconfig specifies UTF-8 (without BOM). Hence, for many files, line 1 is also part of the diff.

Happy to adjust if needed.

Fixes #1038

Checklist

• The existing code style is followed
• The commit message follows our guidelines
• Performed a self-review of my code
• Hard-to-understand areas of my code are commented
• The documentation is updated (as applicable)
• Unit tests are added/updated
• Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

[skwasjer]

Now that I think about it, perhaps it is better to decorate individual methods instead, since the root mapper type is declared in user code?

[latonz]

Thank you for this contribution! The code looks good to me 😊

• Unit tests are not needed IMO.
• BOM in snapshots: Doesn't VerifyTests generate these files always with a BOM? Shouldn't we then adjust the .editorconfig to expect a BOM for snapshot files?

Regarding whether to annotate methods or classes: I'm not sure which is the right way to do it either... How do other source generators (especially the built in ones of .NET (logging, json, ...)) handle this?
Originally the issue was raised to be able to ignore the generated code with coverlet. How does it behave if the attribute is on the class and how if it is on the generated methods? (e.g. does user-implemented mapping coverage still work?)

FYI: Since this is your first contribution to this repo, I have to manually approve all your pipeline runs. As soon as this PR is merged further PR pipelines run automatically after pushing to the PR.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e606f93) 91.18% compared to head (a02e628) 91.19%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1069      +/-   ##
==========================================
+ Coverage   91.18%   91.19%   +0.01%     
==========================================
  Files         220      221       +1     
  Lines        7203     7214      +11     
  Branches      915      917       +2     
==========================================
+ Hits         6568     6579      +11     
  Misses        418      418              
  Partials      217      217              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[skwasjer]

How does it behave if the attribute is on the class and how if it is on the generated methods? (e.g. does user-implemented mapping coverage still work?)

This was exactly my concern. Annotating the class, due to it being partial will affect all type members. I was a bit quick in just slapping it on the classes because I had done so in the past for other generators, however in those cases the classes were non-partial.

How do other source generators (especially the built in ones of .NET (logging, json, ...)) handle this?

Thanks for the hint. Looking at logger generated code, MS seems to indeed annotate type members individually. I am not sure why the static ctor is not annotated though. I will update the PR asap.

// source
public static partial class Log
{
    [LoggerMessage(
        EventId = 0,
        Level = LogLevel.Critical,
        Message = "Hello world!")]
    public static partial void HelloWorld(ILogger logger);
}

// generated
public static class Log
{
	[GeneratedCode("Microsoft.Extensions.Logging.Generators", "8.0.9.3103")]
	private static readonly Action<ILogger, Exception?> __HelloWorldCallback;

	[LoggerMessage(EventId = 0, Level = LogLevel.Critical, Message = "Hello world!")]
	[GeneratedCode("Microsoft.Extensions.Logging.Generators", "8.0.9.3103")]
	public static void HelloWorld(ILogger logger)
	{
		if (logger.IsEnabled(LogLevel.Critical))
		{
			__HelloWorldCallback(logger, null);
		}
	}

	static Log()
	{
		//IL_0011: Unknown result type (might be due to invalid IL or missing references)
		//IL_0016: Unknown result type (might be due to invalid IL or missing references)
		//IL_0023: Expected O, but got Unknown
		EventId eventId = new EventId(0, "HelloWorld");
		LogDefineOptions val = new LogDefineOptions();
		val.set_SkipEnabledCheck(true);
		__HelloWorldCallback = LoggerMessage.Define(LogLevel.Critical, eventId, "Hello world!", val);
	}
}

[skwasjer]

Shouldn't we then adjust the .editorconfig to expect a BOM for snapshot files?

Probably that's a good idea yea. Do you want me to include that in this PR or should this be a separate style: PR?

[latonz]

A separate style: pr sounds good to me, thank you!

[skwasjer]

@latonz as discussed, methods are now annotated. I removed the annotation for public/internal/protected classes but kept it for private/file classes. This is important for STA tools to not start complaining about unused/uncovered types (as potentially their method members are not analyzed).

Some extra considerations on this change:

• With future improvements/changes to Mapperly, there is a responsibility for the contributor and code reviewer to ensure the CodeGeneratedAttribute is applied to any generated type not explicitly declared by user code, and any of its generated type members (not just methods).
• If users of Mapperly have committed the generated code to an SCM, each subsequent version update of Mapperly that they pull will obviously now cause a diff due to the version number that has changed in the GeneratedCodeAttribute even if the generated code itself has not changed. Personally I don't see a problem with it, since it was a conscious choice in the first place to commit generated code. Not sure if it warrants a (separate) note in the docs though since I already highlighted that the attribute contains the NuGet package version.

[latonz]

Thank you for the updates, I added a few small feedback points on the code.

Regarding the points you mentioned:

• Since the class generating syntax is in a central helper method, I think the risk of forgetting this is acceptable and should also be quite low. If it ever happens in the future we can consider implementing arch tests for the generated code (e.g. on the generated code of the integration tests) to ensure it can't happen (similar as we do for the attributes in Riok.Mapperly.Abstractions already)
• IMO the code changes on the attributes with each new version of Mapperly are acceptable. This attribute was designed to include the verison number for some reason. Also I don't know a lot of repositories which check in generated code. If this should rise a lot of feedback / issues we can think of an MSBuild option to disable the attribute.

[latonz]

Thank you for your contribution! 🥳

[github-actions]

🎉 This PR is included in version 3.4.0-next.3 🎉

The release is available on:

• GitHub release
• v3.4.0-next.3

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 3.4.0 🎉

The release is available on:

• GitHub release
• v3.4.0

Your semantic-release bot 📦🚀