Skip to content

Commit

Permalink
feat: add diagnostic if no members are mapped (#1129)
Browse files Browse the repository at this point in the history
Add ExpectNoMemberMappingsAttribute to expect no member mappings
  • Loading branch information
latonz committed Feb 26, 2024
1 parent fbe7894 commit 732cec4
Show file tree
Hide file tree
Showing 60 changed files with 415 additions and 115 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/docs.yml
Expand Up @@ -24,9 +24,12 @@ on:
description: A boolean indicating whether the built pages should be deployed
environment:
required: false
type: string
type: choice
default: 'next'
description: The github deployment which is targeted, gets prefixed by 'docs-', should be 'next' or 'stable'
options:
- next
- stable
description: The github deployment which is targeted, gets prefixed by 'docs-'
version:
required: false
type: string
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/release.yml
Expand Up @@ -50,5 +50,6 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NUGET_SOURCE: ${{ inputs.nuget_source }}
NUGET_API_KEY: ${{ secrets.nuget_api_key }}
MAPPERLY_ENVIRONMENT: ${{ inputs.environment }}
- id: release-version
run: echo "version=${{ env.RELEASE_VERSION }}" >> "$GITHUB_OUTPUT"
5 changes: 5 additions & 0 deletions Directory.Build.props
Expand Up @@ -9,6 +9,11 @@
<ImplicitUsings>enable</ImplicitUsings>
<Version>0.0.1-dev</Version>
<Authors>Mapperly Contributors, Lars Tönz, Manuel Allenspach</Authors>

<!-- release environment constants -->
<DefineConstants Condition="'$(MAPPERLY_ENVIRONMENT)' == 'next'">$(DefineConstants);ENV_NEXT</DefineConstants>
<DefineConstants Condition="'$(MAPPERLY_ENVIRONMENT)' == 'stable'">$(DefineConstants);ENV_STABLE</DefineConstants>
<DefineConstants Condition="'$(MAPPERLY_ENVIRONMENT)' == ''">$(DefineConstants);ENV_LOCAL</DefineConstants>
</PropertyGroup>

</Project>
30 changes: 0 additions & 30 deletions docs/docs/configuration/analyzer-diagnostics.mdx

This file was deleted.

20 changes: 20 additions & 0 deletions docs/docs/configuration/analyzer-diagnostics/RMG066.mdx
@@ -0,0 +1,20 @@
---
sidebar_label: RMG066
description: 'Mapperly analyzer diagnostic RMG066 — No members are mapped in an object mapping'
---

# RMG066 — No members are mapped in an object mapping

If Mapperly maps to an object without mapping any member, `RMG066` is emitted.

The generated implementation part of the mapping method usually looks like this:

```csharp
public partial Target Map(Source source)
{
var target = new Target();
return target;
}
```

Usually this is not expected.
43 changes: 43 additions & 0 deletions docs/docs/configuration/analyzer-diagnostics/index.mdx
@@ -0,0 +1,43 @@
---
sidebar_position: 15
description: A list of all analyzer diagnostics used by Mapperly and how to configure them.
---

import AnalyzerRules from '@site/src/components/AnalyzerRules';

# Analyzer diagnostics

Mapperly emits several diagnostics:

<AnalyzerRules></AnalyzerRules>

:::info
Mapperly only emits updated source code on build.
You won't see the updated mapper code or mapper diagnostics until you perform a build.

This is done for performance reasons,
otherwise the IDE could become laggy.
:::

## Editorconfig

The severity of these diagnostics can be customized via an `.editorconfig` file:

```editorconfig title=".editorconfig"
[*.cs]
dotnet_diagnostic.{RuleID}.severity = error
dotnet_diagnostic.RMG020.severity = error
```

## Suppress diagnostics

See the Microsoft documentation on how to suppress diagnostics in C# [here](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings).
In summary, you can use one of the following approaches:

- Use the `.editorconfig` file (for an entire file, directory or project, depending on the editorconfig scope)
`dotnet_diagnostic.RMG066.severity = none`
- Use the `SuppressMessageAttribute` (for a method)
`[SuppressMessage("Mapper", "RMG066:No members are mapped in an object mapping")]`
- Use precompiler directives (for specific lines of code)
`#pragma warning disable RMG066 // No members are mapped in an object mapping`
`#pragma warning restore RMG066 // No members are mapped in an object mapping`
2 changes: 1 addition & 1 deletion docs/docs/configuration/enum.mdx
Expand Up @@ -113,7 +113,7 @@ public partial class CarMapper
To enforce strict enum mappings
(all source enum values have to be mapped to a target enum value
and all target enum values have to be mapped from a source enum value)
set the following two EditorConfig settings (see also [analyzer diagnostics](./analyzer-diagnostics.mdx)):
set the following two EditorConfig settings (see also [analyzer diagnostics](./analyzer-diagnostics/index.mdx)):

```editorconfig title=".editorconfig"
[*.cs]
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/configuration/mapper.mdx
Expand Up @@ -182,7 +182,7 @@ To enforce strict mappings
(all source members have to be mapped to a target member
and all target members have to be mapped from a source member,
except for ignored members)
set the following two EditorConfig settings (see also [analyzer diagnostics](./analyzer-diagnostics.mdx)):
set the following two EditorConfig settings (see also [analyzer diagnostics](./analyzer-diagnostics/index.mdx)):

```editorconfig title=".editorconfig"
[*.cs]
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/configuration/user-implemented-methods.mdx
Expand Up @@ -86,7 +86,7 @@ For each type-pair only one default mapping can exist.

If multiple mappings exist for the same type-pair without a default mapping, `RMG060` is reported.
By default `RMG060` has a severity of info,
which can be changed using the [`.editorconfig`](./analyzer-diagnostics.mdx#editorconfig).
which can be changed using the [`.editorconfig`](./analyzer-diagnostics/index.mdx#editorconfig).

## Map properties using user-implemented mappings

Expand Down
5 changes: 3 additions & 2 deletions docs/docs/contributing/common-tasks.md
Expand Up @@ -17,9 +17,10 @@ To introduce a new diagnostic follow these steps:
The highest used number can be found in `AnalyzerReleases.Shipped.md` as there may be removed diagnostics which are not present in `DiagnosticDescriptors` anymore
but which should still not be used for new diagnostics.
3. Add the new diagnostic to `AnalyzerReleases.Shipped.md` (Mapperly does not use the `Unshipped` file).
4. Add a unit test generating and asserting the added diagnostic (use `TestHelperOptions.AllowDiagnostics` and `Should().HaveDiagnostic(...)`.
4. Add a new documentation file at `docs/docs/configuration/analyzer-diagnostics/{id}.mdx` (as needed)
5. Add a unit test generating and asserting the added diagnostic (use `TestHelperOptions.AllowDiagnostics` and `Should().HaveDiagnostic(...).HaveAssertedAllDiagnostics()`.

It is not necessary to update the `Analyzer diagnostics` documentation page manually,
It is not necessary to update the `analyzer-diagnostics/index.mdx` documentation file manually,
as it is generated automatically on the basis of the `AnalyzerReleases.Shipped.md` file.

## New public API
Expand Down
6 changes: 6 additions & 0 deletions docs/prebuild.ts
@@ -1,6 +1,7 @@
const { exec } = require('child_process');
const { readFile, writeFile, copyFile, mkdir, readdir, rm } =
require('fs').promises;
const { existsSync } = require('fs');
const { join } = require('path');
const util = require('util');
const { marked } = require('marked');
Expand Down Expand Up @@ -47,6 +48,8 @@ async function buildAnalyzerRulesData(): Promise<void> {
// extract analyzer rules from AnalyzerReleases.Shipped.md and write to a json file
const targetFile = join(generatedDataDir, 'analyzer-rules.json');
const sourceFile = '../src/Riok.Mapperly/AnalyzerReleases.Shipped.md';
const analyzerDiagnosticsDocsDir =
'./docs/configuration/analyzer-diagnostics';

let rules = {};
let removingRules = true;
Expand All @@ -72,6 +75,9 @@ async function buildAnalyzerRulesData(): Promise<void> {
category: row[1].text,
severity: row[2].text,
notes: row[3].text,
hasDocumentation: existsSync(
join(analyzerDiagnosticsDocsDir, `${id}.mdx`),
),
};
}

Expand Down
4 changes: 3 additions & 1 deletion docs/src/components/AnalyzerRules.tsx
@@ -1,7 +1,9 @@
import Link from '@docusaurus/Link';
import React from 'react';

interface AnalyzerRule {
id: string;
hasDocumentation: boolean;
category: string;
severity: string;
notes: string;
Expand All @@ -22,7 +24,7 @@ export default function AnalyzerRules(): JSX.Element {
<tbody>
{rules.map((r) => (
<tr>
<td>{r.id}</td>
<td>{r.hasDocumentation ? <Link to={r.id}>{r.id}</Link> : r.id}</td>
<td>{r.severity}</td>
<td>{r.notes}</td>
</tr>
Expand Down
3 changes: 2 additions & 1 deletion src/Riok.Mapperly/AnalyzerReleases.Shipped.md
Expand Up @@ -137,7 +137,7 @@ Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RMG018 | Mapper | Disabled | Partial static mapping method in an instance mapper

## Release 3.4
## Release 3.5

### New Rules

Expand All @@ -150,3 +150,4 @@ RMG062 | Mapper | Error | The referenced mapping name is ambiguous
RMG063 | Mapper | Error | Cannot configure an enum mapping on a non-enum mapping
RMG064 | Mapper | Error | Cannot configure an object mapping on a non-object mapping
RMG065 | Mapper | Warning | Cannot configure an object mapping on a queryable projection mapping, apply the configurations to an object mapping method instead
RMG066 | Mapper | Warning | No members are mapped in an object mapping
Expand Up @@ -22,6 +22,8 @@ public abstract class MembersMappingBuilderContext<T> : IMembersBuilderContext<T
private readonly IReadOnlyCollection<string> _ignoredUnmatchedTargetMemberNames;
private readonly IReadOnlyCollection<string> _ignoredUnmatchedSourceMemberNames;

private bool _hasMemberMapping;

protected MembersMappingBuilderContext(MappingBuilderContext builderContext, T mapping)
{
BuilderContext = builderContext;
Expand Down Expand Up @@ -99,9 +101,14 @@ public void AddDiagnostics()
AddUnmatchedSourceMembersDiagnostics();
AddMappedAndIgnoredSourceMembersDiagnostics();
AddMappedAndIgnoredTargetMembersDiagnostics();
AddNoMemberMappedDiagnostic();
}

protected void SetSourceMemberMapped(MemberPath sourcePath) => _unmappedSourceMemberNames.Remove(sourcePath.Path.First().Name);
protected void SetSourceMemberMapped(MemberPath sourcePath)
{
_hasMemberMapping = true;
_unmappedSourceMemberNames.Remove(sourcePath.Path.First().Name);
}

private HashSet<string> InitIgnoredUnmatchedProperties(IEnumerable<string> allProperties, IEnumerable<string> mappedProperties)
{
Expand Down Expand Up @@ -232,4 +239,16 @@ private void AddMappedAndIgnoredSourceMembersDiagnostics()
);
}
}

private void AddNoMemberMappedDiagnostic()
{
if (!_hasMemberMapping)
{
BuilderContext.ReportDiagnostic(
DiagnosticDescriptors.NoMemberMappings,
BuilderContext.Source.ToDisplayString(),
BuilderContext.Target.ToDisplayString()
);
}
}
}
2 changes: 1 addition & 1 deletion src/Riok.Mapperly/Descriptors/MappingBuilderContext.cs
Expand Up @@ -12,7 +12,7 @@

namespace Riok.Mapperly.Descriptors;

[DebuggerDisplay("{GetType().Name}({Source.Name} => {Target.Name})")]
[DebuggerDisplay("{GetType().Name}({Source} => {Target})")]
public class MappingBuilderContext : SimpleMappingBuilderContext
{
private readonly FormatProviderCollection _formatProviders;
Expand Down
Expand Up @@ -8,6 +8,7 @@ namespace Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
/// <summary>
/// A default implementation of <see cref="IExistingTargetMapping"/>.
/// </summary>
[DebuggerDisplay("{GetType().Name}({SourceType} => {TargetType})")]
public abstract class ExistingTargetMapping : IExistingTargetMapping
{
protected ExistingTargetMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
Expand Down
@@ -1,3 +1,4 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Riok.Mapperly.Descriptors.Mappings.MemberMappings;

Expand All @@ -7,6 +8,7 @@ namespace Riok.Mapperly.Descriptors.Mappings.ExistingTarget;
/// Represents a complex object mapping implemented in its own method.
/// Maps each property from the source to the target.
/// </summary>
[DebuggerDisplay("{GetType().Name}({SourceType} => {TargetType})")]
public class ObjectMemberExistingTargetMapping(ITypeSymbol sourceType, ITypeSymbol targetType)
: MemberAssignmentMappingContainer,
IExistingTargetMapping,
Expand Down
2 changes: 2 additions & 0 deletions src/Riok.Mapperly/Descriptors/Mappings/MethodMapping.cs
@@ -1,3 +1,4 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -13,6 +14,7 @@ namespace Riok.Mapperly.Descriptors.Mappings;
/// <summary>
/// Represents a mapping which is not a single expression but an entire method.
/// </summary>
[DebuggerDisplay("{GetType().Name}({SourceType} => {TargetType})")]
public abstract class MethodMapping : ITypeMapping
{
protected const string DefaultReferenceHandlerParameterName = "refHandler";
Expand Down
23 changes: 23 additions & 0 deletions src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs
Expand Up @@ -572,4 +572,27 @@ public static class DiagnosticDescriptors
DiagnosticSeverity.Warning,
true
);

public static readonly DiagnosticDescriptor NoMemberMappings = new DiagnosticDescriptor(
"RMG066",
"No members are mapped in an object mapping",
"No members are mapped in the object mapping from {0} to {1}",
DiagnosticCategories.Mapper,
DiagnosticSeverity.Warning,
true,
helpLinkUri: BuildHelpUri("RMG066")
);

private static string BuildHelpUri(string id)
{
#if ENV_NEXT
var host = "next.mapperly.riok.app";
#elif ENV_LOCAL
var host = "localhost:3000";
#else
var host = "mapperly.riok.app";
#endif

return $"https://{host}/docs/configuration/analyzer-diagnostics/{id}";
}
}
Expand Up @@ -11,6 +11,7 @@ public static partial class DeepCloningMapper
[MapperIgnoreSource(nameof(TestObject.IgnoredIntValue))]
[MapperIgnoreSource(nameof(TestObject.IgnoredStringValue))]
[MapperIgnoreSource(nameof(TestObject.ImmutableHashSetValue))]
[MapperIgnoreSource(nameof(TestObject.SpanValue))]
[MapperIgnoreObsoleteMembers]
public static partial TestObject Copy(TestObject src);
}
Expand Down
Expand Up @@ -68,6 +68,7 @@ public static void MapExistingList(List<string> src, List<int> dst)
[MapperIgnoreTarget(nameof(TestObject.IgnoredStringValue))]
[MapperIgnoreTarget(nameof(TestObject.IgnoredIntValue))]
[MapperIgnoreSource(nameof(TestObjectDto.IgnoredIntValue))]
[MapperIgnoreSource(nameof(TestObjectDto.SpanValue))]
public static partial TestObject MapFromDto(TestObjectDto dto);

[MapperIgnoreTarget(nameof(TestObjectDto.IgnoredIntValue))]
Expand Down
1 change: 1 addition & 0 deletions test/Riok.Mapperly.IntegrationTests/Mapper/TestMapper.cs
Expand Up @@ -82,6 +82,7 @@ public TestObjectDto MapToDto(TestObject src)
[MapperIgnoreTarget(nameof(TestObject.IgnoredStringValue))]
[MapperIgnoreTarget(nameof(TestObject.IgnoredIntValue))]
[MapperIgnoreSource(nameof(TestObjectDto.IgnoredIntValue))]
[MapperIgnoreSource(nameof(TestObjectDto.SpanValue))]
public partial TestObject MapFromDto(TestObjectDto dto);

[MapperIgnoreTarget(nameof(TestObjectDto.IgnoredIntValue))]
Expand Down
Expand Up @@ -171,8 +171,8 @@ public partial class UnaffectedMapper
public partial B Map(A source);
}

record A();
record B();
record A(string Value);
record B(string Value);
enum E1 { value, value2 }
enum E2 { Value }
"""
Expand Down
Expand Up @@ -26,7 +26,7 @@ public MapperGenerationResultAssertions HaveAssertedAllDiagnostics()
return this;
}

public MapperGenerationResultAssertions NotHaveDiagnostics(IReadOnlySet<DiagnosticSeverity> allowedDiagnosticSeverities)
public MapperGenerationResultAssertions OnlyHaveDiagnosticSeverities(IReadOnlySet<DiagnosticSeverity> allowedDiagnosticSeverities)
{
_mapper.Diagnostics.FirstOrDefault(d => !allowedDiagnosticSeverities.Contains(d.Severity)).Should().BeNull();
return this;
Expand Down

0 comments on commit 732cec4

Please sign in to comment.