feat: Justification Property in Ignore Attributes and Diagnostic by OlliMartin · Pull Request #2176 · riok/mapperly

OlliMartin · 2026-03-06T18:43:49Z

Justification Property in Ignore Attributes and Diagnostic

Note: This Pull Request is a draft/request for comments. I would like to make sure my changes go into the right direction before developing something that goes against the architecture.

Description

This Pull Request aims to address the feature request of #2172.

With the addition of the Justification property to MapperIgnoreX attributes (more on that below) it is possible to implement a diagnostic to enforce developers to document their decision why a mapping should be ignored.

After reviewing the code I identified the following attributes to be relevant for the diagnostic (please correct me if that's incorrect, not complete):

MapperIgnoreSourceAttribute
MapperIgnoreTargetAttribute
MapperIgnoreSourceValueAttribute
MapperIgnoreTargetValueAttribute

In this first draft I only included handling for MapperIgnoreSource and I would add the handling for the other attributes including additional tests once my approach is confirmed.

Fixes #2172

Open Points

I already adjusted the code (added a Justification) of three Verify/snapshot tests because they would write the new diagnostic otherwise. I do not see a good option to globally ignore the Hidden diagnostic, as the generator output is just passed along to Verify. I'm not fooling myself, presumably I am missing something.
Instead of adjusting all existing tests, I added the new diagnostic to the default ignore set (TestHelperOptions.Default). Please let me know if that is ok for you.
I'm not happy with the naming of WhereNotNullBy. My motivation was to align it with the naming of other LINQ methods such as GroupBy

Checklist

I did not use AI tools to generate this PR, or I have manually verified that the code is correct, optimal, and follows the project guidelines and architecture
I understand that low-quality, AI-generated PRs will be closed immediately without further explanation
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)

OlliMartin · 2026-03-06T19:00:25Z

