diff --git a/DependencyModules.sln b/DependencyModules.sln index 61573f4..e9861a1 100644 --- a/DependencyModules.sln +++ b/DependencyModules.sln @@ -48,6 +48,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SutProject.NUnitTests", "in EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DependencyModules.xUnit4", "src\DependencyModules.xUnit4\DependencyModules.xUnit4.csproj", "{D1AEA1C9-CD76-4664-956B-706962E78E3D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DependencyModules.CodeFixes", "src\DependencyModules.CodeFixes\DependencyModules.CodeFixes.csproj", "{1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -286,6 +288,18 @@ Global {D1AEA1C9-CD76-4664-956B-706962E78E3D}.Release|x64.Build.0 = Release|Any CPU {D1AEA1C9-CD76-4664-956B-706962E78E3D}.Release|x86.ActiveCfg = Release|Any CPU {D1AEA1C9-CD76-4664-956B-706962E78E3D}.Release|x86.Build.0 = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|Any CPU.Build.0 = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|x64.ActiveCfg = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|x64.Build.0 = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|x86.ActiveCfg = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Debug|x86.Build.0 = Debug|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|Any CPU.ActiveCfg = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|Any CPU.Build.0 = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|x64.ActiveCfg = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|x64.Build.0 = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|x86.ActiveCfg = Release|Any CPU + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -311,5 +325,6 @@ Global {F911E2B7-4D5A-4DC2-9E3E-4B2CFC30CF4E} = {FB161494-E422-4D4E-BA04-3473C2EB13B0} {C3B5B8CE-D219-42C8-A1B8-5C283C053E98} = {DC7C95FE-F58F-4F16-8DFA-87E1B67DCEAE} {D1AEA1C9-CD76-4664-956B-706962E78E3D} = {FB161494-E422-4D4E-BA04-3473C2EB13B0} + {1233A4FF-C8EE-4422-A26F-EF18D3FFA0F2} = {FB161494-E422-4D4E-BA04-3473C2EB13B0} EndGlobalSection EndGlobal diff --git a/scripts/verify-packages.sh b/scripts/verify-packages.sh index ba3327a..de2245d 100755 --- a/scripts/verify-packages.sh +++ b/scripts/verify-packages.sh @@ -87,11 +87,15 @@ unzip -Z1 "${FEED}/DependencyModules.SourceGenerator.${VERSION}.nupkg" \ | grep -qx "analyzers/dotnet/cs/DependencyModules.SourceGenerator.dll" \ || fail "DependencyModules.SourceGenerator: analyzer assembly is not at analyzers/dotnet/cs/" +unzip -Z1 "${FEED}/DependencyModules.SourceGenerator.${VERSION}.nupkg" \ + | grep -qx "analyzers/dotnet/cs/DependencyModules.CodeFixes.dll" \ + || fail "DependencyModules.SourceGenerator: code fix assembly is not at analyzers/dotnet/cs/" + # No lib/. An analyzer that reaches a consumer's output ships compiler machinery to run time. unzip -Z1 "${FEED}/DependencyModules.SourceGenerator.${VERSION}.nupkg" | grep -q '^lib/' \ && fail "DependencyModules.SourceGenerator: ships a lib/ folder, so the analyzer would flow to consumer output" -pass "DependencyModules.SourceGenerator analyzer assembly is at analyzers/dotnet/cs/ with no lib/" +pass "DependencyModules.SourceGenerator analyzer and code fix assemblies are at analyzers/dotnet/cs/ with no lib/" # The shipping libraries multi-target. A missing lib/ folder means one TFM quietly stopped being # produced, and consumers on it would resolve no assembly at all. @@ -323,6 +327,22 @@ output="$(dotnet run --project "${APP}/ConsumerApp.csproj" -c Release --no-build || fail "unexpected ${TFM} consumer output: ${output}" pass "${TFM} resolved a generated registration at run time" +# DM0026 is off by default, so the build above passed with [SingletonService] and no As. Turned on +# as an error, the packaged analyzer must fail the build. The emitted files go first, because they +# sit in the project folder and the next build would compile them a second time. A global config +# rather than .editorconfig: on macOS the temp folder is reached through a symlink, so csc sees +# source paths that an .editorconfig section never matches. +rm -rf "${APP}/generated" +printf 'is_global = true\ndotnet_diagnostic.DM0026.severity = error\n' >"${APP}/.globalconfig" +dm0026_output="$(dotnet build "${APP}/ConsumerApp.csproj" -c Release --nologo --no-incremental 2>&1)" \ + && fail "${TFM} consumer built with DM0026 set to error" +grep -q 'error DM0026' <<<"${dm0026_output}" \ + || fail "${TFM} consumer failed to build with DM0026 set to error, but not with DM0026:\n${dm0026_output}" +grep -q 'CS8032' <<<"${dm0026_output}" \ + && fail "${TFM} consumer could not load an analyzer from the package:\n${dm0026_output}" +rm "${APP}/.globalconfig" +pass "${TFM} DM0026 set to error fails the build" + done echo "==> Building a generator project against DependencyModules.SourceGenerator.Impl" diff --git a/src/DependencyModules.CodeFixes/DependencyModules.CodeFixes.csproj b/src/DependencyModules.CodeFixes/DependencyModules.CodeFixes.csproj new file mode 100644 index 0000000..df18975 --- /dev/null +++ b/src/DependencyModules.CodeFixes/DependencyModules.CodeFixes.csproj @@ -0,0 +1,20 @@ + + + + + netstandard2.0 + 11 + enable + enable + false + + + + + + diff --git a/src/DependencyModules.CodeFixes/ServiceTypeCodeFixProvider.cs b/src/DependencyModules.CodeFixes/ServiceTypeCodeFixProvider.cs new file mode 100644 index 0000000..d9ba9c3 --- /dev/null +++ b/src/DependencyModules.CodeFixes/ServiceTypeCodeFixProvider.cs @@ -0,0 +1,167 @@ +using System.Collections.Immutable; +using System.Composition; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Simplification; + +namespace DependencyModules.CodeFixes; + +/// +/// Fixes DM0026 by writing As = typeof(...) with the type the generator infers today. +/// +/// +/// +/// The type comes from the diagnostic's properties, where ServiceTypeAnalyzer put what +/// ServiceTypeInference returned. This assembly does not compute it, so the fix cannot +/// drift from what the generator registers. It also does not reference the generator assembly, +/// so the IDE loads it with nothing beside it to resolve. A diagnostic without the property has no +/// fix. +/// +/// +/// Only the attribute's argument list changes. Other arguments and their trivia stay, and an +/// As that is not a typeof(...) has its expression replaced in place. +/// +/// +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(ServiceTypeCodeFixProvider)), Shared] +public sealed class ServiceTypeCodeFixProvider : CodeFixProvider +{ + // These match DependencyModuleDiagnostics.ServiceTypeNotNamed and the ServiceTypeAnalyzer + // property names in the generator assembly. + private const string DiagnosticId = "DM0026"; + private const string ServiceTypeProperty = "ServiceType"; + private const string ServiceTypeDisplayProperty = "ServiceTypeDisplay"; + + private const string EquivalenceKey = "DM0026.MakeServiceTypeExplicit"; + + public override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(DiagnosticId); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context + .Document.GetSyntaxRootAsync(context.CancellationToken) + .ConfigureAwait(false); + + if (root == null) + { + return; + } + + foreach (var diagnostic in context.Diagnostics) + { + if ( + !diagnostic.Properties.TryGetValue(ServiceTypeProperty, out var serviceType) + || serviceType == null + || root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true) + ?.FirstAncestorOrSelf() + is not { } attribute + ) + { + continue; + } + + diagnostic.Properties.TryGetValue(ServiceTypeDisplayProperty, out var display); + + context.RegisterCodeFix( + CodeAction.Create( + $"Make service type explicit: {display ?? serviceType}", + cancellationToken => + FixAsync(context.Document, attribute, serviceType, cancellationToken), + EquivalenceKey + ), + diagnostic + ); + } + } + + private static async Task FixAsync( + Document document, + AttributeSyntax attribute, + string serviceType, + CancellationToken cancellationToken + ) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + var typeOf = SyntaxFactory.TypeOfExpression( + SyntaxFactory + .ParseTypeName(serviceType) + .WithAdditionalAnnotations(Simplifier.Annotation) + ); + + return document.WithSyntaxRoot(root!.ReplaceNode(attribute, WithAs(attribute, typeOf))); + } + + private static AttributeSyntax WithAs(AttributeSyntax attribute, TypeOfExpressionSyntax typeOf) + { + var argumentList = attribute.ArgumentList; + + var existing = argumentList?.Arguments.FirstOrDefault(argument => + argument.NameEquals?.Name.ToString() == "As" + ); + + if (existing != null) + { + return attribute.ReplaceNode( + existing.Expression, + typeOf.WithTriviaFrom(existing.Expression) + ); + } + + var asArgument = SyntaxFactory.AttributeArgument( + SyntaxFactory.NameEquals( + SyntaxFactory.IdentifierName("As"), + SyntaxFactory.Token( + SyntaxFactory.TriviaList(SyntaxFactory.Space), + SyntaxKind.EqualsToken, + SyntaxFactory.TriviaList(SyntaxFactory.Space) + ) + ), + null, + typeOf + ); + + if (argumentList == null) + { + // Trivia after the name, such as a comment, moves past the new argument list. + return attribute + .WithName(attribute.Name.WithoutTrailingTrivia()) + .WithArgumentList( + SyntaxFactory + .AttributeArgumentList(SyntaxFactory.SingletonSeparatedList(asArgument)) + .WithTrailingTrivia(attribute.Name.GetTrailingTrivia()) + ); + } + + if (argumentList.Arguments.Count == 0) + { + return attribute.WithArgumentList( + argumentList.WithArguments(SyntaxFactory.SingletonSeparatedList(asArgument)) + ); + } + + // The last argument's trailing trivia, such as a comment or the line break before the + // closing parenthesis, moves after the new argument. + var last = argumentList.Arguments.Last(); + + var arguments = argumentList + .Arguments.Take(argumentList.Arguments.Count - 1) + .Append(last.WithoutTrailingTrivia()) + .Append(asArgument.WithTrailingTrivia(last.GetTrailingTrivia())); + + var separators = argumentList + .Arguments.GetSeparators() + .Append( + SyntaxFactory.Token(SyntaxKind.CommaToken).WithTrailingTrivia(SyntaxFactory.Space) + ); + + return attribute.WithArgumentList( + argumentList.WithArguments(SyntaxFactory.SeparatedList(arguments, separators)) + ); + } +} diff --git a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md index 9f37e58..ef77583 100644 --- a/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md +++ b/src/DependencyModules.SourceGenerator.Impl/AnalyzerReleases.Unshipped.md @@ -8,6 +8,7 @@ Rule ID | Category | Severity | Notes DM0023 | DependencyModules | Warning | A factory method with a service attribute cannot be called by the generated module. DM0024 | DependencyModules | Warning | A cross-wired class declares no interface but inherits one. DM0025 | DependencyModules | Warning | A [Decorator] class is not applied. +DM0026 | DependencyModules | Disabled | A service attribute does not name its service type. ### Removed Rules diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs index 4da5b93..a19bebf 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleDiagnostics.cs @@ -456,6 +456,23 @@ public static Diagnostic GeneratorFailureFrom(Exception exception) => isEnabledByDefault: true ); + /// + /// Raised by ServiceTypeAnalyzer for a [SingletonService], [ScopedService] + /// or [TransientService] without As = typeof(...). + /// + /// + /// Off by default, because inference is the documented behaviour. A project that wants every + /// registration written out turns it on with dotnet_diagnostic.DM0026.severity. + /// + public static readonly DiagnosticDescriptor ServiceTypeNotNamed = new( + id: "DM0026", + title: "A service attribute does not name its service type", + messageFormat: "[{0}] on '{1}' does not name its service type with As = typeof(...). {2}.", + category: Category, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: false + ); + /// /// Raised for [CrossWireService] on a generic type. /// diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs index 9e54da5..1bd84c3 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/ServiceModelUtility.cs @@ -454,7 +454,7 @@ ServiceRegistrationModel registrationmodel return classTypeDefinition; } - private static ITypeDefinition GetTypeDeclarationDefinition( + internal static ITypeDefinition GetTypeDeclarationDefinition( TypeDeclarationSyntax typeDeclarationSyntax ) { diff --git a/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj b/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj index b9e8ae7..9c1ce09 100644 --- a/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj +++ b/src/DependencyModules.SourceGenerator/DependencyModules.SourceGenerator.csproj @@ -12,7 +12,7 @@ The source generator of DependencyModules. When you compile, it writes the IServiceCollection registration code for each [DependencyModule] and for the classes with [SingletonService], [ScopedService], [TransientService], or [CrossWireService]. The generated code does not use reflection to find services. Use it with the DependencyModules.Runtime package. true false - + $(NoWarn);NU5128 @@ -49,6 +49,18 @@ + + + + + + +