test/Riok.Mapperly.Tests/Mapping/DerivedTypeTest.cs

            [MapProperty(nameof(A.BaseValueA), nameof(B.BaseValueB)]
            public partial B Map(A src);

-            [MapperIgnoreSource(nameof(A.BaseValueA)]


I stumbled across this while going through the failing snapshot tests:
How/why does this work in the first place, I can't wrap my head around it - it is not valid C# syntax, right?
There is a missing ) here, it should be [MapperIgnoreSource(nameof(A.BaseValueA))]

I think the way to go would be disabling the new diagnostic for all tests except where we explicitly want to test it. I think you should be able to control editorsettings configs in the tests using TestHelperOptions.AnalyzerConfigOptions

Oops, my comment does not really answer your question but a question you raised in the PR description.

It sometimes still works, even if the syntax tree is not parsed 100% correct as the parsed parts may still be enough fot mapperly to work🤷

Copilot

Pull request overview

This draft PR introduces a Justification property to MapperIgnoreSourceAttribute and adds a new hidden analyzer diagnostic (RMG096) to encourage documenting ignored mappings, along with initial tests and documentation.

Changes:

Add Justification to MapperIgnoreSourceAttribute (public API update) and update affected snapshots/tests.
Add RMG096 diagnostic descriptor and emit it for [MapperIgnoreSource] entries missing a justification.
Add initial test coverage and documentation for RMG096; adjust test helper defaults to ignore the new hidden diagnostic by default.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/Riok.Mapperly.Tests/TestHelperOptions.cs	Adds default ignored diagnostic set including RMG096.
test/Riok.Mapperly.Tests/Mapping/ReferenceHandlingTest.cs	Updates a `MapperIgnoreSource` usage to include `Justification`.
test/Riok.Mapperly.Tests/Mapping/IncludeMappingConfigurationTest.cs	Updates a `MapperIgnoreSource` usage to include `Justification`.
test/Riok.Mapperly.Tests/Mapping/DerivedTypeTest.cs	Updates a `MapperIgnoreSource` usage to include `Justification`.
test/Riok.Mapperly.Tests/DocumentationDiagnostics/MapperIgnoreJustificationTests.cs	Adds new tests asserting RMG096 behavior for `MapperIgnoreSource`.
test/Riok.Mapperly.Abstractions.Tests/_snapshots/PublicApiTest.PublicApiHasNotChanged.verified.cs	Updates public API snapshot for the new property.
src/Riok.Mapperly/Helpers/EnumerableExtensions.cs	Adds `WhereNotNullBy` helper used during configuration reading.
src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs	Introduces diagnostic descriptor RMG096.
src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs	Emits RMG096 when `MapperIgnoreSource` has no justification.
src/Riok.Mapperly/AnalyzerReleases.Shipped.md	Tracks RMG096 in analyzer release tracking file.
src/Riok.Mapperly.Abstractions/MapperIgnoreSourceAttribute.cs	Adds `Justification` property and XML docs.
docs/docs/configuration/analyzer-diagnostics/RMG096.mdx	Adds documentation page for RMG096.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T13:00:05Z

src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs

+        foreach (var missingIgnoreSourceJustificationAttribute in ignoreSourceMemberAttributes.Where(attr => attr.Justification is null))
+        {
+            _diagnostics.ReportDiagnostic(
+                DiagnosticDescriptors.MapperIgnoreAttributeMissingJustification,
+                configRef.Method,
+                missingIgnoreSourceJustificationAttribute.Source
+            );


The new RMG096 diagnostic only checks attr.Justification is null. This allows Justification = "" or whitespace to pass even though it provides no documentation. Consider treating null/empty/whitespace as missing (eg string.IsNullOrWhiteSpace).

Copilot · 2026-03-10T13:00:05Z

src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs

+            _diagnostics.ReportDiagnostic(
+                DiagnosticDescriptors.MapperIgnoreAttributeMissingJustification,
+                configRef.Method,
+                missingIgnoreSourceJustificationAttribute.Source
+            );


RMG096 is reported on configRef.Method, so the diagnostic will point at the method declaration rather than the specific [MapperIgnoreSource(...)] attribute that is missing a justification. For better UX, consider carrying the attribute syntax location through configuration reading (eg via a small config type inheriting HasSyntaxReference) and reporting the diagnostic at that attribute location.

Copilot · 2026-03-10T13:00:06Z

src/Riok.Mapperly/AnalyzerReleases.Shipped.md

 RMG093 | Mapper | Error | Target type is not assignable to the included target type
 RMG094 | Mapper | Error | Circular existing target mapping without setter detected
 RMG095 | Mapper | Warning | Invalid MSBuild configuration option
+RMG096 | Mapper | Hidden | A MapperIgnore* attribute does not specify a Justification


This adds a brand-new analyzer rule (RMG096) to AnalyzerReleases.Shipped.md. New rules are typically tracked in AnalyzerReleases.Unshipped.md until they are actually released; otherwise release tracking will claim the rule was already shipped.

Copilot · 2026-03-10T13:00:06Z

test/Riok.Mapperly.Tests/TestHelperOptions.cs

+    public static readonly IReadOnlySet<DiagnosticDescriptor> DefaultIgnoredDiagnostics = new HashSet<DiagnosticDescriptor>
+    {
+        // ignore NoMemberMappings as a lot of tests use this for simplicity
+        DiagnosticDescriptors.NoMemberMappings,
+        // ignore missing Justification properties unless specifically tested for,
+        // in which case the diagnostic is removed before using the options.
+        DiagnosticDescriptors.MapperIgnoreAttributeMissingJustification,
+    };


DefaultIgnoredDiagnostics is backed by a mutable HashSet but exposed as a static shared IReadOnlySet. Because the underlying set can still be mutated (via casts or by keeping a reference), this can create test cross-talk and order-dependent failures. Prefer an immutable collection (eg ImmutableHashSet) or create a new set per TestHelperOptions instance.

Copilot · 2026-03-10T13:00:06Z

test/Riok.Mapperly.Tests/DocumentationDiagnostics/MapperIgnoreJustificationTests.cs

+    private TestHelperOptions TestHelperOptions =>
+        TestHelperOptions.Default with
+        {
+            IgnoredDiagnostics = TestHelperOptions
+                .DefaultIgnoredDiagnostics.Except([DiagnosticDescriptors.MapperIgnoreAttributeMissingJustification])
+                .ToImmutableHashSet(),
+            AllowedDiagnosticSeverities = new HashSet<DiagnosticSeverity>() { DiagnosticSeverity.Hidden },
+        };


The property name TestHelperOptions shadows the TestHelperOptions type name, which makes the initializer hard to read and easy to misinterpret. Renaming this to something like Options/HelperOptions would improve clarity and avoid confusion in future edits.

Copilot · 2026-03-10T13:00:06Z

docs/docs/configuration/analyzer-diagnostics/RMG096.mdx

+If Mapperly detects a mapping ignore without a Justification, `RMG096` is emitted.
+
+The following attributes may raise this diagnostic:
+
+- `MapperIgnoreSource`
+- `MapperIgnoreTarget`
+- `MapperIgnoreSourceValue`
+- `MapperIgnoreTargetValue`


The documentation claims RMG096 may be raised by MapperIgnoreTarget / MapperIgnoreSourceValue / MapperIgnoreTargetValue, but the current implementation only reports it for MapperIgnoreSource (and only that attribute has Justification in this PR). Either narrow the doc to the currently supported attribute(s) or implement the diagnostic for the listed attributes before publishing this page.

Copilot · 2026-03-10T13:00:07Z

docs/docs/configuration/analyzer-diagnostics/RMG096.mdx

+
+class SourceClass {
+	string IgnoredProperty { get; set; }
+}


In the example, IgnoredProperty has no access modifier, so it's private by default and wouldn't be a mappable member in most scenarios. Marking it public (or using a public property in the sample) would make the example align with the intended use of ignore attributes.

Copilot · 2026-03-10T13:00:07Z

test/Riok.Mapperly.Tests/Mapping/DerivedTypeTest.cs

            [MapProperty(nameof(A.BaseValueA), nameof(B.BaseValueB)]
            public partial B Map(A src);

-            [MapperIgnoreSource(nameof(A.BaseValueA)]
+            [MapperIgnoreSource(nameof(A.BaseValueA), Justification = "BaseValueA is ignored for testing purposes")]
            [MapperIgnoreTarget(nameof(B.BaseValueB)]


The C# code embedded in the raw string appears to have malformed attribute syntax (missing a closing ) before ]) for [MapProperty(...] and [MapperIgnoreTarget(...]. Even if the generator can run with parse errors, this makes the test source harder to reason about and can hide real failures; please fix the attribute syntax in the test snippet.

latonz

Thank you for this contribution 🥳 I added my feedback.

latonz · 2026-03-10T13:02:22Z

docs/docs/configuration/analyzer-diagnostics/RMG096.mdx

@@ -0,0 +1,30 @@
+---
+sidebar_label: RMG096
+description: 'Mapperly analyzer diagnostic RMG096 — Ignored mapping is missing Justification'


Nitpick: lowercase justification

Same at several locations in this document.

latonz · 2026-03-10T13:03:13Z

docs/docs/configuration/analyzer-diagnostics/RMG096.mdx

+- `MapperIgnoreSource`
+- `MapperIgnoreTarget`
+- `MapperIgnoreSourceValue`
+- `MapperIgnoreTargetValue`


We should add MapperIgnoreAttribute.

latonz · 2026-03-10T13:09:15Z

src/Riok.Mapperly/Helpers/EnumerableExtensions.cs

+    public static IEnumerable<TItem> WhereNotNullBy<TItem, TSelector>(this IEnumerable<TItem> enumerable, Func<TItem, TSelector?> selector)
+        where TSelector : notnull
+    {
+#nullable disable
+        return enumerable.Where(x => selector(x) != null);
+#nullable restore
+    }
+


What is the benefit of this new extension method? The WhereNotNull doesn't accept a parameter and ensures the source enumerable is notnull-annotated after calling the method. This does not work the same with a selector 🤔

latonz · 2026-03-10T13:10:15Z

src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs

            return MapperConfiguration.Members;
        }

+        foreach (var missingIgnoreSourceJustificationAttribute in ignoreSourceMemberAttributes.Where(attr => attr.Justification is null))


When adding support for the other attributes I'd move this into its own method and call it just next to resolving the attributes.

latonz · 2026-03-10T13:10:30Z

src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs


+    public static readonly DiagnosticDescriptor MapperIgnoreAttributeMissingJustification = new(
+        "RMG096",
+        "Ignored mapping is missing Justification",


nitpick: I'd lowercase the justification.

latonz · 2026-03-10T13:11:01Z

src/Riok.Mapperly/Configuration/MapperConfigurationReader.cs

+                configRef.Method,
+                missingIgnoreSourceJustificationAttribute.Source
+            );
+        }


Add a help link for the added docs, see other diagnostics on how to do it.

latonz · 2026-03-10T13:14:17Z

test/Riok.Mapperly.Tests/Mapping/DerivedTypeTest.cs

            [MapProperty(nameof(A.BaseValueA), nameof(B.BaseValueB)]
            public partial B Map(A src);

-            [MapperIgnoreSource(nameof(A.BaseValueA)]


I think the way to go would be disabling the new diagnostic for all tests except where we explicitly want to test it. I think you should be able to control editorsettings configs in the tests using TestHelperOptions.AnalyzerConfigOptions

OlliMartin added 7 commits March 6, 2026 17:29

Create justification property on MapperIgnoreSource

c69ae6c

Create diagnostic descriptor; Wording tbd.

efb0c27

Adjust Descriptor var name, Run&accept snapshot tests

a899676

Create very basic tests for MapperIgnoreSource attribute

34ede99

Add WhereNotNullBy extension, write diagnostics for IgnoreSource

4b76d16

Fix snapshot tests -> Approach must be aligned (TBD)

8159352

Create diagnostic description markdown

8786bf1

OlliMartin commented Mar 6, 2026

View reviewed changes

OlliMartin changed the title ~~RFC | Feature/2172 Justification Property and Diagnostic~~ RFC | Feature/2172 Justification Property in Ignore Attributes and Diagnostic Mar 7, 2026

latonz requested review from Copilot and latonz March 10, 2026 12:52

Copilot started reviewing on behalf of latonz March 10, 2026 12:53 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

latonz requested changes Mar 10, 2026

View reviewed changes

latonz added the enhancement New feature or request label Mar 10, 2026

latonz changed the title ~~RFC | Feature/2172 Justification Property in Ignore Attributes and Diagnostic~~ feat: Justification Property in Ignore Attributes and Diagnostic Mar 10, 2026

Uh oh!

Conversation

OlliMartin commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Justification Property in Ignore Attributes and Diagnostic

Description

Open Points

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

latonz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

OlliMartin commented Mar 6, 2026 •

edited

Loading