From a19a69ed8c835f67f7f8a62811899279c4a415dc Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:05:58 +0100 Subject: [PATCH 01/12] Add Kafka Analyzer --- Brighter.slnx | 1 + Directory.Packages.props | 1 + .../MissingPartitionerCodeFixProvider.cs | 101 ++++++++++++ .../PartitionerValueCodeFixProvider.cs | 100 ++++++++++++ ...aramore.Brighter.Analyzer.CodeFixes.csproj | 20 +++ .../Paramore.Brighter.Analyzer.Package.csproj | 2 + .../AnalyzerReleases.Shipped.md | 5 +- .../KafkaPublicationPartitionerAnalyzer.cs | 106 +++++++++++++ .../BrighterAnalyzerGlobals.cs | 41 +++-- .../DiagnosticsIds.cs | 19 +-- .../KafkaPublicationPartitionerVisitor.cs | 83 ++++++++++ src/Paramore.Brighter.Analyzer/docs/BRT006.md | 30 ++++ src/Paramore.Brighter.Analyzer/docs/BRT007.md | 31 ++++ src/Paramore.Brighter.Analyzer/docs/BRT008.md | 31 ++++ ...KafkaPublicationPartitionerAnalyzerTest.cs | 150 ++++++++++++++++++ .../CodeFixes/BaseCodeFixTest.cs | 28 ++++ .../MissingPartitionerCodeFixProviderTest.cs | 53 +++++++ .../PartitionerValueCodeFixProviderTest.cs | 106 +++++++++++++ .../Paramore.Brighter.Analyzer.Tests.csproj | 3 + 19 files changed, 884 insertions(+), 27 deletions(-) create mode 100644 src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs create mode 100644 src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs create mode 100644 src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj create mode 100644 src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs create mode 100644 src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs create mode 100644 src/Paramore.Brighter.Analyzer/docs/BRT006.md create mode 100644 src/Paramore.Brighter.Analyzer/docs/BRT007.md create mode 100644 src/Paramore.Brighter.Analyzer/docs/BRT008.md create mode 100644 tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs create mode 100644 tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs create mode 100644 tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs create mode 100644 tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs diff --git a/Brighter.slnx b/Brighter.slnx index 794ccea98c..b163ff5f02 100644 --- a/Brighter.slnx +++ b/Brighter.slnx @@ -242,6 +242,7 @@ + diff --git a/Directory.Packages.props b/Directory.Packages.props index 34b9e51b87..089e81ff2b 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -78,6 +78,7 @@ + diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs new file mode 100644 index 0000000000..a3ae3b9d05 --- /dev/null +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -0,0 +1,101 @@ +#region License +/* The MIT License (MIT) +Copyright © 2026 Ian Cooper + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the “Software”), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. */ + +#endregion + +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Formatting; + +namespace Paramore.Brighter.Analyzer.CodeFixes; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MissingPartitionerCodeFixProvider)), Shared] +public class MissingPartitionerCodeFixProvider : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => [DiagnosticsIds.MissingPartitioner]; + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + var objectCreation = root?.FindNode(context.Diagnostics[0].Location.SourceSpan) + .DescendantNodesAndSelf() + .OfType() + .FirstOrDefault(); + + if (objectCreation == null) + { + return; + } + + var target = BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue; + + context.RegisterCodeFix( + CodeAction.Create( + title: $"Set 'Partitioner' to 'Partitioner.{target}'", + createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, target, ct), + equivalenceKey: nameof(MissingPartitionerCodeFixProvider)), + context.Diagnostics[0]); + } + + private static async Task AddPartitionerAsync( + Document document, + BaseObjectCreationExpressionSyntax objectCreation, + string target, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + var assignment = SyntaxFactory.AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerProperty), + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerEnum), + SyntaxFactory.IdentifierName(target))); + + var initializer = objectCreation.Initializer == null + ? SyntaxFactory.InitializerExpression( + SyntaxKind.ObjectInitializerExpression, + SyntaxFactory.SingletonSeparatedList(assignment)) + : objectCreation.Initializer.AddExpressions(assignment); + + var newObjectCreation = objectCreation + .WithInitializer(initializer) + .WithAdditionalAnnotations(Formatter.Annotation); + + var newRoot = root!.ReplaceNode(objectCreation, newObjectCreation); + var formatted = Formatter.Format(newRoot, Formatter.Annotation, document.Project.Solution.Workspace, cancellationToken: cancellationToken); + + return document.WithSyntaxRoot(formatted); + } +} diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs new file mode 100644 index 0000000000..a03bbc3302 --- /dev/null +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs @@ -0,0 +1,100 @@ +#region License +/* The MIT License (MIT) +Copyright © 2026 Ian Cooper + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the “Software”), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. */ + +#endregion + +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Paramore.Brighter.Analyzer.CodeFixes; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PartitionerValueCodeFixProvider)), Shared] +public class PartitionerValueCodeFixProvider : CodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds => [DiagnosticsIds.ConsistentRandomPartitioner, DiagnosticsIds.ConsistentPartitioner]; + + 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) + { + var target = diagnostic.Id == DiagnosticsIds.ConsistentRandomPartitioner + ? BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue + : BrighterAnalyzerGlobals.Murmur2PartitionerValue; + + var assignment = root.FindNode(diagnostic.Location.SourceSpan) + .DescendantNodesAndSelf() + .OfType() + .FirstOrDefault(a => a.Left is IdentifierNameSyntax id && + id.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty); + + if (assignment == null) + { + continue; + } + + context.RegisterCodeFix( + CodeAction.Create( + title: $"Use 'Partitioner.{target}'", + createChangedDocument: ct => ReplacePartitionerValueAsync(context.Document, assignment, target, ct), + equivalenceKey: $"{nameof(PartitionerValueCodeFixProvider)}:{target}"), + diagnostic); + } + } + + private static async Task ReplacePartitionerValueAsync( + Document document, + AssignmentExpressionSyntax assignment, + string target, + CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + var newName = SyntaxFactory.IdentifierName(target); + ExpressionSyntax newValue = assignment.Right switch + { + MemberAccessExpressionSyntax memberAccess => memberAccess.WithName(newName), + _ => newName + }; + + var newRoot = root!.ReplaceNode( + assignment.Right, + newValue.WithTriviaFrom(assignment.Right)); + + return document.WithSyntaxRoot(newRoot); + } +} diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj new file mode 100644 index 0000000000..f16584964a --- /dev/null +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj @@ -0,0 +1,20 @@ + + + + $(BrighterNetStandardTargetFrameworks) + Rafael Andrade + Code fixes for the brighter library analyzers + Analyzer;CodeFix;Command Processor;Brighter + false + true + + + + + + + + + + + diff --git a/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj b/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj index 23ffadb7c7..e3b77e159f 100644 --- a/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj +++ b/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj @@ -20,6 +20,7 @@ + @@ -29,6 +30,7 @@ + diff --git a/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md b/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md index a32c29b2e2..2bb610fd18 100644 --- a/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md +++ b/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md @@ -8,4 +8,7 @@ BRT001 | Design | Warning | ([BRT001](./docs/BRT001.md)) Request Type assign BRT002 | Design | Warning | ([BRT002](./docs/BRT002.md)) RequestType is not child of IRequest BRT003 | Design | Warning | ([BRT003](./docs/BRT003.md)) MessagePump assignment is Missing BRT004 | Design | Warning | ([BRT004](./docs/BRT004.md)) Wrap attribute is applied to wrong Method -BRT005 | Design | Warning | ([BRT005](./docs/BRT005.md)) UnWrap attribute is applied to wrong Method \ No newline at end of file +BRT005 | Design | Warning | ([BRT005](./docs/BRT005.md)) UnWrap attribute is applied to wrong Method +BRT006 | Design | Info | ([BRT006](./docs/BRT006.md)) Missing Partitioner assignment +BRT007 | Design | Warning | ([BRT007](./docs/BRT007.md)) ConsistentRandom Partitioner Used +BRT008 | Design | Warning | ([BRT008](./docs/BRT008.md)) Consistent Partitioner Used diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs new file mode 100644 index 0000000000..ac2ed5a9c0 --- /dev/null +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -0,0 +1,106 @@ +#region License + +/* The MIT License (MIT) +Copyright © 2026 Ian Cooper + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the “Software”), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. */ + +#endregion + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Paramore.Brighter.Analyzer.Visitors.Operation; + +namespace Paramore.Brighter.Analyzer.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer +{ + private const string PartitionerCategory = "Design"; + + public static readonly DiagnosticDescriptor s_missingPartitionerRule = new( + id: DiagnosticsIds.MissingPartitioner, + title: "Missing Partitioner", + messageFormat: "Partitioner assignment is missing from {0}. Consider setting it explicitly.", + category: PartitionerCategory, + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true + ); + + public static readonly DiagnosticDescriptor s_consistentRandomPartitionerRule = new( + id: DiagnosticsIds.ConsistentRandomPartitioner, + title: "ConsistentRandom Partitioner Used", + messageFormat: + "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications. (Existing publications can safely ignore this warning).", + category: PartitionerCategory, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true + ); + + public static readonly DiagnosticDescriptor s_consistentPartitionerRule = new( + id: DiagnosticsIds.ConsistentPartitioner, + title: "Consistent Partitioner Used", + messageFormat: + "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications. (Existing publications can safely ignore this warning).", + category: PartitionerCategory, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true + ); + + public override ImmutableArray SupportedDiagnostics => [s_missingPartitionerRule, s_consistentRandomPartitionerRule, s_consistentPartitionerRule]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterOperationAction(AnalyzerOperation, OperationKind.ObjectCreation); + } + + private static void AnalyzerOperation(OperationAnalysisContext context) + { + var visitor = new KafkaPublicationPartitionerVisitor(); + context.Operation.Accept(visitor); + + if (!visitor.IsKafkaPublication) + { + return; + } + + if (!visitor.IsPartitionerAssigned) + { + context.ReportDiagnostic(Diagnostic.Create( + s_missingPartitionerRule, + context.Operation.Syntax.GetLocation(), + visitor.PublicationName)); + } + else if (visitor.IsConsistentRandom) + { + context.ReportDiagnostic(Diagnostic.Create( + s_consistentRandomPartitionerRule, + context.Operation.Syntax.GetLocation())); + } + else if (visitor.IsConsistent) + { + context.ReportDiagnostic(Diagnostic.Create( + s_consistentPartitionerRule, + context.Operation.Syntax.GetLocation())); + } + } +} diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index 4ad546154d..6c3c39feb6 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -22,22 +22,29 @@ THE SOFTWARE. */ #endregion -namespace Paramore.Brighter.Analyzer +namespace Paramore.Brighter.Analyzer; + +public class BrighterAnalyzerGlobals { - public class BrighterAnalyzerGlobals - { - public const string PublicationClassName = "Publication"; - public const string BrighterAssembly = "Paramore.Brighter"; - public const string RequestTypeProperty = "RequestType"; - public const string IRequestInterface = "IRequest"; - - public const string MessagePumpTypeEnumName = "MessagePumpType"; - public const string SubscriptionClassName = "Subscription"; - - public const string MessageMapperInterface = "IAmAMessageMapper"; - public const string UnwrapWithAttribute = "UnwrapWithAttribute"; - public const string WrapWithAttribute = "WrapWithAttribute"; - public const string MapToMessage = "MapToMessage"; - public const string MapToRequest = "MapToRequest"; - } + public const string PublicationClassName = "Publication"; + public const string KafkaPublicationClassName = "KafkaPublication"; + public const string BrighterAssembly = "Paramore.Brighter"; + public const string KafkaMessagingGatewayAssembly = "Paramore.Brighter.MessagingGateway.Kafka"; + public const string RequestTypeProperty = "RequestType"; + public const string PartitionerProperty = "Partitioner"; + public const string PartitionerEnum = "Partitioner"; + public const string ConsistentRandomPartitionerValue = "ConsistentRandom"; + public const string ConsistentPartitionerValue = "Consistent"; + public const string Murmur2RandomPartitionerValue = "Murmur2Random"; + public const string Murmur2PartitionerValue = "Murmur2"; + public const string IRequestInterface = "IRequest"; + + public const string MessagePumpTypeEnumName = "MessagePumpType"; + public const string SubscriptionClassName = "Subscription"; + + public const string MessageMapperInterface = "IAmAMessageMapper"; + public const string UnwrapWithAttribute = "UnwrapWithAttribute"; + public const string WrapWithAttribute = "WrapWithAttribute"; + public const string MapToMessage = "MapToMessage"; + public const string MapToRequest = "MapToRequest"; } diff --git a/src/Paramore.Brighter.Analyzer/DiagnosticsIds.cs b/src/Paramore.Brighter.Analyzer/DiagnosticsIds.cs index 96e86e3c18..36237104c2 100644 --- a/src/Paramore.Brighter.Analyzer/DiagnosticsIds.cs +++ b/src/Paramore.Brighter.Analyzer/DiagnosticsIds.cs @@ -22,15 +22,16 @@ THE SOFTWARE. */ #endregion +namespace Paramore.Brighter.Analyzer; -namespace Paramore.Brighter.Analyzer +public static class DiagnosticsIds { - public static class DiagnosticsIds - { - public const string RequestTypeMissing = "BRT001"; - public const string WrongRequestType = "BRT002"; - public const string MessagePumpMissing = "BRT003"; - public const string WrapWithAttribute = "BRT004"; - public const string UnWrapWithAttribute = "BRT005"; - } + public const string RequestTypeMissing = "BRT001"; + public const string WrongRequestType = "BRT002"; + public const string MessagePumpMissing = "BRT003"; + public const string WrapWithAttribute = "BRT004"; + public const string UnWrapWithAttribute = "BRT005"; + public const string MissingPartitioner = "BRT006"; + public const string ConsistentRandomPartitioner = "BRT007"; + public const string ConsistentPartitioner = "BRT008"; } diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs new file mode 100644 index 0000000000..8045b62b87 --- /dev/null +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -0,0 +1,83 @@ +#region License +/* The MIT License (MIT) +Copyright © 2026 Aboubakr Nasef + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the “Software”), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. */ + +#endregion + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Operations; +using Paramore.Brighter.Analyzer.Visitors.Symbol; + +namespace Paramore.Brighter.Analyzer.Visitors.Operation; + +public class KafkaPublicationPartitionerVisitor : OperationWalker +{ + public bool IsKafkaPublication { get; private set; } + public bool IsPartitionerAssigned { get; private set; } + public bool IsConsistentRandom { get; private set; } + public bool IsConsistent { get; private set; } + public string PublicationName { get; private set; } + + public override void VisitObjectCreation(IObjectCreationOperation operation) + { + if (operation.Type!.Accept(new ChildOfVisitor(BrighterAnalyzerGlobals.KafkaPublicationClassName, BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly))) + { + PublicationName = operation.Type.Name; + IsKafkaPublication = true; + } + + // base walks the children (including the initializer), which drives + // VisitSimpleAssignment for any Partitioner assignment. + base.VisitObjectCreation(operation); + } + + public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) + { + if (operation.Target is IPropertyReferenceOperation propertyReference && + propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty) + { + IsPartitionerAssigned = true; + + switch (GetPartitionerValueName(operation.Value)) + { + case BrighterAnalyzerGlobals.ConsistentRandomPartitionerValue: + IsConsistentRandom = true; + break; + case BrighterAnalyzerGlobals.ConsistentPartitionerValue: + IsConsistent = true; + break; + } + } + + base.VisitSimpleAssignment(operation); + } + + private static string GetPartitionerValueName(IOperation value) + { + // Unwrap an implicit conversion (e.g. enum widening) if present. + if (value is IConversionOperation conversion) + { + value = conversion.Operand; + } + + return value is IFieldReferenceOperation fieldReference ? fieldReference.Field.Name : null; + } +} diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md new file mode 100644 index 0000000000..c06cf6c243 --- /dev/null +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -0,0 +1,30 @@ +# BRT006: Missing Partitioner + +## Description +This rule detects a `KafkaPublication` that is created without an explicit `Partitioner` assignment. + +## Why is this an info? +When the `Partitioner` is not set explicitly, the publication falls back to the default partitioning strategy. This is reported as **Info** (not a warning) because relying on the default may be intentional; the analyzer simply surfaces the choice so it is made deliberately rather than by omission. + +The Brighter team wants users who work with Kafka to make the `Partitioner` choice explicit. The partitioner determines how message keys are distributed across a topic's partitions, and its impact — hash algorithm, keyless-message handling, and the risk of uneven load or *hot partitions* — is not common knowledge among many people who use Kafka. Setting it explicitly makes that decision visible in the code and prompts the author to understand the trade-offs (see [BRT007](./BRT007.md) and [BRT008](./BRT008.md)) rather than inheriting a default whose behaviour they may not be aware of. + +## How to fix +Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2Random` is the recommended value for new publications. + +### Example +```csharp +// Info: Partitioner assignment is missing +var publication = new KafkaPublication +{ + // ... other properties +}; + +// Fixed: Partitioner assigned explicitly +var publication = new KafkaPublication +{ + Partitioner = Partitioner.Murmur2Random + // ... other properties +}; +``` + +A code fix is available that adds `Partitioner = Partitioner.Murmur2Random` to the publication initializer. diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT007.md b/src/Paramore.Brighter.Analyzer/docs/BRT007.md new file mode 100644 index 0000000000..1dc4677ddb --- /dev/null +++ b/src/Paramore.Brighter.Analyzer/docs/BRT007.md @@ -0,0 +1,31 @@ +# BRT007: ConsistentRandom Partitioner Used + +## Description +This rule detects a `KafkaPublication` whose `Partitioner` is set to `Partitioner.ConsistentRandom`. + +## Why is this a warning? +`Murmur2Random` is preferred over `ConsistentRandom` for new `KafkaPublications`. Both hash the message key to select a partition, but `Murmur2Random` uses the MurmurHash2 algorithm, which spreads keys more evenly across partitions than the CRC32-based hash used by `ConsistentRandom`. + +An uneven hash concentrates a disproportionate share of keys onto a few partitions — the *hot partition* problem. Because each partition is served by a single consumer within a consumer group and a single broker as its leader, a hot partition becomes a throughput bottleneck: it lags and backs up while the remaining partitions sit under-used, so the topic can no longer scale across all of its partitions. `Murmur2Random`'s more uniform distribution keeps load balanced and avoids this, and it also matches the partitioning the standard Kafka clients use by default, so a given key lands on the partition other producers and consumers expect. + +Existing publications that already rely on `ConsistentRandom` can safely ignore this warning to preserve their current partition assignment. + +## How to fix +Change the `Partitioner` from `Partitioner.ConsistentRandom` to `Partitioner.Murmur2Random`. + +### Example +```csharp +// Warning: ConsistentRandom used +var publication = new KafkaPublication +{ + Partitioner = Partitioner.ConsistentRandom +}; + +// Fixed: prefer Murmur2Random +var publication = new KafkaPublication +{ + Partitioner = Partitioner.Murmur2Random +}; +``` + +A code fix is available that replaces `Partitioner.ConsistentRandom` with `Partitioner.Murmur2Random`. diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT008.md b/src/Paramore.Brighter.Analyzer/docs/BRT008.md new file mode 100644 index 0000000000..b5ff09c92f --- /dev/null +++ b/src/Paramore.Brighter.Analyzer/docs/BRT008.md @@ -0,0 +1,31 @@ +# BRT008: Consistent Partitioner Used + +## Description +This rule detects a `KafkaPublication` whose `Partitioner` is set to `Partitioner.Consistent`. + +## Why is this a warning? +`Murmur2` is preferred over `Consistent` for new `KafkaPublications`. Both hash the message key to select a partition, but `Murmur2` uses the MurmurHash2 algorithm, which spreads keys more evenly across partitions than the CRC32-based hash used by `Consistent`. + +An uneven hash concentrates a disproportionate share of keys onto a few partitions — the *hot partition* problem. Because each partition is served by a single consumer within a consumer group and a single broker as its leader, a hot partition becomes a throughput bottleneck: it lags and backs up while the remaining partitions sit under-used, so the topic can no longer scale across all of its partitions. `Murmur2`'s more uniform distribution keeps load balanced and avoids this, and it also matches the partitioning the standard Kafka clients use by default, so a given key lands on the partition other producers and consumers expect. + +Existing publications that already rely on `Consistent` can safely ignore this warning to preserve their current partition assignment. + +## How to fix +Change the `Partitioner` from `Partitioner.Consistent` to `Partitioner.Murmur2`. + +### Example +```csharp +// Warning: Consistent used +var publication = new KafkaPublication +{ + Partitioner = Partitioner.Consistent +}; + +// Fixed: prefer Murmur2 +var publication = new KafkaPublication +{ + Partitioner = Partitioner.Murmur2 +}; +``` + +A code fix is available that replaces `Partitioner.Consistent` with `Partitioner.Murmur2`. diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs new file mode 100644 index 0000000000..67610600fa --- /dev/null +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -0,0 +1,150 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using Paramore.Brighter.Analyzer.Analyzers; + +namespace Paramore.Brighter.Analyzer.Tests.Analyzers +{ + public class KafkaPublicationPartitionerAnalyzerTest : BaseAnalyzerTest + { + [Fact] + public async Task When_KafkaPublication_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication()|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Generic_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class MyRequest : IRequest + { + public System.Guid Id { get; set; } + public System.Guid SpanId { get; set; } + } + + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication()|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Is_Created_With_ConsistentRandom_Should_Report_Warning() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = Partitioner.ConsistentRandom + }|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentRandomPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Consistent_Should_Report_Warning() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = Partitioner.Consistent + }|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Murmur2Random_Should_Not_Report() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Partitioner.Murmur2Random + }; + } + } +} +"""; + + await testContext.RunAsync(); + } + } +} \ No newline at end of file diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs new file mode 100644 index 0000000000..7747a4d8fa --- /dev/null +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs @@ -0,0 +1,28 @@ + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; + +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +{ + public abstract class BaseCodeFixTest + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() + { + protected CSharpCodeFixTest testContext; + + protected BaseCodeFixTest() + { + testContext = new CSharpCodeFixTest + { + ReferenceAssemblies = ReferenceAssemblies.Net.Net90 + }; + testContext.TestState.OutputKind = OutputKind.ConsoleApplication; + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location)); + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + testContext.CompilerDiagnostics = CompilerDiagnostics.None; + } + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs new file mode 100644 index 0000000000..50e3da7fac --- /dev/null +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -0,0 +1,53 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Paramore.Brighter.Analyzer.Analyzers; +using Paramore.Brighter.Analyzer.CodeFixes; +using Xunit; + +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +{ + public class MissingPartitionerCodeFixProviderTest + : BaseCodeFixTest + { + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Add_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication()|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs new file mode 100644 index 0000000000..4d4054f0e7 --- /dev/null +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs @@ -0,0 +1,106 @@ +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Testing; +using Paramore.Brighter.Analyzer.Analyzers; +using Paramore.Brighter.Analyzer.CodeFixes; +using Xunit; + +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +{ + public class PartitionerValueCodeFixProviderTest + : BaseCodeFixTest + { + [Fact] + public async Task When_ConsistentRandom_Is_Used_Should_Offer_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = Partitioner.ConsistentRandom + }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Partitioner.Murmur2Random + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentRandomPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Consistent_Is_Used_Should_Offer_Murmur2() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = Partitioner.Consistent + }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Partitioner.Murmur2 + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Paramore.Brighter.Analyzer.Tests.csproj b/tests/Paramore.Brighter.Analyzer.Tests/Paramore.Brighter.Analyzer.Tests.csproj index 1eef7fe76a..fcb1e9c9d3 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Paramore.Brighter.Analyzer.Tests.csproj +++ b/tests/Paramore.Brighter.Analyzer.Tests/Paramore.Brighter.Analyzer.Tests.csproj @@ -14,12 +14,15 @@ + + + From e6ada14592c03585ebf80e4c6514110c18bc218c Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:09:39 +0100 Subject: [PATCH 02/12] Update Diagnositc level and message --- src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md | 2 +- .../Analyzers/KafkaPublicationPartitionerAnalyzer.cs | 6 +++--- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md b/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md index 2bb610fd18..f92f0d2f08 100644 --- a/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md +++ b/src/Paramore.Brighter.Analyzer/AnalyzerReleases.Shipped.md @@ -9,6 +9,6 @@ BRT002 | Design | Warning | ([BRT002](./docs/BRT002.md)) RequestType is not BRT003 | Design | Warning | ([BRT003](./docs/BRT003.md)) MessagePump assignment is Missing BRT004 | Design | Warning | ([BRT004](./docs/BRT004.md)) Wrap attribute is applied to wrong Method BRT005 | Design | Warning | ([BRT005](./docs/BRT005.md)) UnWrap attribute is applied to wrong Method -BRT006 | Design | Info | ([BRT006](./docs/BRT006.md)) Missing Partitioner assignment +BRT006 | Design | Warning | ([BRT006](./docs/BRT006.md)) Missing Partitioner assignment BRT007 | Design | Warning | ([BRT007](./docs/BRT007.md)) ConsistentRandom Partitioner Used BRT008 | Design | Warning | ([BRT008](./docs/BRT008.md)) Consistent Partitioner Used diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index ac2ed5a9c0..b9496a1d22 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -40,7 +40,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer title: "Missing Partitioner", messageFormat: "Partitioner assignment is missing from {0}. Consider setting it explicitly.", category: PartitionerCategory, - defaultSeverity: DiagnosticSeverity.Info, + defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true ); @@ -48,7 +48,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer id: DiagnosticsIds.ConsistentRandomPartitioner, title: "ConsistentRandom Partitioner Used", messageFormat: - "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications. (Existing publications can safely ignore this warning).", + "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true @@ -58,7 +58,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer id: DiagnosticsIds.ConsistentPartitioner, title: "Consistent Partitioner Used", messageFormat: - "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications. (Existing publications can safely ignore this warning).", + "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'Consistent' to preserve their current partition assignment.", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index c06cf6c243..7328e59827 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -3,8 +3,8 @@ ## Description This rule detects a `KafkaPublication` that is created without an explicit `Partitioner` assignment. -## Why is this an info? -When the `Partitioner` is not set explicitly, the publication falls back to the default partitioning strategy. This is reported as **Info** (not a warning) because relying on the default may be intentional; the analyzer simply surfaces the choice so it is made deliberately rather than by omission. +## Why is this a warning? +When the `Partitioner` is not set explicitly, the publication falls back to the default partitioning strategy — which is `Partitioner.ConsistentRandom`. Omitting the assignment therefore silently selects the same value that [BRT007](./BRT007.md) discourages, without that choice being visible in the code. Because the implicit case has the same partition-distribution impact as the explicit one, it is reported at the same **Warning** severity rather than as a lower-priority suggestion. The Brighter team wants users who work with Kafka to make the `Partitioner` choice explicit. The partitioner determines how message keys are distributed across a topic's partitions, and its impact — hash algorithm, keyless-message handling, and the risk of uneven load or *hot partitions* — is not common knowledge among many people who use Kafka. Setting it explicitly makes that decision visible in the code and prompts the author to understand the trade-offs (see [BRT007](./BRT007.md) and [BRT008](./BRT008.md)) rather than inheriting a default whose behaviour they may not be aware of. From 7d05e92b2b43d2cd80df97fe5f321a314d03158a Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:15:35 +0100 Subject: [PATCH 03/12] apply code review --- .../MissingPartitionerCodeFixProvider.cs | 12 ++- .../KafkaPublicationPartitionerAnalyzer.cs | 14 ++-- .../BrighterAnalyzerGlobals.cs | 3 +- .../KafkaPublicationPartitionerVisitor.cs | 11 ++- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 2 +- ...KafkaPublicationPartitionerAnalyzerTest.cs | 73 +++++++++++++++++-- .../MissingPartitionerCodeFixProviderTest.cs | 2 +- .../PartitionerValueCodeFixProviderTest.cs | 4 +- 8 files changed, 96 insertions(+), 25 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index a3ae3b9d05..cb116dbfeb 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -33,6 +33,7 @@ THE SOFTWARE. */ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Simplification; namespace Paramore.Brighter.Analyzer.CodeFixes; @@ -79,9 +80,10 @@ private static async Task AddPartitionerAsync( SyntaxKind.SimpleAssignmentExpression, SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerProperty), SyntaxFactory.MemberAccessExpression( - SyntaxKind.SimpleMemberAccessExpression, - SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerEnum), - SyntaxFactory.IdentifierName(target))); + SyntaxKind.SimpleMemberAccessExpression, + SyntaxFactory.ParseName($"{BrighterAnalyzerGlobals.KafkaMessagingGatewayNamespace}.{BrighterAnalyzerGlobals.PartitionerEnum}"), + SyntaxFactory.IdentifierName(target)) + .WithAdditionalAnnotations(Simplifier.Annotation)); var initializer = objectCreation.Initializer == null ? SyntaxFactory.InitializerExpression( @@ -96,6 +98,8 @@ private static async Task AddPartitionerAsync( var newRoot = root!.ReplaceNode(objectCreation, newObjectCreation); var formatted = Formatter.Format(newRoot, Formatter.Annotation, document.Project.Solution.Workspace, cancellationToken: cancellationToken); - return document.WithSyntaxRoot(formatted); + // Reduce the fully qualified Partitioner reference where the using is + // already present; otherwise keep it qualified so the fix always compiles. + return await Simplifier.ReduceAsync(document.WithSyntaxRoot(formatted), Simplifier.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); } } diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index b9496a1d22..3c1e929f2d 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -35,7 +35,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer { private const string PartitionerCategory = "Design"; - public static readonly DiagnosticDescriptor s_missingPartitionerRule = new( + public static readonly DiagnosticDescriptor MissingPartitionerRule = new( id: DiagnosticsIds.MissingPartitioner, title: "Missing Partitioner", messageFormat: "Partitioner assignment is missing from {0}. Consider setting it explicitly.", @@ -44,7 +44,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true ); - public static readonly DiagnosticDescriptor s_consistentRandomPartitionerRule = new( + public static readonly DiagnosticDescriptor ConsistentRandomPartitionerRule = new( id: DiagnosticsIds.ConsistentRandomPartitioner, title: "ConsistentRandom Partitioner Used", messageFormat: @@ -54,7 +54,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true ); - public static readonly DiagnosticDescriptor s_consistentPartitionerRule = new( + public static readonly DiagnosticDescriptor ConsistentPartitionerRule = new( id: DiagnosticsIds.ConsistentPartitioner, title: "Consistent Partitioner Used", messageFormat: @@ -64,7 +64,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true ); - public override ImmutableArray SupportedDiagnostics => [s_missingPartitionerRule, s_consistentRandomPartitionerRule, s_consistentPartitionerRule]; + public override ImmutableArray SupportedDiagnostics => [MissingPartitionerRule, ConsistentRandomPartitionerRule, ConsistentPartitionerRule]; public override void Initialize(AnalysisContext context) { @@ -86,20 +86,20 @@ private static void AnalyzerOperation(OperationAnalysisContext context) if (!visitor.IsPartitionerAssigned) { context.ReportDiagnostic(Diagnostic.Create( - s_missingPartitionerRule, + MissingPartitionerRule, context.Operation.Syntax.GetLocation(), visitor.PublicationName)); } else if (visitor.IsConsistentRandom) { context.ReportDiagnostic(Diagnostic.Create( - s_consistentRandomPartitionerRule, + ConsistentRandomPartitionerRule, context.Operation.Syntax.GetLocation())); } else if (visitor.IsConsistent) { context.ReportDiagnostic(Diagnostic.Create( - s_consistentPartitionerRule, + ConsistentPartitionerRule, context.Operation.Syntax.GetLocation())); } } diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index 6c3c39feb6..c4a3d68817 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -1,4 +1,4 @@ -#region License +#region License /* The MIT License (MIT) Copyright © 2026 Aboubakr Nasef @@ -30,6 +30,7 @@ public class BrighterAnalyzerGlobals public const string KafkaPublicationClassName = "KafkaPublication"; public const string BrighterAssembly = "Paramore.Brighter"; public const string KafkaMessagingGatewayAssembly = "Paramore.Brighter.MessagingGateway.Kafka"; + public const string KafkaMessagingGatewayNamespace = "Paramore.Brighter.MessagingGateway.Kafka"; public const string RequestTypeProperty = "RequestType"; public const string PartitionerProperty = "Partitioner"; public const string PartitionerEnum = "Partitioner"; diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index 8045b62b87..77dd445e88 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -42,11 +42,14 @@ public override void VisitObjectCreation(IObjectCreationOperation operation) { PublicationName = operation.Type.Name; IsKafkaPublication = true; - } - // base walks the children (including the initializer), which drives - // VisitSimpleAssignment for any Partitioner assignment. - base.VisitObjectCreation(operation); + // base walks the children (including the initializer), which drives + // VisitSimpleAssignment for any Partitioner assignment. Only descend + // when this operation is the KafkaPublication itself; descending into + // unrelated object creations would pick up nested publications and + // report the diagnostic at the wrong location. + base.VisitObjectCreation(operation); + } } public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index 7328e59827..7a1b0cbc17 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -13,7 +13,7 @@ Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2 ### Example ```csharp -// Info: Partitioner assignment is missing +// Warning: Partitioner assignment is missing var publication = new KafkaPublication { // ... other properties diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index 67610600fa..eb24515bba 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -28,7 +28,7 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); await testContext.RunAsync(); } @@ -59,7 +59,7 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); await testContext.RunAsync(); } @@ -87,7 +87,7 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); await testContext.RunAsync(); } @@ -115,7 +115,7 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); await testContext.RunAsync(); } @@ -146,5 +146,68 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_KafkaPublication_Without_Partitioner_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class Holder + { + public Holder(KafkaPublication publication) { } } -} \ No newline at end of file + + class TypeName + { + public void Method() + { + var holder = new Holder({|#0:new KafkaPublication()|}); + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_With_Consistent_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class Holder + { + public Holder(KafkaPublication publication) { } + } + + class TypeName + { + public void Method() + { + var holder = new Holder({|#0:new KafkaPublication + { + Partitioner = Partitioner.Consistent + }|}); + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index 50e3da7fac..b00feb401e 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -45,7 +45,7 @@ public void Method() """; testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_missingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); await testContext.RunAsync(); } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs index 4d4054f0e7..ce41ff49df 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs @@ -51,7 +51,7 @@ public void Method() """; testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentRandomPartitionerRule).WithLocation(0)); + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); await testContext.RunAsync(); } @@ -98,7 +98,7 @@ public void Method() """; testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.s_consistentPartitionerRule).WithLocation(0)); + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); await testContext.RunAsync(); } From 318ad40f926861acea1269f475d1f7a662fc7b1b Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:55:36 +0100 Subject: [PATCH 04/12] Apply Kafka analyzer review feedback - Only treat Partitioner assignments on KafkaPublication itself as the publication's partitioner (a nested object's own Partitioner property, e.g. Confluent's ProducerConfig, no longer suppresses BRT006) - Don't report BRT006 when the partitioner is assigned to the same local later in the same block - Don't offer the BRT007/8 code fix when the assigned expression is neither a member access nor a bare identifier (would not compile) - Verify fixed code compiles in code-fix tests (CompilerDiagnostics.Errors) - Add tests: Partitioner.Random negative, nested config with its own Partitioner, set-after-construction, code fix without the Kafka using - Document in BRT006.md that the code fix changes partition assignment and note the helper-method limitation - Add helpLinkUri for BRT006-BRT008, render generic publications as KafkaPublication, drop no-op packaging metadata from CodeFixes.csproj --- .../MissingPartitionerCodeFixProvider.cs | 6 +- .../PartitionerValueCodeFixProvider.cs | 6 +- ...aramore.Brighter.Analyzer.CodeFixes.csproj | 4 - .../KafkaPublicationPartitionerAnalyzer.cs | 58 +++++++++++- .../BrighterAnalyzerGlobals.cs | 1 - .../KafkaPublicationPartitionerVisitor.cs | 11 ++- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 4 + ...KafkaPublicationPartitionerAnalyzerTest.cs | 90 ++++++++++++++++++- .../CodeFixes/BaseCodeFixTest.cs | 4 +- .../MissingPartitionerCodeFixProviderTest.cs | 35 ++++++++ 10 files changed, 200 insertions(+), 19 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index cb116dbfeb..638e21dfdf 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -81,7 +81,7 @@ private static async Task AddPartitionerAsync( SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerProperty), SyntaxFactory.MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, - SyntaxFactory.ParseName($"{BrighterAnalyzerGlobals.KafkaMessagingGatewayNamespace}.{BrighterAnalyzerGlobals.PartitionerEnum}"), + SyntaxFactory.ParseExpression($"{BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly}.{BrighterAnalyzerGlobals.PartitionerEnum}"), SyntaxFactory.IdentifierName(target)) .WithAdditionalAnnotations(Simplifier.Annotation)); @@ -96,10 +96,10 @@ private static async Task AddPartitionerAsync( .WithAdditionalAnnotations(Formatter.Annotation); var newRoot = root!.ReplaceNode(objectCreation, newObjectCreation); - var formatted = Formatter.Format(newRoot, Formatter.Annotation, document.Project.Solution.Workspace, cancellationToken: cancellationToken); + var formatted = await Formatter.FormatAsync(document.WithSyntaxRoot(newRoot), Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); // Reduce the fully qualified Partitioner reference where the using is // already present; otherwise keep it qualified so the fix always compiles. - return await Simplifier.ReduceAsync(document.WithSyntaxRoot(formatted), Simplifier.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); + return await Simplifier.ReduceAsync(formatted, Simplifier.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); } } diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs index a03bbc3302..d1c2669ae2 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs @@ -62,8 +62,12 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) .FirstOrDefault(a => a.Left is IdentifierNameSyntax id && id.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty); - if (assignment == null) + if (assignment == null || + assignment.Right is not (MemberAccessExpressionSyntax or IdentifierNameSyntax)) { + // Only a member access (Partitioner.Consistent) or a bare identifier + // (using static) can be rewritten safely; anything else (e.g. a cast) + // would not compile after the fix, so don't offer one. continue; } diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj index f16584964a..66812f3fa8 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj @@ -2,11 +2,7 @@ $(BrighterNetStandardTargetFrameworks) - Rafael Andrade - Code fixes for the brighter library analyzers - Analyzer;CodeFix;Command Processor;Brighter false - true diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 3c1e929f2d..450465db40 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -24,8 +24,10 @@ THE SOFTWARE. */ #endregion using System.Collections.Immutable; +using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; using Paramore.Brighter.Analyzer.Visitors.Operation; namespace Paramore.Brighter.Analyzer.Analyzers; @@ -41,7 +43,8 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer messageFormat: "Partitioner assignment is missing from {0}. Consider setting it explicitly.", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true + isEnabledByDefault: true, + helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT006.md" ); public static readonly DiagnosticDescriptor ConsistentRandomPartitionerRule = new( @@ -51,7 +54,8 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true + isEnabledByDefault: true, + helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT007.md" ); public static readonly DiagnosticDescriptor ConsistentPartitionerRule = new( @@ -61,7 +65,8 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'Consistent' to preserve their current partition assignment.", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true + isEnabledByDefault: true, + helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT008.md" ); public override ImmutableArray SupportedDiagnostics => [MissingPartitionerRule, ConsistentRandomPartitionerRule, ConsistentPartitionerRule]; @@ -75,8 +80,10 @@ public override void Initialize(AnalysisContext context) private static void AnalyzerOperation(OperationAnalysisContext context) { + var operation = (IObjectCreationOperation)context.Operation; + var visitor = new KafkaPublicationPartitionerVisitor(); - context.Operation.Accept(visitor); + operation.Accept(visitor); if (!visitor.IsKafkaPublication) { @@ -85,6 +92,11 @@ private static void AnalyzerOperation(OperationAnalysisContext context) if (!visitor.IsPartitionerAssigned) { + if (IsPartitionerAssignedAfterConstruction(operation)) + { + return; + } + context.ReportDiagnostic(Diagnostic.Create( MissingPartitionerRule, context.Operation.Syntax.GetLocation(), @@ -103,4 +115,42 @@ private static void AnalyzerOperation(OperationAnalysisContext context) context.Operation.Syntax.GetLocation())); } } + + // Recognizes the common pattern where the partitioner is set on the new + // local right after construction, e.g.: + // var publication = new KafkaPublication(); + // publication.Partitioner = Partitioner.Murmur2Random; + // Assignments made elsewhere (helper methods, other blocks) are not tracked. + private static bool IsPartitionerAssignedAfterConstruction(IObjectCreationOperation operation) + { + ILocalSymbol local = operation.Parent switch + { + IVariableInitializerOperation { Parent: IVariableDeclaratorOperation declarator } => declarator.Symbol, + ISimpleAssignmentOperation { Target: ILocalReferenceOperation localReference } => localReference.Local, + _ => null + }; + + if (local == null) + { + return false; + } + + for (var ancestor = operation.Parent; ancestor != null; ancestor = ancestor.Parent) + { + if (ancestor is IBlockOperation block) + { + return block.Operations + .OfType() + .Select(statement => statement.Operation) + .OfType() + .Any(assignment => + assignment.Target is IPropertyReferenceOperation propertyReference && + propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && + propertyReference.Instance is ILocalReferenceOperation instance && + SymbolEqualityComparer.Default.Equals(instance.Local, local)); + } + } + + return false; + } } diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index c4a3d68817..01964319ef 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -30,7 +30,6 @@ public class BrighterAnalyzerGlobals public const string KafkaPublicationClassName = "KafkaPublication"; public const string BrighterAssembly = "Paramore.Brighter"; public const string KafkaMessagingGatewayAssembly = "Paramore.Brighter.MessagingGateway.Kafka"; - public const string KafkaMessagingGatewayNamespace = "Paramore.Brighter.MessagingGateway.Kafka"; public const string RequestTypeProperty = "RequestType"; public const string PartitionerProperty = "Partitioner"; public const string PartitionerEnum = "Partitioner"; diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index 77dd445e88..e87f6b84e7 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -30,6 +30,10 @@ namespace Paramore.Brighter.Analyzer.Visitors.Operation; public class KafkaPublicationPartitionerVisitor : OperationWalker { + private static readonly ChildOfVisitor s_kafkaPublicationCheck = new( + BrighterAnalyzerGlobals.KafkaPublicationClassName, + BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly); + public bool IsKafkaPublication { get; private set; } public bool IsPartitionerAssigned { get; private set; } public bool IsConsistentRandom { get; private set; } @@ -38,9 +42,9 @@ public class KafkaPublicationPartitionerVisitor : OperationWalker public override void VisitObjectCreation(IObjectCreationOperation operation) { - if (operation.Type!.Accept(new ChildOfVisitor(BrighterAnalyzerGlobals.KafkaPublicationClassName, BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly))) + if (operation.Type!.Accept(s_kafkaPublicationCheck)) { - PublicationName = operation.Type.Name; + PublicationName = operation.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); IsKafkaPublication = true; // base walks the children (including the initializer), which drives @@ -55,7 +59,8 @@ public override void VisitObjectCreation(IObjectCreationOperation operation) public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) { if (operation.Target is IPropertyReferenceOperation propertyReference && - propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty) + propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && + propertyReference.Property.ContainingType.Accept(s_kafkaPublicationCheck)) { IsPartitionerAssigned = true; diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index 7a1b0cbc17..5b1b7ee732 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -11,6 +11,10 @@ The Brighter team wants users who work with Kafka to make the `Partitioner` choi ## How to fix Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2Random` is the recommended value for new publications. +**Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. As with [BRT007](./BRT007.md) and [BRT008](./BRT008.md), existing publications that rely on the current partition assignment can safely ignore (or suppress) this warning instead of applying the fix. + +The rule recognizes a `Partitioner` assignment made in the object initializer, or one made directly on the same local variable later in the same block. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. + ### Example ```csharp // Warning: Partitioner assignment is missing diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index eb24515bba..3d09843de1 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -59,7 +59,7 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); await testContext.RunAsync(); } @@ -209,5 +209,93 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Random_Should_Not_Report() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Partitioner.Random + }; + } + } +} +"""; + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Nested_Object_Has_Own_Partitioner_Property_Should_Still_Report_Missing_Partitioner() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class Config + { + public int Partitioner { get; set; } + } + + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + DefaultHeaders = new System.Collections.Generic.Dictionary + { + ["key"] = new Config { Partitioner = 3 } + } + }|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Set_After_Construction_Should_Not_Report() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication(); + publication.Partitioner = Partitioner.Murmur2Random; + } + } +} +"""; + + await testContext.RunAsync(); + } } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs index 7747a4d8fa..b3de695435 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs @@ -19,10 +19,10 @@ protected BaseCodeFixTest() { ReferenceAssemblies = ReferenceAssemblies.Net.Net90 }; - testContext.TestState.OutputKind = OutputKind.ConsoleApplication; + testContext.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location)); testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.CompilerDiagnostics = CompilerDiagnostics.None; + testContext.CompilerDiagnostics = CompilerDiagnostics.Errors; } } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index b00feb401e..324249f64d 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -42,6 +42,41 @@ public void Method() } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Kafka_Using_Is_Missing_Should_Add_Fully_Qualified_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ """ +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication()|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication() { Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random }; + } + } +} """; testContext.ExpectedDiagnostics.Add( From b2b89d873998166afb29f4fec783b9138e59044e Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 11:02:11 +0100 Subject: [PATCH 05/12] Apply code review --- .../MissingPartitionerCodeFixProvider.cs | 67 +++++++--- .../PartitionerValueCodeFixProvider.cs | 9 +- .../KafkaPublicationPartitionerAnalyzer.cs | 124 +++++++++++++----- .../BrighterAnalyzerGlobals.cs | 9 ++ .../KafkaPublicationPartitionerVisitor.cs | 19 ++- .../Analyzers/BaseKafkaAnalyzerTest.cs | 13 ++ ...KafkaPublicationPartitionerAnalyzerTest.cs | 119 +++++++++++++---- .../MissingPartitionerCodeFixProviderTest.cs | 50 +++++++ .../PartitionerValueCodeFixProviderTest.cs | 120 +++++++++++++++++ 9 files changed, 454 insertions(+), 76 deletions(-) create mode 100644 tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index 638e21dfdf..5a90de3189 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -47,25 +47,32 @@ public class MissingPartitionerCodeFixProvider : CodeFixProvider public override async Task RegisterCodeFixesAsync(CodeFixContext context) { var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - - var objectCreation = root?.FindNode(context.Diagnostics[0].Location.SourceSpan) - .DescendantNodesAndSelf() - .OfType() - .FirstOrDefault(); - - if (objectCreation == null) + if (root == null) { return; } - var target = BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue; - - context.RegisterCodeFix( - CodeAction.Create( - title: $"Set 'Partitioner' to 'Partitioner.{target}'", - createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, target, ct), - equivalenceKey: nameof(MissingPartitionerCodeFixProvider)), - context.Diagnostics[0]); + foreach (var diagnostic in context.Diagnostics) + { + var objectCreation = root.FindNode(diagnostic.Location.SourceSpan) + .DescendantNodesAndSelf() + .OfType() + .FirstOrDefault(); + + if (objectCreation == null) + { + continue; + } + + var target = BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue; + + context.RegisterCodeFix( + CodeAction.Create( + title: $"Set 'Partitioner' to 'Partitioner.{target}'", + createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, target, ct), + equivalenceKey: nameof(MissingPartitionerCodeFixProvider)), + diagnostic); + } } private static async Task AddPartitionerAsync( @@ -81,7 +88,7 @@ private static async Task AddPartitionerAsync( SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.PartitionerProperty), SyntaxFactory.MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, - SyntaxFactory.ParseExpression($"{BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly}.{BrighterAnalyzerGlobals.PartitionerEnum}"), + SyntaxFactory.ParseExpression($"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.PartitionerEnum}"), SyntaxFactory.IdentifierName(target)) .WithAdditionalAnnotations(Simplifier.Annotation)); @@ -89,7 +96,7 @@ private static async Task AddPartitionerAsync( ? SyntaxFactory.InitializerExpression( SyntaxKind.ObjectInitializerExpression, SyntaxFactory.SingletonSeparatedList(assignment)) - : objectCreation.Initializer.AddExpressions(assignment); + : AddInitializerExpression(objectCreation.Initializer, assignment); var newObjectCreation = objectCreation .WithInitializer(initializer) @@ -102,4 +109,30 @@ private static async Task AddPartitionerAsync( // already present; otherwise keep it qualified so the fix always compiles. return await Simplifier.ReduceAsync(formatted, Simplifier.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); } + + private static InitializerExpressionSyntax AddInitializerExpression( + InitializerExpressionSyntax initializer, + ExpressionSyntax expression) + { + // InitializerExpressionSyntax.AddExpressions inserts the separator comma right + // after the last expression but before its trailing trivia, so the comma ends + // up on the wrong line. Rewire the trivia by hand: the newline + indent that + // follows an existing separator (or the open brace) leads the new expression, + // and the last expression's trailing trivia (e.g. the newline before the + // closing brace) moves behind the new expression. + var lastExpression = initializer.Expressions.Last(); + + var leadingTrivia = initializer.Expressions.Count > 1 + ? initializer.Expressions.GetSeparator(initializer.Expressions.Count - 2).TrailingTrivia + : initializer.OpenBraceToken.TrailingTrivia; + + var newExpression = expression + .WithLeadingTrivia(leadingTrivia) + .WithTrailingTrivia(lastExpression.GetTrailingTrivia()); + + return initializer.WithExpressions( + initializer.Expressions + .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) + .Add(newExpression)); + } } diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs index d1c2669ae2..edda48b112 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs @@ -59,8 +59,13 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) var assignment = root.FindNode(diagnostic.Location.SourceSpan) .DescendantNodesAndSelf() .OfType() - .FirstOrDefault(a => a.Left is IdentifierNameSyntax id && - id.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty); + .FirstOrDefault(a => a.Left switch + { + // Partitioner = ... (object initializer) or publication.Partitioner = ... (post-construction) + IdentifierNameSyntax id => id.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty, + MemberAccessExpressionSyntax memberAccess => memberAccess.Name.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty, + _ => false + }); if (assignment == null || assignment.Right is not (MemberAccessExpressionSyntax or IdentifierNameSyntax)) diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 450465db40..e93eb98d57 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -50,22 +50,22 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer public static readonly DiagnosticDescriptor ConsistentRandomPartitionerRule = new( id: DiagnosticsIds.ConsistentRandomPartitioner, title: "ConsistentRandom Partitioner Used", - messageFormat: - "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", + messageFormat: "Prefer 'Murmur2Random' over 'ConsistentRandom' for new KafkaPublications", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "'ConsistentRandom' can produce uneven key distribution and hot partitions; 'Murmur2Random' keeps distribution even. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT007.md" ); public static readonly DiagnosticDescriptor ConsistentPartitionerRule = new( id: DiagnosticsIds.ConsistentPartitioner, title: "Consistent Partitioner Used", - messageFormat: - "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications to keep key distribution even and avoid hot partitions. Existing publications can keep 'Consistent' to preserve their current partition assignment.", + messageFormat: "Prefer 'Murmur2' over 'Consistent' for new KafkaPublications", category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "'Consistent' can produce uneven key distribution and hot partitions; 'Murmur2' keeps distribution even. Existing publications can keep 'Consistent' to preserve their current partition assignment.", helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT008.md" ); @@ -75,25 +75,41 @@ public override void Initialize(AnalysisContext context) { context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); - context.RegisterOperationAction(AnalyzerOperation, OperationKind.ObjectCreation); + context.RegisterCompilationStartAction(compilationContext => + { + // Solutions that don't reference the Kafka gateway can never create a + // KafkaPublication; don't pay for an operation callback there at all. + if (compilationContext.Compilation.GetTypeByMetadataName( + $"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.KafkaPublicationClassName}") == null) + { + return; + } + + compilationContext.RegisterOperationAction(AnalyzerObjectCreation, OperationKind.ObjectCreation); + compilationContext.RegisterOperationAction(AnalyzeAssignment, OperationKind.SimpleAssignment); + }); } - private static void AnalyzerOperation(OperationAnalysisContext context) + private static void AnalyzerObjectCreation(OperationAnalysisContext context) { var operation = (IObjectCreationOperation)context.Operation; - var visitor = new KafkaPublicationPartitionerVisitor(); - operation.Accept(visitor); - - if (!visitor.IsKafkaPublication) + // Cheap rejection before allocating the visitor; most object creations + // in a compilation are not KafkaPublications. + if (!KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(operation.Type)) { return; } + var visitor = new KafkaPublicationPartitionerVisitor(); + operation.Accept(visitor); + if (!visitor.IsPartitionerAssigned) { - if (IsPartitionerAssignedAfterConstruction(operation)) + if (FindPartitionerAssignmentAfterConstruction(operation) != null) { + // The partitioner is set on the local after construction; any + // discouraged value is reported by AnalyzeAssignment instead. return; } @@ -116,12 +132,54 @@ private static void AnalyzerOperation(OperationAnalysisContext context) } } - // Recognizes the common pattern where the partitioner is set on the new - // local right after construction, e.g.: + // Reports discouraged partitioner values assigned outside an object + // initializer, e.g.: + // var publication = new KafkaPublication(); + // publication.Partitioner = Partitioner.Consistent; + // The diagnostic is reported from the assignment's own callback so it stays + // local to the analyzed operation. + private static void AnalyzeAssignment(OperationAnalysisContext context) + { + var assignment = (ISimpleAssignmentOperation)context.Operation; + + // Initializer assignments (new KafkaPublication { Partitioner = ... }) are + // handled by AnalyzerObjectCreation. Their parent is an + // IObjectOrCollectionInitializerOperation; IMemberInitializerOperation + // only appears in `with` expressions. + if (assignment.Parent is IObjectOrCollectionInitializerOperation or IMemberInitializerOperation) + { + return; + } + + if (assignment.Target is not IPropertyReferenceOperation propertyReference || + propertyReference.Property.Name != BrighterAnalyzerGlobals.PartitionerProperty || + !KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(propertyReference.Property.ContainingType)) + { + return; + } + + switch (KafkaPublicationPartitionerVisitor.GetPartitionerValueName(assignment.Value)) + { + case BrighterAnalyzerGlobals.ConsistentRandomPartitionerValue: + context.ReportDiagnostic(Diagnostic.Create( + ConsistentRandomPartitionerRule, + assignment.Syntax.GetLocation())); + break; + case BrighterAnalyzerGlobals.ConsistentPartitionerValue: + context.ReportDiagnostic(Diagnostic.Create( + ConsistentPartitionerRule, + assignment.Syntax.GetLocation())); + break; + } + } + + // Finds a partitioner assignment made on the new local right after + // construction, e.g.: // var publication = new KafkaPublication(); // publication.Partitioner = Partitioner.Murmur2Random; - // Assignments made elsewhere (helper methods, other blocks) are not tracked. - private static bool IsPartitionerAssignedAfterConstruction(IObjectCreationOperation operation) + // Returns null when there is no such assignment. Assignments made + // elsewhere (helper methods, other blocks) are not tracked. + private static ISimpleAssignmentOperation FindPartitionerAssignmentAfterConstruction(IObjectCreationOperation operation) { ILocalSymbol local = operation.Parent switch { @@ -132,25 +190,31 @@ private static bool IsPartitionerAssignedAfterConstruction(IObjectCreationOperat if (local == null) { - return false; + return null; } - for (var ancestor = operation.Parent; ancestor != null; ancestor = ancestor.Parent) + // Only the nearest enclosing block is searched; an assignment inside a + // nested block (e.g. an if) still triggers BRT006 — a documented + // limitation (see BRT006.md). + var ancestor = operation.Parent; + while (ancestor != null && ancestor is not IBlockOperation) { - if (ancestor is IBlockOperation block) - { - return block.Operations - .OfType() - .Select(statement => statement.Operation) - .OfType() - .Any(assignment => - assignment.Target is IPropertyReferenceOperation propertyReference && - propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && - propertyReference.Instance is ILocalReferenceOperation instance && - SymbolEqualityComparer.Default.Equals(instance.Local, local)); - } + ancestor = ancestor.Parent; + } + + if (ancestor is not IBlockOperation block) + { + return null; } - return false; + return block.Operations + .OfType() + .Select(statement => statement.Operation) + .OfType() + .FirstOrDefault(assignment => + assignment.Target is IPropertyReferenceOperation propertyReference && + propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && + propertyReference.Instance is ILocalReferenceOperation instance && + SymbolEqualityComparer.Default.Equals(instance.Local, local)); } } diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index 01964319ef..559316488d 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -30,7 +30,16 @@ public class BrighterAnalyzerGlobals public const string KafkaPublicationClassName = "KafkaPublication"; public const string BrighterAssembly = "Paramore.Brighter"; public const string KafkaMessagingGatewayAssembly = "Paramore.Brighter.MessagingGateway.Kafka"; + + // The Kafka namespace happens to equal the assembly name today; keep them as + // separate constants so renaming the assembly can't silently break code that + // needs the namespace (metadata names, generated qualified references). + public const string KafkaNamespace = "Paramore.Brighter.MessagingGateway.Kafka"; public const string RequestTypeProperty = "RequestType"; + + // PartitionerProperty (the KafkaPublication property) and PartitionerEnum (the + // enum type) intentionally share the value "Partitioner" — they name different + // symbols and could diverge. public const string PartitionerProperty = "Partitioner"; public const string PartitionerEnum = "Partitioner"; public const string ConsistentRandomPartitionerValue = "ConsistentRandom"; diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index e87f6b84e7..0d3f47e6c1 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -40,18 +40,27 @@ public class KafkaPublicationPartitionerVisitor : OperationWalker public bool IsConsistent { get; private set; } public string PublicationName { get; private set; } + // Type can be null for erroneous code in the IDE; treat it as no match. + internal static bool IsKafkaPublicationType(ITypeSymbol type) + { + return type != null && type.Accept(s_kafkaPublicationCheck); + } + public override void VisitObjectCreation(IObjectCreationOperation operation) { - if (operation.Type!.Accept(s_kafkaPublicationCheck)) + if (IsKafkaPublicationType(operation.Type)) { PublicationName = operation.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); IsKafkaPublication = true; // base walks the children (including the initializer), which drives // VisitSimpleAssignment for any Partitioner assignment. Only descend - // when this operation is the KafkaPublication itself; descending into - // unrelated object creations would pick up nested publications and - // report the diagnostic at the wrong location. + // when this operation is the KafkaPublication itself; reporting for + // unrelated object creations would attribute nested publications to + // the wrong location. Note this also descends into nested object + // creations, so a nested KafkaPublication carrying its own Partitioner + // would mark the outer one as assigned too — a contrived edge case + // accepted for simplicity. base.VisitObjectCreation(operation); } } @@ -78,7 +87,7 @@ public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) base.VisitSimpleAssignment(operation); } - private static string GetPartitionerValueName(IOperation value) + internal static string GetPartitionerValueName(IOperation value) { // Unwrap an implicit conversion (e.g. enum widening) if present. if (value is IConversionOperation conversion) diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs new file mode 100644 index 0000000000..ae7f861465 --- /dev/null +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs @@ -0,0 +1,13 @@ +using Microsoft.CodeAnalysis; +using Paramore.Brighter.Analyzer.Analyzers; + +namespace Paramore.Brighter.Analyzer.Tests.Analyzers +{ + public abstract class BaseKafkaAnalyzerTest : BaseAnalyzerTest + { + protected BaseKafkaAnalyzerTest() + { + testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); + } + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index 3d09843de1..971f8c95ac 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -1,18 +1,15 @@ using System.Threading.Tasks; -using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Xunit; using Paramore.Brighter.Analyzer.Analyzers; namespace Paramore.Brighter.Analyzer.Tests.Analyzers { - public class KafkaPublicationPartitionerAnalyzerTest : BaseAnalyzerTest + public class KafkaPublicationPartitionerAnalyzerTest : BaseKafkaAnalyzerTest { [Fact] public async Task When_KafkaPublication_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -36,8 +33,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Generic_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -67,8 +62,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Is_Created_With_ConsistentRandom_Should_Report_Warning() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -95,8 +88,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Is_Created_With_Consistent_Should_Report_Warning() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -123,8 +114,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Is_Created_With_Murmur2Random_Should_Not_Report() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -150,8 +139,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Without_Partitioner_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -180,8 +167,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_With_Consistent_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -213,8 +198,6 @@ public void Method() [Fact] public async Task When_KafkaPublication_Is_Created_With_Random_Should_Not_Report() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -240,8 +223,6 @@ public void Method() [Fact] public async Task When_Nested_Object_Has_Own_Partitioner_Property_Should_Still_Report_Missing_Partitioner() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -276,8 +257,6 @@ public void Method() [Fact] public async Task When_Partitioner_Is_Set_After_Construction_Should_Not_Report() { - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -297,5 +276,101 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Consistent_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication(); + {|#0:publication.Partitioner = Partitioner.Consistent|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_ConsistentRandom_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication(); + {|#0:publication.Partitioner = Partitioner.ConsistentRandom|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Plain_Publication_Is_Created_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new Publication(); + } + } +} +"""; + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Subclass_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class MyPublication : KafkaPublication + { + } + + class TypeName + { + public void Method() + { + var publication = {|#0:new MyPublication()|}; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("MyPublication")); + + await testContext.RunAsync(); + } } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index 324249f64d..b19bced5f9 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -77,6 +77,56 @@ public void Method() } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_To_Existing_Initializer() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Topic = new RoutingKey("x"), + NumPartitions = 3 + }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Topic = new RoutingKey("x"), + NumPartitions = 3, + Partitioner = Partitioner.Murmur2Random + }; + } + } +} """; testContext.ExpectedDiagnostics.Add( diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs index ce41ff49df..59e832653d 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs @@ -95,6 +95,126 @@ public void Method() } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Bare_Identifier_Via_Using_Static_Is_Used_Should_Offer_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; +using static Paramore.Brighter.MessagingGateway.Kafka.Partitioner; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = ConsistentRandom + }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; +using static Paramore.Brighter.MessagingGateway.Kafka.Partitioner; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Murmur2Random + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Consistent_Is_Set_After_Construction_Should_Offer_Murmur2() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication(); + {|#0:publication.Partitioner = Partitioner.Consistent|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication(); + publication.Partitioner = Partitioner.Murmur2; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Value_Is_Parenthesized_Should_Not_Offer_Fix() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Partitioner = (Partitioner.Consistent) + }|}; + } + } +} """; testContext.ExpectedDiagnostics.Add( From 3c3de0b4328f1a7c7e3bc3819cb94af06666e63f Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:52:16 +0100 Subject: [PATCH 06/12] Apply code review --- .../MissingPartitionerCodeFixProvider.cs | 28 +++- .../PartitionerValueCodeFixProvider.cs | 19 ++- ...aramore.Brighter.Analyzer.CodeFixes.csproj | 3 + .../Analyzers/BaseKafkaAnalyzerTest.cs | 3 + ...KafkaPublicationPartitionerAnalyzerTest.cs | 128 ++++++++++++++++-- .../MissingPartitionerCodeFixProviderTest.cs | 94 +++++++++++++ .../PartitionerValueCodeFixProviderTest.cs | 45 ++++-- 7 files changed, 284 insertions(+), 36 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index 5a90de3189..354db2d8bb 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -118,9 +118,14 @@ private static InitializerExpressionSyntax AddInitializerExpression( // after the last expression but before its trailing trivia, so the comma ends // up on the wrong line. Rewire the trivia by hand: the newline + indent that // follows an existing separator (or the open brace) leads the new expression, - // and the last expression's trailing trivia (e.g. the newline before the - // closing brace) moves behind the new expression. + // and the newline before the closing brace moves behind the new expression. + // Comments stay with the expression they document: anything before the final + // newline (e.g. "// one per shard") becomes the separator's trailing trivia. var lastExpression = initializer.Expressions.Last(); + var trailingTrivia = lastExpression.GetTrailingTrivia(); + + var commentTrivia = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)); + var endOfLineTrivia = trailingTrivia.SkipWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)); var leadingTrivia = initializer.Expressions.Count > 1 ? initializer.Expressions.GetSeparator(initializer.Expressions.Count - 2).TrailingTrivia @@ -128,11 +133,20 @@ private static InitializerExpressionSyntax AddInitializerExpression( var newExpression = expression .WithLeadingTrivia(leadingTrivia) - .WithTrailingTrivia(lastExpression.GetTrailingTrivia()); + .WithTrailingTrivia(endOfLineTrivia); + + var expressions = initializer.Expressions + .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) + .Add(newExpression); + + if (commentTrivia.Any()) + { + var nodesAndTokens = expressions.GetWithSeparators(); + var separator = nodesAndTokens[nodesAndTokens.Count - 2].AsToken().WithTrailingTrivia(commentTrivia); + nodesAndTokens = nodesAndTokens.Replace(nodesAndTokens[nodesAndTokens.Count - 2], separator); + expressions = SyntaxFactory.SeparatedList(nodesAndTokens); + } - return initializer.WithExpressions( - initializer.Expressions - .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) - .Add(newExpression)); + return initializer.WithExpressions(expressions); } } diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs index edda48b112..3da1fa92c5 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs @@ -67,12 +67,12 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) _ => false }); - if (assignment == null || - assignment.Right is not (MemberAccessExpressionSyntax or IdentifierNameSyntax)) + if (assignment == null || !CanRewrite(assignment.Right)) { - // Only a member access (Partitioner.Consistent) or a bare identifier - // (using static) can be rewritten safely; anything else (e.g. a cast) - // would not compile after the fix, so don't offer one. + // Only a member access (Partitioner.Consistent), a parenthesized + // member access, or a bare identifier (using static) can be + // rewritten safely; anything else (e.g. a cast) would not + // compile after the fix, so don't offer one. continue; } @@ -85,6 +85,13 @@ assignment.Right is not (MemberAccessExpressionSyntax or IdentifierNameSyntax)) } } + private static bool CanRewrite(ExpressionSyntax right) + { + return right is MemberAccessExpressionSyntax + or IdentifierNameSyntax + or ParenthesizedExpressionSyntax { Expression: MemberAccessExpressionSyntax }; + } + private static async Task ReplacePartitionerValueAsync( Document document, AssignmentExpressionSyntax assignment, @@ -97,6 +104,8 @@ private static async Task ReplacePartitionerValueAsync( ExpressionSyntax newValue = assignment.Right switch { MemberAccessExpressionSyntax memberAccess => memberAccess.WithName(newName), + ParenthesizedExpressionSyntax { Expression: MemberAccessExpressionSyntax memberAccess } parenthesized => + parenthesized.WithExpression(memberAccess.WithName(newName)), _ => newName }; diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj index 66812f3fa8..d3da459921 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj @@ -3,6 +3,9 @@ $(BrighterNetStandardTargetFrameworks) false + diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs index ae7f861465..8d75d095ef 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs @@ -1,4 +1,5 @@ using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; using Paramore.Brighter.Analyzer.Analyzers; namespace Paramore.Brighter.Analyzer.Tests.Analyzers @@ -7,7 +8,9 @@ public abstract class BaseKafkaAnalyzerTest : BaseAnalyzerTest + { + {|#0:Partitioner = Partitioner.ConsistentRandom|} + }; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + + await testContext.RunAsync(); + } } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index b19bced5f9..eb97a828b2 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -127,6 +127,100 @@ public void Method() } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_After_Trailing_Comment() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication + { + Topic = new RoutingKey("x"), + NumPartitions = 3 // one per shard + }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Topic = new RoutingKey("x"), + NumPartitions = 3, // one per shard + Partitioner = Partitioner.Murmur2Random + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_On_Target_Typed_New_Should_Add_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + KafkaPublication publication = {|#0:new()|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + KafkaPublication publication = new() + { + Partitioner = Partitioner.Murmur2Random + }; + } + } +} """; testContext.ExpectedDiagnostics.Add( diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs index 59e832653d..d6dc1592a6 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs @@ -22,10 +22,10 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new KafkaPublication { - Partitioner = Partitioner.ConsistentRandom - }|}; + {|#0:Partitioner = Partitioner.ConsistentRandom|} + }; } } } @@ -69,10 +69,10 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new KafkaPublication { - Partitioner = Partitioner.Consistent - }|}; + {|#0:Partitioner = Partitioner.Consistent|} + }; } } } @@ -117,10 +117,10 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new KafkaPublication { - Partitioner = ConsistentRandom - }|}; + {|#0:Partitioner = ConsistentRandom|} + }; } } } @@ -196,7 +196,7 @@ public void Method() } [Fact] - public async Task When_Value_Is_Parenthesized_Should_Not_Offer_Fix() + public async Task When_Value_Is_Parenthesized_Should_Offer_Murmur2() { testContext.TestCode = /* lang=c#-test */ """ using Paramore.Brighter; @@ -208,10 +208,29 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new KafkaPublication + { + {|#0:Partitioner = (Partitioner.Consistent)|} + }; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication { - Partitioner = (Partitioner.Consistent) - }|}; + Partitioner = (Partitioner.Murmur2) + }; } } } From a23655a6eff2fb0776fce127ee65f7c1b2fde14d Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:55:17 +0100 Subject: [PATCH 07/12] Add missing files --- .../KafkaPublicationPartitionerAnalyzer.cs | 48 ++++++++++++------- .../KafkaPublicationPartitionerVisitor.cs | 6 +-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index e93eb98d57..872674ac77 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -44,6 +44,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, + description: "Setting the Partitioner explicitly makes the choice visible. Be aware that changing the partitioner re-partitions the topic; existing publications can keep the implicit default to preserve their current partition assignment.", helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT006.md" ); @@ -106,9 +107,9 @@ private static void AnalyzerObjectCreation(OperationAnalysisContext context) if (!visitor.IsPartitionerAssigned) { - if (FindPartitionerAssignmentAfterConstruction(operation) != null) + if (HasPartitionerAssignmentAfterConstruction(operation)) { - // The partitioner is set on the local after construction; any + // The partitioner is set on the instance after construction; any // discouraged value is reported by AnalyzeAssignment instead. return; } @@ -122,13 +123,13 @@ private static void AnalyzerObjectCreation(OperationAnalysisContext context) { context.ReportDiagnostic(Diagnostic.Create( ConsistentRandomPartitionerRule, - context.Operation.Syntax.GetLocation())); + visitor.PartitionerAssignmentLocation)); } else if (visitor.IsConsistent) { context.ReportDiagnostic(Diagnostic.Create( ConsistentPartitionerRule, - context.Operation.Syntax.GetLocation())); + visitor.PartitionerAssignmentLocation)); } } @@ -173,24 +174,27 @@ private static void AnalyzeAssignment(OperationAnalysisContext context) } } - // Finds a partitioner assignment made on the new local right after - // construction, e.g.: + // Checks whether the partitioner is assigned on the just-created instance + // later in the same block, e.g.: // var publication = new KafkaPublication(); // publication.Partitioner = Partitioner.Murmur2Random; - // Returns null when there is no such assignment. Assignments made - // elsewhere (helper methods, other blocks) are not tracked. - private static ISimpleAssignmentOperation FindPartitionerAssignmentAfterConstruction(IObjectCreationOperation operation) + // Works for locals, fields, properties and parameters. Assignments made + // before the construction, or elsewhere (helper methods, other blocks), + // are not tracked. + private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOperation operation) { - ILocalSymbol local = operation.Parent switch + ISymbol symbol = operation.Parent switch { IVariableInitializerOperation { Parent: IVariableDeclaratorOperation declarator } => declarator.Symbol, ISimpleAssignmentOperation { Target: ILocalReferenceOperation localReference } => localReference.Local, + ISimpleAssignmentOperation { Target: IFieldReferenceOperation fieldReference } => fieldReference.Field, + ISimpleAssignmentOperation { Target: IPropertyReferenceOperation propertyReference } => propertyReference.Property, _ => null }; - if (local == null) + if (symbol == null) { - return null; + return false; } // Only the nearest enclosing block is searched; an assignment inside a @@ -204,17 +208,29 @@ private static ISimpleAssignmentOperation FindPartitionerAssignmentAfterConstruc if (ancestor is not IBlockOperation block) { - return null; + return false; } return block.Operations .OfType() .Select(statement => statement.Operation) .OfType() - .FirstOrDefault(assignment => + .Any(assignment => + assignment.Syntax.SpanStart > operation.Syntax.SpanStart && assignment.Target is IPropertyReferenceOperation propertyReference && propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && - propertyReference.Instance is ILocalReferenceOperation instance && - SymbolEqualityComparer.Default.Equals(instance.Local, local)); + IsReferenceTo(propertyReference.Instance, symbol)); + } + + private static bool IsReferenceTo(IOperation instance, ISymbol symbol) + { + return instance switch + { + ILocalReferenceOperation localReference => SymbolEqualityComparer.Default.Equals(localReference.Local, symbol), + IFieldReferenceOperation fieldReference => SymbolEqualityComparer.Default.Equals(fieldReference.Field, symbol), + IPropertyReferenceOperation propertyReference => SymbolEqualityComparer.Default.Equals(propertyReference.Property, symbol), + IParameterReferenceOperation parameterReference => SymbolEqualityComparer.Default.Equals(parameterReference.Parameter, symbol), + _ => false + }; } } diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index 0d3f47e6c1..a61319cca7 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -34,11 +34,11 @@ public class KafkaPublicationPartitionerVisitor : OperationWalker BrighterAnalyzerGlobals.KafkaPublicationClassName, BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly); - public bool IsKafkaPublication { get; private set; } public bool IsPartitionerAssigned { get; private set; } public bool IsConsistentRandom { get; private set; } public bool IsConsistent { get; private set; } public string PublicationName { get; private set; } + public Location PartitionerAssignmentLocation { get; private set; } // Type can be null for erroneous code in the IDE; treat it as no match. internal static bool IsKafkaPublicationType(ITypeSymbol type) @@ -51,7 +51,6 @@ public override void VisitObjectCreation(IObjectCreationOperation operation) if (IsKafkaPublicationType(operation.Type)) { PublicationName = operation.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); - IsKafkaPublication = true; // base walks the children (including the initializer), which drives // VisitSimpleAssignment for any Partitioner assignment. Only descend @@ -69,9 +68,10 @@ public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) { if (operation.Target is IPropertyReferenceOperation propertyReference && propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && - propertyReference.Property.ContainingType.Accept(s_kafkaPublicationCheck)) + IsKafkaPublicationType(propertyReference.Property.ContainingType)) { IsPartitionerAssigned = true; + PartitionerAssignmentLocation = operation.Syntax.GetLocation(); switch (GetPartitionerValueName(operation.Value)) { From 1e320c6904ab92e61c42d07123a801bd7efe2f90 Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:41:39 +0100 Subject: [PATCH 08/12] Apply code review --- flake.nix | 38 ++ samples/AsyncAPI/KafkaAsyncAPI/Program.cs | 3 + .../GreetingsSender/Program.cs | 3 + .../GreetingsSender/Program.cs | 3 + .../TaskStatusSender/Program.cs | 8 +- .../GreetingsSender/Program.cs | 5 +- .../KafkaTaskQueue/GreetingsSender/Program.cs | 5 +- .../GreetingsSender/Program.cs | 5 +- .../MultiBus/GreetingsSender/Program.cs | 5 +- .../TransportMaker/ConfigureTransport.cs | 5 +- .../MissingPartitionerCodeFixProvider.cs | 54 ++- .../KafkaPublicationPartitionerAnalyzer.cs | 120 ++++- .../KafkaPublicationPartitionerVisitor.cs | 48 +- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 2 +- src/Paramore.Brighter.Analyzer/docs/BRT007.md | 4 +- src/Paramore.Brighter.Analyzer/docs/BRT008.md | 6 +- .../Analyzers/BaseAnalyzerTest.cs | 32 +- .../Analyzers/BaseKafkaAnalyzerTest.cs | 21 +- ...KafkaPublicationPartitionerAnalyzerTest.cs | 451 +++++++++++++----- ...cationRequestTypeAssignmentAnalyzerTest.cs | 69 +-- .../SubscriptionConstructorAnalyzerTest.cs | 57 ++- .../Analyzers/WrapAttributeAnalyzerTest.cs | 30 +- .../CodeFixes/BaseCodeFixTest.cs | 43 +- .../MissingPartitionerCodeFixProviderTest.cs | 235 +++++++-- .../PartitionerValueCodeFixProviderTest.cs | 239 ++++++++-- 25 files changed, 1101 insertions(+), 390 deletions(-) create mode 100644 flake.nix diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000000..1e74ce47c3 --- /dev/null +++ b/flake.nix @@ -0,0 +1,38 @@ +{ + description = "A very basic flake"; + + inputs = { + nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable"; + }; + + outputs = + { + nixpkgs, + flake-utils, + ... + }: + flake-utils.lib.eachDefaultSystem ( + system: + let + # pkgs = nixpkgs.legacyPackages.${system}; + pkgs = import nixpkgs { inherit system; }; + + # Define the .NET SDK version you want to use + dotnetSdk = pkgs.dotnetCorePackages.sdk_10_0-bin; + in + { + devShells.default = pkgs.mkShell { + packages = [ + dotnetSdk + + pkgs.netcoredbg # Debugger for .NET Core + # pkgs.roslyn-ls # LSP for VS Code / Emacs / Vim + ]; + }; + + # Environment variables + # 1. Essential: Tell dotnet tools where to find the SDK + DOTNET_ROOT = "${dotnetSdk}"; + } + ); +} diff --git a/samples/AsyncAPI/KafkaAsyncAPI/Program.cs b/samples/AsyncAPI/KafkaAsyncAPI/Program.cs index b0dd624236..df3cbd0e47 100644 --- a/samples/AsyncAPI/KafkaAsyncAPI/Program.cs +++ b/samples/AsyncAPI/KafkaAsyncAPI/Program.cs @@ -81,6 +81,9 @@ THE SOFTWARE. */ new() { Topic = new RoutingKey("order.created"), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs index 05e9bc87b3..8a76e7fcab 100644 --- a/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs @@ -59,6 +59,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs index 8f7df77794..e7a33c7c4c 100644 --- a/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs @@ -59,6 +59,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs b/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs index d46a0d5caa..f84d49ea3a 100644 --- a/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs +++ b/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2025 Ian Cooper @@ -52,6 +52,9 @@ THE SOFTWARE. */ //the same topic for both TaskCreated and TaskUpdated, but different cloud events types Topic = new RoutingKey("task.update"), Type = new CloudEventsType("io.goparamore.task.created"), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, @@ -61,6 +64,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("task.update"), Type = new CloudEventsType("io.goparamore.task.updated"), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs index addd77d556..d659193e5d 100644 --- a/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -62,6 +62,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, MaxInFlightRequestsPerConnection = 1 diff --git a/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs index 26de70c991..03ac66e83b 100644 --- a/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -80,6 +80,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs index 93f5444c43..73b538a226 100644 --- a/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -96,6 +96,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs b/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs index f389e4503b..e5dbefb4ad 100644 --- a/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs +++ b/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -72,6 +72,9 @@ THE SOFTWARE. */ { Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs b/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs index 3730b9b839..acdd631973 100644 --- a/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs +++ b/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs @@ -1,4 +1,4 @@ -using Confluent.Kafka; +using Confluent.Kafka; using Confluent.SchemaRegistry; using Microsoft.Extensions.DependencyInjection; using Paramore.Brighter; @@ -97,6 +97,9 @@ public static IAmAProducerRegistry GetKafkaProducerRegistry() where T: class, { Topic = new RoutingKey(typeof(T).Name), RequestType = typeof(T), + // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across + // partitions, avoiding hot partitions, and matches the standard Kafka client default + Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, MaxInFlightRequestsPerConnection = 1, diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index 354db2d8bb..af88d54a0e 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -64,12 +64,10 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) continue; } - var target = BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue; - context.RegisterCodeFix( CodeAction.Create( - title: $"Set 'Partitioner' to 'Partitioner.{target}'", - createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, target, ct), + title: $"Set 'Partitioner' to 'Partitioner.{BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue}'", + createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, ct), equivalenceKey: nameof(MissingPartitionerCodeFixProvider)), diagnostic); } @@ -78,7 +76,6 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) private static async Task AddPartitionerAsync( Document document, BaseObjectCreationExpressionSyntax objectCreation, - string target, CancellationToken cancellationToken) { var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); @@ -89,7 +86,7 @@ private static async Task AddPartitionerAsync( SyntaxFactory.MemberAccessExpression( SyntaxKind.SimpleMemberAccessExpression, SyntaxFactory.ParseExpression($"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.PartitionerEnum}"), - SyntaxFactory.IdentifierName(target)) + SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue)) .WithAdditionalAnnotations(Simplifier.Annotation)); var initializer = objectCreation.Initializer == null @@ -124,29 +121,52 @@ private static InitializerExpressionSyntax AddInitializerExpression( var lastExpression = initializer.Expressions.Last(); var trailingTrivia = lastExpression.GetTrailingTrivia(); - var commentTrivia = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)); - var endOfLineTrivia = trailingTrivia.SkipWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)); - - var leadingTrivia = initializer.Expressions.Count > 1 - ? initializer.Expressions.GetSeparator(initializer.Expressions.Count - 2).TrailingTrivia - : initializer.OpenBraceToken.TrailingTrivia; + var beforeEndOfLine = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); + var fromEndOfLine = trailingTrivia.SkipWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); - var newExpression = expression - .WithLeadingTrivia(leadingTrivia) - .WithTrailingTrivia(endOfLineTrivia); + SyntaxTriviaList separatorTrailingTrivia; + ExpressionSyntax newExpression; + if (fromEndOfLine.Count == 0) + { + // Single-line initializer ("{ Topic = x }"): keep it on one line, + // with single spaces around the new expression. + separatorTrailingTrivia = beforeEndOfLine.Any(IsComment) + ? SyntaxFactory.TriviaList(beforeEndOfLine) + : default; + newExpression = expression + .WithLeadingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.Space)) + .WithTrailingTrivia(separatorTrailingTrivia.Count > 0 + ? SyntaxFactory.TriviaList(SyntaxFactory.Space) + : SyntaxFactory.TriviaList(beforeEndOfLine)); + } + else + { + separatorTrailingTrivia = SyntaxFactory.TriviaList(beforeEndOfLine); + var leadingTrivia = initializer.Expressions.Count > 1 + ? initializer.Expressions.GetSeparator(initializer.Expressions.Count - 2).TrailingTrivia + : initializer.OpenBraceToken.TrailingTrivia; + newExpression = expression + .WithLeadingTrivia(leadingTrivia) + .WithTrailingTrivia(fromEndOfLine); + } var expressions = initializer.Expressions .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) .Add(newExpression); - if (commentTrivia.Any()) + if (separatorTrailingTrivia.Count > 0) { var nodesAndTokens = expressions.GetWithSeparators(); - var separator = nodesAndTokens[nodesAndTokens.Count - 2].AsToken().WithTrailingTrivia(commentTrivia); + var separator = nodesAndTokens[nodesAndTokens.Count - 2].AsToken().WithTrailingTrivia(separatorTrailingTrivia); nodesAndTokens = nodesAndTokens.Replace(nodesAndTokens[nodesAndTokens.Count - 2], separator); expressions = SyntaxFactory.SeparatedList(nodesAndTokens); } return initializer.WithExpressions(expressions); } + + private static bool IsComment(SyntaxTrivia trivia) + { + return trivia.IsKind(SyntaxKind.SingleLineCommentTrivia) || trivia.IsKind(SyntaxKind.MultiLineCommentTrivia); + } } diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 872674ac77..3a4bed2423 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -26,6 +26,7 @@ THE SOFTWARE. */ using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; using Paramore.Brighter.Analyzer.Visitors.Operation; @@ -55,7 +56,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "'ConsistentRandom' can produce uneven key distribution and hot partitions; 'Murmur2Random' keeps distribution even. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", + description: "'Murmur2Random' spreads keys more evenly across partitions than the CRC32-based 'ConsistentRandom', avoiding hot partitions. Existing publications can keep 'ConsistentRandom' to preserve their current partition assignment.", helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT007.md" ); @@ -66,7 +67,7 @@ public class KafkaPublicationPartitionerAnalyzer : DiagnosticAnalyzer category: PartitionerCategory, defaultSeverity: DiagnosticSeverity.Warning, isEnabledByDefault: true, - description: "'Consistent' can produce uneven key distribution and hot partitions; 'Murmur2' keeps distribution even. Existing publications can keep 'Consistent' to preserve their current partition assignment.", + description: "'Murmur2' spreads keys more evenly across partitions than the CRC32-based 'Consistent', avoiding hot partitions. Existing publications can keep 'Consistent' to preserve their current partition assignment.", helpLinkUri: "https://github.com/BrighterCommand/Brighter/blob/master/src/Paramore.Brighter.Analyzer/docs/BRT008.md" ); @@ -80,37 +81,50 @@ public override void Initialize(AnalysisContext context) { // Solutions that don't reference the Kafka gateway can never create a // KafkaPublication; don't pay for an operation callback there at all. - if (compilationContext.Compilation.GetTypeByMetadataName( - $"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.KafkaPublicationClassName}") == null) + // Resolve the symbols once and compare by symbol from here on. + var kafkaPublicationSymbol = compilationContext.Compilation.GetTypeByMetadataName( + $"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.KafkaPublicationClassName}"); + var partitionerEnumSymbol = compilationContext.Compilation.GetTypeByMetadataName( + $"{BrighterAnalyzerGlobals.KafkaNamespace}.{BrighterAnalyzerGlobals.PartitionerEnum}"); + if (kafkaPublicationSymbol == null || partitionerEnumSymbol == null) { return; } - compilationContext.RegisterOperationAction(AnalyzerObjectCreation, OperationKind.ObjectCreation); - compilationContext.RegisterOperationAction(AnalyzeAssignment, OperationKind.SimpleAssignment); + compilationContext.RegisterOperationAction( + operationContext => AnalyzerObjectCreation(operationContext, kafkaPublicationSymbol, partitionerEnumSymbol), + OperationKind.ObjectCreation); + compilationContext.RegisterOperationAction( + operationContext => AnalyzeAssignment(operationContext, kafkaPublicationSymbol, partitionerEnumSymbol), + OperationKind.SimpleAssignment); }); } - private static void AnalyzerObjectCreation(OperationAnalysisContext context) + private static void AnalyzerObjectCreation( + OperationAnalysisContext context, + INamedTypeSymbol kafkaPublicationSymbol, + INamedTypeSymbol partitionerEnumSymbol) { var operation = (IObjectCreationOperation)context.Operation; // Cheap rejection before allocating the visitor; most object creations // in a compilation are not KafkaPublications. - if (!KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(operation.Type)) + if (!KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(operation.Type, kafkaPublicationSymbol)) { return; } - var visitor = new KafkaPublicationPartitionerVisitor(); + var visitor = new KafkaPublicationPartitionerVisitor(kafkaPublicationSymbol, partitionerEnumSymbol); operation.Accept(visitor); if (!visitor.IsPartitionerAssigned) { - if (HasPartitionerAssignmentAfterConstruction(operation)) + if (HasPartitionerAssignmentAfterConstruction(operation) || + SetsPartitionerInConstructor(operation.Type, kafkaPublicationSymbol)) { - // The partitioner is set on the instance after construction; any - // discouraged value is reported by AnalyzeAssignment instead. + // The partitioner is set on the instance after construction or by + // the type's own constructor; any discouraged value is reported + // by AnalyzeAssignment instead. return; } @@ -133,33 +147,96 @@ private static void AnalyzerObjectCreation(OperationAnalysisContext context) } } + // A subclass can set the partitioner in its own constructor, e.g.: + // class OrdersPublication : KafkaPublication + // { + // public OrdersPublication() { Partitioner = Partitioner.Murmur2Random; } + // } + // Don't report BRT006 for such a type — an initializer added by the code fix + // would override the subclass's deliberate choice. Only constructors declared + // below KafkaPublication itself are considered; its own Partitioner default is + // exactly what BRT006 flags as implicit. + private static bool SetsPartitionerInConstructor( + ITypeSymbol type, + INamedTypeSymbol kafkaPublicationSymbol) + { + for (var current = type as INamedTypeSymbol; + current != null && !SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, kafkaPublicationSymbol); + current = current.BaseType) + { + foreach (var constructor in current.InstanceConstructors) + { + if (ConstructorAssignsPartitioner(constructor)) + { + return true; + } + } + } + + return false; + } + + // Syntactic check (analyzers must not call Compilation.GetSemanticModel, RS1030): + // an assignment to `Partitioner` or `this.Partitioner` in the constructor body. + // In a KafkaPublication subclass constructor an unqualified `Partitioner` can + // only bind to the inherited property or a local of the same name — the latter + // is contrived and accepted. + private static bool ConstructorAssignsPartitioner(IMethodSymbol constructor) + { + foreach (var syntaxReference in constructor.DeclaringSyntaxReferences) + { + var assignsPartitioner = syntaxReference.GetSyntax() + .DescendantNodes() + .OfType() + .Any(assignment => assignment.Left switch + { + IdentifierNameSyntax id => id.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty, + MemberAccessExpressionSyntax { Expression: ThisExpressionSyntax } memberAccess => + memberAccess.Name.Identifier.ValueText == BrighterAnalyzerGlobals.PartitionerProperty, + _ => false + }); + + if (assignsPartitioner) + { + return true; + } + } + + return false; + } + // Reports discouraged partitioner values assigned outside an object // initializer, e.g.: // var publication = new KafkaPublication(); // publication.Partitioner = Partitioner.Consistent; // The diagnostic is reported from the assignment's own callback so it stays // local to the analyzed operation. - private static void AnalyzeAssignment(OperationAnalysisContext context) + private static void AnalyzeAssignment( + OperationAnalysisContext context, + INamedTypeSymbol kafkaPublicationSymbol, + INamedTypeSymbol partitionerEnumSymbol) { var assignment = (ISimpleAssignmentOperation)context.Operation; - // Initializer assignments (new KafkaPublication { Partitioner = ... }) are - // handled by AnalyzerObjectCreation. Their parent is an - // IObjectOrCollectionInitializerOperation; IMemberInitializerOperation - // only appears in `with` expressions. - if (assignment.Parent is IObjectOrCollectionInitializerOperation or IMemberInitializerOperation) + // Assignments inside an object creation's initializer + // (new KafkaPublication { Partitioner = ... }) are handled by + // AnalyzerObjectCreation. Assignments in a nested member initializer + // (new Holder { Publication = { Partitioner = ... } }) — whose parent + // initializer hangs off an IMemberInitializerOperation, not a creation — + // ARE handled here. + if (assignment.Parent is IObjectOrCollectionInitializerOperation { Parent: not IMemberInitializerOperation }) { return; } if (assignment.Target is not IPropertyReferenceOperation propertyReference || propertyReference.Property.Name != BrighterAnalyzerGlobals.PartitionerProperty || - !KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(propertyReference.Property.ContainingType)) + !KafkaPublicationPartitionerVisitor.IsKafkaPublicationType(propertyReference.Property.ContainingType, kafkaPublicationSymbol)) { return; } - switch (KafkaPublicationPartitionerVisitor.GetPartitionerValueName(assignment.Value)) + switch (KafkaPublicationPartitionerVisitor.GetPartitionerValueName(assignment.Value, partitionerEnumSymbol)) { case BrighterAnalyzerGlobals.ConsistentRandomPartitionerValue: context.ReportDiagnostic(Diagnostic.Create( @@ -215,6 +292,9 @@ private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOpe .OfType() .Select(statement => statement.Operation) .OfType() + // Field/property targets are compared by symbol, not instance: an + // assignment through another object sharing the field (a.Pub vs b.Pub) + // would also match — an accepted, contrived edge case. .Any(assignment => assignment.Syntax.SpanStart > operation.Syntax.SpanStart && assignment.Target is IPropertyReferenceOperation propertyReference && diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index a61319cca7..3baa705bbb 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -24,15 +24,19 @@ THE SOFTWARE. */ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Operations; -using Paramore.Brighter.Analyzer.Visitors.Symbol; namespace Paramore.Brighter.Analyzer.Visitors.Operation; public class KafkaPublicationPartitionerVisitor : OperationWalker { - private static readonly ChildOfVisitor s_kafkaPublicationCheck = new( - BrighterAnalyzerGlobals.KafkaPublicationClassName, - BrighterAnalyzerGlobals.KafkaMessagingGatewayAssembly); + private readonly INamedTypeSymbol _kafkaPublicationSymbol; + private readonly INamedTypeSymbol _partitionerEnumSymbol; + + public KafkaPublicationPartitionerVisitor(INamedTypeSymbol kafkaPublicationSymbol, INamedTypeSymbol partitionerEnumSymbol) + { + _kafkaPublicationSymbol = kafkaPublicationSymbol; + _partitionerEnumSymbol = partitionerEnumSymbol; + } public bool IsPartitionerAssigned { get; private set; } public bool IsConsistentRandom { get; private set; } @@ -40,15 +44,9 @@ public class KafkaPublicationPartitionerVisitor : OperationWalker public string PublicationName { get; private set; } public Location PartitionerAssignmentLocation { get; private set; } - // Type can be null for erroneous code in the IDE; treat it as no match. - internal static bool IsKafkaPublicationType(ITypeSymbol type) - { - return type != null && type.Accept(s_kafkaPublicationCheck); - } - public override void VisitObjectCreation(IObjectCreationOperation operation) { - if (IsKafkaPublicationType(operation.Type)) + if (IsKafkaPublicationType(operation.Type, _kafkaPublicationSymbol)) { PublicationName = operation.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); @@ -68,12 +66,12 @@ public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) { if (operation.Target is IPropertyReferenceOperation propertyReference && propertyReference.Property.Name == BrighterAnalyzerGlobals.PartitionerProperty && - IsKafkaPublicationType(propertyReference.Property.ContainingType)) + IsKafkaPublicationType(propertyReference.Property.ContainingType, _kafkaPublicationSymbol)) { IsPartitionerAssigned = true; PartitionerAssignmentLocation = operation.Syntax.GetLocation(); - switch (GetPartitionerValueName(operation.Value)) + switch (GetPartitionerValueName(operation.Value, _partitionerEnumSymbol)) { case BrighterAnalyzerGlobals.ConsistentRandomPartitionerValue: IsConsistentRandom = true; @@ -87,7 +85,21 @@ public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) base.VisitSimpleAssignment(operation); } - internal static string GetPartitionerValueName(IOperation value) + // Type can be null for erroneous code in the IDE; treat it as no match. + internal static bool IsKafkaPublicationType(ITypeSymbol type, INamedTypeSymbol kafkaPublicationSymbol) + { + for (var current = type; current != null; current = current.BaseType) + { + if (SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, kafkaPublicationSymbol)) + { + return true; + } + } + + return false; + } + + internal static string GetPartitionerValueName(IOperation value, INamedTypeSymbol partitionerEnumSymbol) { // Unwrap an implicit conversion (e.g. enum widening) if present. if (value is IConversionOperation conversion) @@ -95,6 +107,12 @@ internal static string GetPartitionerValueName(IOperation value) value = conversion.Operand; } - return value is IFieldReferenceOperation fieldReference ? fieldReference.Field.Name : null; + // Only fields of the Kafka Partitioner enum itself count; a user field + // that merely shares a member name (e.g. `Defaults.Consistent`) must not + // be treated as the enum value. + return value is IFieldReferenceOperation fieldReference && + SymbolEqualityComparer.Default.Equals(fieldReference.Field.ContainingType, partitionerEnumSymbol) + ? fieldReference.Field.Name + : null; } } diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index 5b1b7ee732..d418a016e9 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -13,7 +13,7 @@ Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2 **Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. As with [BRT007](./BRT007.md) and [BRT008](./BRT008.md), existing publications that rely on the current partition assignment can safely ignore (or suppress) this warning instead of applying the fix. -The rule recognizes a `Partitioner` assignment made in the object initializer, or one made directly on the same local variable later in the same block. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. +The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block, and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. ### Example ```csharp diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT007.md b/src/Paramore.Brighter.Analyzer/docs/BRT007.md index 1dc4677ddb..cd999ccb99 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT007.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT007.md @@ -4,9 +4,7 @@ This rule detects a `KafkaPublication` whose `Partitioner` is set to `Partitioner.ConsistentRandom`. ## Why is this a warning? -`Murmur2Random` is preferred over `ConsistentRandom` for new `KafkaPublications`. Both hash the message key to select a partition, but `Murmur2Random` uses the MurmurHash2 algorithm, which spreads keys more evenly across partitions than the CRC32-based hash used by `ConsistentRandom`. - -An uneven hash concentrates a disproportionate share of keys onto a few partitions — the *hot partition* problem. Because each partition is served by a single consumer within a consumer group and a single broker as its leader, a hot partition becomes a throughput bottleneck: it lags and backs up while the remaining partitions sit under-used, so the topic can no longer scale across all of its partitions. `Murmur2Random`'s more uniform distribution keeps load balanced and avoids this, and it also matches the partitioning the standard Kafka clients use by default, so a given key lands on the partition other producers and consumers expect. +`Murmur2Random` is preferred over `ConsistentRandom` for new `KafkaPublications`. Both hash the message key to select a partition and spread keyless messages randomly, but they use different hash algorithms: `ConsistentRandom` uses CRC32, while `Murmur2Random` uses MurmurHash2 — the algorithm the standard Java Kafka producer uses. `Murmur2Random` is functionally equivalent to the Java producer's default partitioner, so a given key lands on the same partition for Brighter and for the standard Kafka clients. When other producers or consumers rely on key-based routing or ordering, that cross-client compatibility is what keeps a key on the partition the rest of the ecosystem expects. Existing publications that already rely on `ConsistentRandom` can safely ignore this warning to preserve their current partition assignment. diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT008.md b/src/Paramore.Brighter.Analyzer/docs/BRT008.md index b5ff09c92f..69d39d1e56 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT008.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT008.md @@ -4,9 +4,11 @@ This rule detects a `KafkaPublication` whose `Partitioner` is set to `Partitioner.Consistent`. ## Why is this a warning? -`Murmur2` is preferred over `Consistent` for new `KafkaPublications`. Both hash the message key to select a partition, but `Murmur2` uses the MurmurHash2 algorithm, which spreads keys more evenly across partitions than the CRC32-based hash used by `Consistent`. +`Murmur2` is preferred over `Consistent` for new `KafkaPublications`. Both hash the message key to select a partition, but `Murmur2` uses the MurmurHash2 algorithm, which spreads keys more evenly across partitions than the CRC32-based hash used by `Consistent`. Murmur2 is also the algorithm the standard Java Kafka producer uses for keyed messages, so a given key lands on the same partition for Brighter and for the standard Kafka clients — which matters when other producers or consumers rely on key-based routing or ordering. -An uneven hash concentrates a disproportionate share of keys onto a few partitions — the *hot partition* problem. Because each partition is served by a single consumer within a consumer group and a single broker as its leader, a hot partition becomes a throughput bottleneck: it lags and backs up while the remaining partitions sit under-used, so the topic can no longer scale across all of its partitions. `Murmur2`'s more uniform distribution keeps load balanced and avoids this, and it also matches the partitioning the standard Kafka clients use by default, so a given key lands on the partition other producers and consumers expect. +An uneven hash concentrates a disproportionate share of keys onto a few partitions — the *hot partition* problem. Because each partition is served by a single consumer within a consumer group and a single broker as its leader, a hot partition becomes a throughput bottleneck: it lags and backs up while the remaining partitions sit under-used, so the topic can no longer scale across all of its partitions. `Murmur2`'s more uniform distribution keeps load balanced and avoids this. + +Like `Consistent`, `Murmur2` pins messages with empty or NULL keys to a single partition. If the publication sends keyless messages, prefer `Partitioner.Murmur2Random` (see [BRT007](./BRT007.md)) instead, which spreads them across partitions. Existing publications that already rely on `Consistent` can safely ignore this warning to preserve their current partition assignment. diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseAnalyzerTest.cs index 7f3e4da868..dfdc8353e0 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseAnalyzerTest.cs @@ -1,24 +1,28 @@ - -using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; using Paramore.Brighter.Analyzer.Analyzers; -namespace Paramore.Brighter.Analyzer.Tests.Analyzers +namespace Paramore.Brighter.Analyzer.Tests.Analyzers; + +public abstract class BaseAnalyzerTest + where T : DiagnosticAnalyzer, new() { - public abstract class BaseAnalyzerTest where T : DiagnosticAnalyzer, new() + protected CSharpAnalyzerTest testContext; + + protected BaseAnalyzerTest() { - protected CSharpAnalyzerTest testContext; - protected BaseAnalyzerTest() + testContext = new CSharpAnalyzerTest { - testContext = new CSharpAnalyzerTest - { - ReferenceAssemblies = ReferenceAssemblies.Net.Net90 - }; - testContext.TestState.OutputKind = OutputKind.ConsoleApplication; - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location)); - testContext.CompilerDiagnostics = CompilerDiagnostics.None; - } + ReferenceAssemblies = ReferenceAssemblies.Net.Net90, + }; + + testContext.TestState.OutputKind = OutputKind.ConsoleApplication; + testContext.TestState.AdditionalReferences.Add( + MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location) + ); + + testContext.CompilerDiagnostics = CompilerDiagnostics.None; } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs index 8d75d095ef..3a7eb059b7 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/BaseKafkaAnalyzerTest.cs @@ -1,16 +1,19 @@ -using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Paramore.Brighter.Analyzer.Analyzers; -namespace Paramore.Brighter.Analyzer.Tests.Analyzers +namespace Paramore.Brighter.Analyzer.Tests.Analyzers; + +public abstract class BaseKafkaAnalyzerTest : BaseAnalyzerTest { - public abstract class BaseKafkaAnalyzerTest : BaseAnalyzerTest + protected BaseKafkaAnalyzerTest() { - protected BaseKafkaAnalyzerTest() - { - testContext.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.CompilerDiagnostics = CompilerDiagnostics.Errors; - } + testContext.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; + testContext.TestState.AdditionalReferences.Add( + MetadataReference.CreateFromFile( + typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location + ) + ); + testContext.CompilerDiagnostics = CompilerDiagnostics.Errors; } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index 84ee2d21ab..9df7758474 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -1,16 +1,17 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; -using Xunit; using Paramore.Brighter.Analyzer.Analyzers; +using Xunit; + +namespace Paramore.Brighter.Analyzer.Tests.Analyzers; -namespace Paramore.Brighter.Analyzer.Tests.Analyzers +public class KafkaPublicationPartitionerAnalyzerTest : BaseKafkaAnalyzerTest { - public class KafkaPublicationPartitionerAnalyzerTest : BaseKafkaAnalyzerTest + [Fact] + public async Task When_KafkaPublication_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() { - [Fact] - public async Task When_KafkaPublication_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -25,15 +26,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Generic_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Generic_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -54,15 +60,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Is_Created_With_ConsistentRandom_Should_Report_Warning() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Is_Created_With_ConsistentRandom_Should_Report_Warning() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -80,15 +91,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Is_Created_With_Consistent_Should_Report_Warning() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Consistent_Should_Report_Warning() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -106,15 +122,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Is_Created_With_Murmur2Random_Should_Not_Report() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Murmur2Random_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -132,14 +153,15 @@ public void Method() } } """; - - await testContext.RunAsync(); - } - [Fact] - public async Task When_KafkaPublication_Without_Partitioner_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() - { - testContext.TestCode = /* lang=c#-test */ """ + await testContext.RunAsync(); + } + + [Fact] + public async Task When_KafkaPublication_Without_Partitioner_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -159,15 +181,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_With_Consistent_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_With_Consistent_Is_Nested_In_Another_Object_Creation_Should_Report_Once_At_Publication() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -190,15 +217,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Is_Created_With_Random_Should_Not_Report() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Random_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -217,13 +249,14 @@ public void Method() } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Nested_Object_Has_Own_Partitioner_Property_Should_Still_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Nested_Object_Has_Own_Partitioner_Property_Should_Still_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -249,15 +282,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Set_After_Construction_Should_Not_Report() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Set_After_Construction_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -274,13 +312,14 @@ public void Method() } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Consistent_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Consistent_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -296,15 +335,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_ConsistentRandom_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_ConsistentRandom_Is_Set_After_Construction_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -320,15 +364,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Plain_Publication_Is_Created_Should_Not_Report() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Plain_Publication_Is_Created_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace ConsoleApplication1 @@ -343,13 +392,14 @@ public void Method() } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Subclass_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Subclass_Is_Created_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -368,15 +418,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("MyPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("MyPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Set_On_Field_After_Construction_Should_Not_Report() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Set_On_Field_After_Construction_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -395,13 +450,14 @@ public void Method() } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Set_Before_Construction_Should_Still_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Set_Before_Construction_Should_Still_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -419,15 +475,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Is_Created_With_Target_Typed_New_Without_Partitioner_Should_Report_Missing_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Is_Created_With_Target_Typed_New_Without_Partitioner_Should_Report_Missing_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -442,15 +503,20 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_KafkaPublication_Generic_Is_Created_With_ConsistentRandom_Should_Report_Warning() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_KafkaPublication_Generic_Is_Created_With_ConsistentRandom_Should_Report_Warning() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -474,9 +540,146 @@ public void Method() } } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Subclass_Sets_Partitioner_In_Constructor_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class OrdersPublication : KafkaPublication + { + public OrdersPublication() + { + Partitioner = Partitioner.Murmur2Random; + } + } + + class TypeName + { + public void Method() + { + var publication = new OrdersPublication(); + } + } +} +"""; + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Subclass_Sets_Consistent_In_Constructor_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class OrdersPublication : KafkaPublication + { + public OrdersPublication() + { + {|#0:Partitioner = Partitioner.Consistent|}; + } + } + + class TypeName + { + public void Method() + { + var publication = new OrdersPublication(); + } + } +} +"""; + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Value_Is_A_User_Field_Named_Like_The_Enum_Member_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + static class Defaults + { + public static readonly Partitioner Consistent = Partitioner.Murmur2; + } + + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Partitioner = Defaults.Consistent + }; + } + } +} +"""; + + await testContext.RunAsync(); + } - await testContext.RunAsync(); + [Fact] + public async Task When_Consistent_Is_Set_In_Nested_Member_Initializer_Should_Report_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class Holder + { + public KafkaPublication Publication { get; set; } = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; + } + + class TypeName + { + public void Method() + { + var holder = new Holder + { + Publication = { {|#0:Partitioner = Partitioner.Consistent|} } + }; } } } +"""; + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/PublicationRequestTypeAssignmentAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/PublicationRequestTypeAssignmentAnalyzerTest.cs index a8e904138a..978f26bfed 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/PublicationRequestTypeAssignmentAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/PublicationRequestTypeAssignmentAnalyzerTest.cs @@ -2,17 +2,16 @@ using Paramore.Brighter.Analyzer.Analyzers; using Paramore.Brighter.Analyzer.Tests.Analyzers; +namespace Paramore.Brighter.Analyzer.Test.Analyzers; -namespace Paramore.Brighter.Analyzer.Test.Analyzers +public class PublicationRequestTypeAssignmentAnalyzerTest + : BaseAnalyzerTest { - public class PublicationRequestTypeAssignmentAnalyzerTest : BaseAnalyzerTest + [Fact] + public async Task When_Initializing_Publication_WithOut_RequestType() { - - [Fact] - public async Task When_Initializing_Publication_WithOut_RequestType() - { - - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -24,14 +23,19 @@ public class PublicationTest : Publication } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(PublicationRequestTypeAssignmentAnalyzer.RequestTypeMissingRule).WithLocation(0).WithArguments("PublicationTest")); - await testContext.RunAsync(); - } + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(PublicationRequestTypeAssignmentAnalyzer.RequestTypeMissingRule) + .WithLocation(0) + .WithArguments("PublicationTest") + ); + await testContext.RunAsync(); + } - [Fact] - public async Task When_Initializing_Publication_With_Right_RequestType() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Initializing_Publication_With_Right_RequestType() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -51,13 +55,14 @@ public EventSample(Id id) : base(id) } } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Initializing_Publication_With_Wrong_RequestType() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Initializing_Publication_With_Wrong_RequestType() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -71,15 +76,20 @@ public class PublicationTest : Publication public class EventSample{} } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(PublicationRequestTypeAssignmentAnalyzer.WrongRequestTypeRule).WithLocation(0).WithArguments("EventSample")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(PublicationRequestTypeAssignmentAnalyzer.WrongRequestTypeRule) + .WithLocation(0) + .WithArguments("EventSample") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Initializing_Non_Publication_Type() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Initializing_Non_Publication_Type() + { + testContext.TestCode = /* lang=c#-test */ + """ using System.Collections.Generic; namespace TestNamespace { @@ -92,7 +102,6 @@ public void Method() } } """; - await testContext.RunAsync(); - } + await testContext.RunAsync(); } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/SubscriptionConstructorAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/SubscriptionConstructorAnalyzerTest.cs index 8d2bf4d8a2..4fdab08f56 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/SubscriptionConstructorAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/SubscriptionConstructorAnalyzerTest.cs @@ -2,15 +2,15 @@ using Paramore.Brighter.Analyzer.Analyzers; using Paramore.Brighter.Analyzer.Tests.Analyzers; -namespace Paramore.Brighter.Analyzer.Test.Analyzers +namespace Paramore.Brighter.Analyzer.Test.Analyzers; + +public class SubscriptionConstructorAnalyzerTest : BaseAnalyzerTest { - public class SubscriptionConstructorAnalyzerTest : BaseAnalyzerTest + [Fact] + public async Task When_Initializing_Subscription_With_MessagePump() { - [Fact] - public async Task When_Initializing_Subscription_With_MessagePump() - { - - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -25,14 +25,14 @@ public SubscriptionTest(SubscriptionName subscriptionName, ChannelName channelNa } """; - await testContext.RunAsync(); - } - - [Fact] - public async Task When_Initializing_Subscription_WithOut_MessagePump() - { + await testContext.RunAsync(); + } - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Initializing_Subscription_WithOut_MessagePump() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -47,14 +47,19 @@ public SubscriptionTest(SubscriptionName subscriptionName, ChannelName channelNa } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(SubscriptionConstructorAnalyzer.MessagePumpMissingRule).WithLocation(0).WithArguments("SubscriptionTest")); - await testContext.RunAsync(); - } - [Fact] - public async Task When_Initializing_SubscriptionNested_WithOut_MessagePump() - { + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(SubscriptionConstructorAnalyzer.MessagePumpMissingRule) + .WithLocation(0) + .WithArguments("SubscriptionTest") + ); + await testContext.RunAsync(); + } - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Initializing_SubscriptionNested_WithOut_MessagePump() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; namespace TestNamespace { @@ -75,9 +80,11 @@ public SubscriptionTestNested(SubscriptionName subscriptionName, ChannelName cha } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(SubscriptionConstructorAnalyzer.MessagePumpMissingRule).WithLocation(0).WithArguments("SubscriptionTestNested")); - await testContext.RunAsync(); - } - + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(SubscriptionConstructorAnalyzer.MessagePumpMissingRule) + .WithLocation(0) + .WithArguments("SubscriptionTestNested") + ); + await testContext.RunAsync(); } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/WrapAttributeAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/WrapAttributeAnalyzerTest.cs index 8ad50ccffd..a212215c7e 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/WrapAttributeAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/WrapAttributeAnalyzerTest.cs @@ -2,16 +2,15 @@ using Paramore.Brighter.Analyzer.Analyzers; using Paramore.Brighter.Analyzer.Tests.Analyzers; -namespace Paramore.Brighter.Analyzer.Test.Analyzers +namespace Paramore.Brighter.Analyzer.Test.Analyzers; + +public class WrapAttributeAnalyzerTest : BaseAnalyzerTest { - public class WrapAttributeAnalyzerTest: BaseAnalyzerTest + [Fact] + public async Task When_Adding_Attribute_To_MessageMapper() { - - [Fact] - public async Task When_Adding_Attribute_To_MessageMapper() - { - - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.Transforms.Attributes; namespace TestNamespace @@ -38,9 +37,16 @@ public class SampleEvent(Id id) : Event(id) } """; - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(WrapAttributeAnalyzer.WrapAttributeRule ).WithLocation(0).WithArguments("CompressAttribute")); - testContext.ExpectedDiagnostics.Add(new DiagnosticResult(WrapAttributeAnalyzer.UnWrapWithAttributeRule ).WithLocation(1).WithArguments("DecompressAttribute")); - await testContext.RunAsync(); - } + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(WrapAttributeAnalyzer.WrapAttributeRule) + .WithLocation(0) + .WithArguments("CompressAttribute") + ); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(WrapAttributeAnalyzer.UnWrapWithAttributeRule) + .WithLocation(1) + .WithArguments("DecompressAttribute") + ); + await testContext.RunAsync(); } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs index b3de695435..0fcd53bfc7 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/BaseCodeFixTest.cs @@ -1,28 +1,35 @@ - -using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; -namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes; + +public abstract class BaseCodeFixTest + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() { - public abstract class BaseCodeFixTest - where TAnalyzer : DiagnosticAnalyzer, new() - where TCodeFix : CodeFixProvider, new() - { - protected CSharpCodeFixTest testContext; + protected CSharpCodeFixTest testContext; - protected BaseCodeFixTest() + protected BaseCodeFixTest() + { + testContext = new CSharpCodeFixTest { - testContext = new CSharpCodeFixTest - { - ReferenceAssemblies = ReferenceAssemblies.Net.Net90 - }; - testContext.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location)); - testContext.TestState.AdditionalReferences.Add(MetadataReference.CreateFromFile(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location)); - testContext.CompilerDiagnostics = CompilerDiagnostics.Errors; - } + ReferenceAssemblies = ReferenceAssemblies.Net.Net90, + }; + + testContext.TestState.OutputKind = OutputKind.DynamicallyLinkedLibrary; + testContext.TestState.AdditionalReferences.Add( + MetadataReference.CreateFromFile(typeof(Publication).Assembly.Location) + ); + + testContext.TestState.AdditionalReferences.Add( + MetadataReference.CreateFromFile( + typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location + ) + ); + + testContext.CompilerDiagnostics = CompilerDiagnostics.Errors; } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index eb97a828b2..2dced6db0b 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -4,15 +4,16 @@ using Paramore.Brighter.Analyzer.CodeFixes; using Xunit; -namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes; + +public class MissingPartitionerCodeFixProviderTest + : BaseCodeFixTest { - public class MissingPartitionerCodeFixProviderTest - : BaseCodeFixTest + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Add_Murmur2Random() { - [Fact] - public async Task When_Partitioner_Is_Missing_Should_Add_Murmur2Random() - { - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -28,7 +29,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -44,16 +46,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Kafka_Using_Is_Missing_Should_Add_Fully_Qualified_Partitioner() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Kafka_Using_Is_Missing_Should_Add_Fully_Qualified_Partitioner() + { + testContext.TestCode = /* lang=c#-test */ + """ namespace ConsoleApplication1 { class TypeName @@ -66,7 +72,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ namespace ConsoleApplication1 { class TypeName @@ -79,16 +86,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Missing_Should_Append_To_Existing_Initializer() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_To_Existing_Initializer() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -108,7 +119,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -129,16 +141,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Missing_Should_Append_After_Trailing_Comment() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_After_Trailing_Comment() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -158,7 +174,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -179,16 +196,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Partitioner_Is_Missing_On_Target_Typed_New_Should_Add_Murmur2Random() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Partitioner_Is_Missing_On_Target_Typed_New_Should_Add_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -204,7 +225,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -223,10 +245,129 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule).WithLocation(0).WithArguments("KafkaPublication")); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_On_Single_Line_Initializer_Should_Append_On_Same_Line() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = {|#0:new KafkaPublication { Topic = new RoutingKey("x") }|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; - await testContext.RunAsync(); +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication { Topic = new RoutingKey("x"), Partitioner = Partitioner.Murmur2Random }; } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task FixAll_Should_Add_Partitioner_To_All_Publications() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = {|#0:new KafkaPublication()|}; + var second = {|#1:new KafkaPublication()|}; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var second = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.BatchFixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var second = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(1) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } +} diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs index d6dc1592a6..9dbb7d7db9 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/PartitionerValueCodeFixProviderTest.cs @@ -4,15 +4,16 @@ using Paramore.Brighter.Analyzer.CodeFixes; using Xunit; -namespace Paramore.Brighter.Analyzer.Tests.CodeFixes +namespace Paramore.Brighter.Analyzer.Tests.CodeFixes; + +public class PartitionerValueCodeFixProviderTest + : BaseCodeFixTest { - public class PartitionerValueCodeFixProviderTest - : BaseCodeFixTest + [Fact] + public async Task When_ConsistentRandom_Is_Used_Should_Offer_Murmur2Random() { - [Fact] - public async Task When_ConsistentRandom_Is_Used_Should_Offer_Murmur2Random() - { - testContext.TestCode = /* lang=c#-test */ """ + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -31,7 +32,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -50,16 +52,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Consistent_Is_Used_Should_Offer_Murmur2() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Consistent_Is_Used_Should_Offer_Murmur2() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -78,7 +84,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -97,16 +104,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Bare_Identifier_Via_Using_Static_Is_Used_Should_Offer_Murmur2Random() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Bare_Identifier_Via_Using_Static_Is_Used_Should_Offer_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; using static Paramore.Brighter.MessagingGateway.Kafka.Partitioner; @@ -126,7 +137,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; using static Paramore.Brighter.MessagingGateway.Kafka.Partitioner; @@ -146,16 +158,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentRandomPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Consistent_Is_Set_After_Construction_Should_Offer_Murmur2() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Consistent_Is_Set_After_Construction_Should_Offer_Murmur2() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -172,7 +188,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -189,16 +206,20 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); - await testContext.RunAsync(); - } + await testContext.RunAsync(); + } - [Fact] - public async Task When_Value_Is_Parenthesized_Should_Offer_Murmur2() - { - testContext.TestCode = /* lang=c#-test */ """ + [Fact] + public async Task When_Value_Is_Parenthesized_Should_Offer_Murmur2() + { + testContext.TestCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -217,7 +238,8 @@ public void Method() } """; - testContext.FixedCode = /* lang=c#-test */ """ + testContext.FixedCode = /* lang=c#-test */ + """ using Paramore.Brighter; using Paramore.Brighter.MessagingGateway.Kafka; @@ -236,10 +258,133 @@ public void Method() } """; - testContext.ExpectedDiagnostics.Add( - new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule).WithLocation(0)); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } - await testContext.RunAsync(); + [Fact] + public async Task When_Value_Is_A_Cast_Should_Not_Offer_Fix() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + {|#0:Partitioner = (Partitioner)Partitioner.Consistent|} + }; } } } +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task FixAll_Should_Replace_All_Discouraged_Values() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = new KafkaPublication + { + {|#0:Partitioner = Partitioner.Consistent|} + }; + var second = new KafkaPublication + { + {|#1:Partitioner = Partitioner.Consistent|} + }; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = new KafkaPublication + { + Partitioner = Partitioner.Murmur2 + }; + var second = new KafkaPublication + { + Partitioner = Partitioner.Murmur2 + }; + } + } +} +"""; + + testContext.BatchFixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var first = new KafkaPublication + { + Partitioner = Partitioner.Murmur2 + }; + var second = new KafkaPublication + { + Partitioner = Partitioner.Murmur2 + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(1) + ); + + await testContext.RunAsync(); + } +} From 057c345288fc45f45a1a70e880527f7d99dab492 Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 22:18:32 +0100 Subject: [PATCH 09/12] Apply code review --- flake.nix | 38 ------ samples/AsyncAPI/KafkaAsyncAPI/Program.cs | 2 +- .../GreetingsSender/Program.cs | 2 +- .../GreetingsSender/Program.cs | 2 +- .../TaskStatusSender/Program.cs | 6 +- .../GreetingsSender/Program.cs | 4 +- .../KafkaTaskQueue/GreetingsSender/Program.cs | 4 +- .../GreetingsSender/Program.cs | 4 +- .../MultiBus/GreetingsSender/Program.cs | 4 +- .../TransportMaker/ConfigureTransport.cs | 4 +- .../MissingPartitionerCodeFixProvider.cs | 51 ++++++-- .../PartitionerValueCodeFixProvider.cs | 2 +- .../KafkaPublicationPartitionerAnalyzer.cs | 49 +++++-- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 2 +- ...KafkaPublicationPartitionerAnalyzerTest.cs | 16 +-- .../MissingPartitionerCodeFixProviderTest.cs | 121 ++++++++++++++++-- 16 files changed, 214 insertions(+), 97 deletions(-) delete mode 100644 flake.nix diff --git a/flake.nix b/flake.nix deleted file mode 100644 index 1e74ce47c3..0000000000 --- a/flake.nix +++ /dev/null @@ -1,38 +0,0 @@ -{ - description = "A very basic flake"; - - inputs = { - nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable"; - }; - - outputs = - { - nixpkgs, - flake-utils, - ... - }: - flake-utils.lib.eachDefaultSystem ( - system: - let - # pkgs = nixpkgs.legacyPackages.${system}; - pkgs = import nixpkgs { inherit system; }; - - # Define the .NET SDK version you want to use - dotnetSdk = pkgs.dotnetCorePackages.sdk_10_0-bin; - in - { - devShells.default = pkgs.mkShell { - packages = [ - dotnetSdk - - pkgs.netcoredbg # Debugger for .NET Core - # pkgs.roslyn-ls # LSP for VS Code / Emacs / Vim - ]; - }; - - # Environment variables - # 1. Essential: Tell dotnet tools where to find the SDK - DOTNET_ROOT = "${dotnetSdk}"; - } - ); -} diff --git a/samples/AsyncAPI/KafkaAsyncAPI/Program.cs b/samples/AsyncAPI/KafkaAsyncAPI/Program.cs index df3cbd0e47..450224bd38 100644 --- a/samples/AsyncAPI/KafkaAsyncAPI/Program.cs +++ b/samples/AsyncAPI/KafkaAsyncAPI/Program.cs @@ -82,7 +82,7 @@ THE SOFTWARE. */ { Topic = new RoutingKey("order.created"), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs index 8a76e7fcab..1b60daff81 100644 --- a/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaDeferOnError/GreetingsSender/Program.cs @@ -60,7 +60,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs index e7a33c7c4c..3ed5f39bf6 100644 --- a/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaDontAckOnError/GreetingsSender/Program.cs @@ -60,7 +60,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs b/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs index f84d49ea3a..bacce93f23 100644 --- a/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs +++ b/samples/TaskQueue/KafkaDynamicEventStream/TaskStatusSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2025 Ian Cooper @@ -53,7 +53,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("task.update"), Type = new CloudEventsType("io.goparamore.task.created"), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, @@ -65,7 +65,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("task.update"), Type = new CloudEventsType("io.goparamore.task.updated"), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs index d659193e5d..8814c3416f 100644 --- a/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaSchemaRegistry/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -63,7 +63,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs index 03ac66e83b..0aa04092f8 100644 --- a/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaTaskQueue/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -81,7 +81,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs b/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs index 73b538a226..badb8dd4fc 100644 --- a/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs +++ b/samples/TaskQueue/KafkaTaskQueueWithDLQ/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -97,7 +97,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs b/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs index e5dbefb4ad..6bc7c81333 100644 --- a/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs +++ b/samples/TaskQueue/MultiBus/GreetingsSender/Program.cs @@ -1,4 +1,4 @@ -#region Licence +#region Licence /* The MIT License (MIT) Copyright © 2017 Wayne Hunsley @@ -73,7 +73,7 @@ THE SOFTWARE. */ Topic = new RoutingKey("greeting.event"), RequestType = typeof(GreetingEvent), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Partitioner.Murmur2Random, NumPartitions = 3, MessageSendMaxRetries = 3, diff --git a/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs b/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs index acdd631973..bda408018f 100644 --- a/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs +++ b/samples/WebAPI/WebAPI_Common/TransportMaker/ConfigureTransport.cs @@ -1,4 +1,4 @@ -using Confluent.Kafka; +using Confluent.Kafka; using Confluent.SchemaRegistry; using Microsoft.Extensions.DependencyInjection; using Paramore.Brighter; @@ -98,7 +98,7 @@ public static IAmAProducerRegistry GetKafkaProducerRegistry() where T: class, Topic = new RoutingKey(typeof(T).Name), RequestType = typeof(T), // Murmur2Random is recommended: its MurmurHash2 hash spreads keys evenly across - // partitions, avoiding hot partitions, and matches the standard Kafka client default + // partitions, avoiding hot partitions Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random, MessageSendMaxRetries = 3, MessageTimeoutMs = 1000, diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index af88d54a0e..663d8c6311 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -55,7 +55,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) foreach (var diagnostic in context.Diagnostics) { var objectCreation = root.FindNode(diagnostic.Location.SourceSpan) - .DescendantNodesAndSelf() + .AncestorsAndSelf() .OfType() .FirstOrDefault(); @@ -66,7 +66,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( - title: $"Set 'Partitioner' to 'Partitioner.{BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue}'", + title: $"Set 'Partitioner' to 'Partitioner.{BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue}' (re-partitions the topic)", createChangedDocument: ct => AddPartitionerAsync(context.Document, objectCreation, ct), equivalenceKey: nameof(MissingPartitionerCodeFixProvider)), diagnostic); @@ -89,11 +89,23 @@ private static async Task AddPartitionerAsync( SyntaxFactory.IdentifierName(BrighterAnalyzerGlobals.Murmur2RandomPartitionerValue)) .WithAdditionalAnnotations(Simplifier.Annotation)); - var initializer = objectCreation.Initializer == null - ? SyntaxFactory.InitializerExpression( + InitializerExpressionSyntax initializer; + if (objectCreation.Initializer == null) + { + initializer = SyntaxFactory.InitializerExpression( SyntaxKind.ObjectInitializerExpression, - SyntaxFactory.SingletonSeparatedList(assignment)) - : AddInitializerExpression(objectCreation.Initializer, assignment); + SyntaxFactory.SingletonSeparatedList(assignment)); + } + else if (objectCreation.Initializer.Expressions.Count == 0) + { + // new KafkaPublication { } — keep the existing (empty) braces and trivia. + initializer = objectCreation.Initializer.WithExpressions( + SyntaxFactory.SingletonSeparatedList(assignment)); + } + else + { + initializer = AddInitializerExpression(objectCreation.Initializer, assignment); + } var newObjectCreation = objectCreation .WithInitializer(initializer) @@ -118,7 +130,24 @@ private static InitializerExpressionSyntax AddInitializerExpression( // and the newline before the closing brace moves behind the new expression. // Comments stay with the expression they document: anything before the final // newline (e.g. "// one per shard") becomes the separator's trailing trivia. - var lastExpression = initializer.Expressions.Last(); + var expressions = initializer.Expressions; + + // Fold a trailing comma ("{ A = 1, B = 2, }") away first: its trivia + // (typically the newline before the closing brace) moves onto the last + // expression so the branches below see a normal list. + var nodesAndTokens = expressions.GetWithSeparators(); + if (nodesAndTokens.Count > 0 && nodesAndTokens[nodesAndTokens.Count - 1].IsToken) + { + var trailingSeparator = nodesAndTokens[nodesAndTokens.Count - 1].AsToken(); + nodesAndTokens = nodesAndTokens.RemoveAt(nodesAndTokens.Count - 1); + nodesAndTokens = nodesAndTokens.Replace( + nodesAndTokens[nodesAndTokens.Count - 1], + ((ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode()) + .WithTrailingTrivia(trailingSeparator.TrailingTrivia)); + expressions = SyntaxFactory.SeparatedList(nodesAndTokens); + } + + var lastExpression = expressions.Last(); var trailingTrivia = lastExpression.GetTrailingTrivia(); var beforeEndOfLine = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); @@ -142,21 +171,21 @@ private static InitializerExpressionSyntax AddInitializerExpression( else { separatorTrailingTrivia = SyntaxFactory.TriviaList(beforeEndOfLine); - var leadingTrivia = initializer.Expressions.Count > 1 - ? initializer.Expressions.GetSeparator(initializer.Expressions.Count - 2).TrailingTrivia + var leadingTrivia = expressions.Count > 1 + ? expressions.GetSeparator(expressions.Count - 2).TrailingTrivia : initializer.OpenBraceToken.TrailingTrivia; newExpression = expression .WithLeadingTrivia(leadingTrivia) .WithTrailingTrivia(fromEndOfLine); } - var expressions = initializer.Expressions + expressions = expressions .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) .Add(newExpression); if (separatorTrailingTrivia.Count > 0) { - var nodesAndTokens = expressions.GetWithSeparators(); + nodesAndTokens = expressions.GetWithSeparators(); var separator = nodesAndTokens[nodesAndTokens.Count - 2].AsToken().WithTrailingTrivia(separatorTrailingTrivia); nodesAndTokens = nodesAndTokens.Replace(nodesAndTokens[nodesAndTokens.Count - 2], separator); expressions = SyntaxFactory.SeparatedList(nodesAndTokens); diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs index 3da1fa92c5..44e3e96b83 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/PartitionerValueCodeFixProvider.cs @@ -78,7 +78,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( - title: $"Use 'Partitioner.{target}'", + title: $"Use 'Partitioner.{target}' (re-partitions the topic)", createChangedDocument: ct => ReplacePartitionerValueAsync(context.Document, assignment, target, ct), equivalenceKey: $"{nameof(PartitionerValueCodeFixProvider)}:{target}"), diagnostic); diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 3a4bed2423..28e69d04af 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -23,8 +23,10 @@ THE SOFTWARE. */ #endregion +using System.Collections.Concurrent; using System.Collections.Immutable; using System.Linq; +using System.Threading; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; @@ -91,8 +93,12 @@ public override void Initialize(AnalysisContext context) return; } + // Memoises the constructor inspection per type, so a subclass + // instantiated in many places is only walked once per compilation. + var constructorCheckCache = new ConcurrentDictionary(SymbolEqualityComparer.Default); + compilationContext.RegisterOperationAction( - operationContext => AnalyzerObjectCreation(operationContext, kafkaPublicationSymbol, partitionerEnumSymbol), + operationContext => AnalyzerObjectCreation(operationContext, kafkaPublicationSymbol, partitionerEnumSymbol, constructorCheckCache), OperationKind.ObjectCreation); compilationContext.RegisterOperationAction( operationContext => AnalyzeAssignment(operationContext, kafkaPublicationSymbol, partitionerEnumSymbol), @@ -103,7 +109,8 @@ public override void Initialize(AnalysisContext context) private static void AnalyzerObjectCreation( OperationAnalysisContext context, INamedTypeSymbol kafkaPublicationSymbol, - INamedTypeSymbol partitionerEnumSymbol) + INamedTypeSymbol partitionerEnumSymbol, + ConcurrentDictionary constructorCheckCache) { var operation = (IObjectCreationOperation)context.Operation; @@ -120,7 +127,7 @@ private static void AnalyzerObjectCreation( if (!visitor.IsPartitionerAssigned) { if (HasPartitionerAssignmentAfterConstruction(operation) || - SetsPartitionerInConstructor(operation.Type, kafkaPublicationSymbol)) + SetsPartitionerInConstructor(operation.Type, kafkaPublicationSymbol, constructorCheckCache, context.CancellationToken)) { // The partitioner is set on the instance after construction or by // the type's own constructor; any discouraged value is reported @@ -130,7 +137,7 @@ private static void AnalyzerObjectCreation( context.ReportDiagnostic(Diagnostic.Create( MissingPartitionerRule, - context.Operation.Syntax.GetLocation(), + GetCreationLocation(operation.Syntax), visitor.PublicationName)); } else if (visitor.IsConsistentRandom) @@ -147,6 +154,19 @@ private static void AnalyzerObjectCreation( } } + // Report on the type name (or the `new` keyword for a target-typed new) + // rather than the whole creation, so a large initializer isn't squiggled + // in full. + private static Location GetCreationLocation(SyntaxNode creationSyntax) + { + return creationSyntax switch + { + ObjectCreationExpressionSyntax objectCreation => objectCreation.Type.GetLocation(), + ImplicitObjectCreationExpressionSyntax implicitCreation => implicitCreation.NewKeyword.GetLocation(), + _ => creationSyntax.GetLocation() + }; + } + // A subclass can set the partitioner in its own constructor, e.g.: // class OrdersPublication : KafkaPublication // { @@ -158,18 +178,23 @@ private static void AnalyzerObjectCreation( // exactly what BRT006 flags as implicit. private static bool SetsPartitionerInConstructor( ITypeSymbol type, - INamedTypeSymbol kafkaPublicationSymbol) + INamedTypeSymbol kafkaPublicationSymbol, + ConcurrentDictionary constructorCheckCache, + CancellationToken cancellationToken) { for (var current = type as INamedTypeSymbol; current != null && !SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, kafkaPublicationSymbol); current = current.BaseType) { - foreach (var constructor in current.InstanceConstructors) + if (!constructorCheckCache.TryGetValue(current, out var assigns)) { - if (ConstructorAssignsPartitioner(constructor)) - { - return true; - } + assigns = current.InstanceConstructors.Any(constructor => ConstructorAssignsPartitioner(constructor, cancellationToken)); + constructorCheckCache[current] = assigns; + } + + if (assigns) + { + return true; } } @@ -181,11 +206,11 @@ private static bool SetsPartitionerInConstructor( // In a KafkaPublication subclass constructor an unqualified `Partitioner` can // only bind to the inherited property or a local of the same name — the latter // is contrived and accepted. - private static bool ConstructorAssignsPartitioner(IMethodSymbol constructor) + private static bool ConstructorAssignsPartitioner(IMethodSymbol constructor, CancellationToken cancellationToken) { foreach (var syntaxReference in constructor.DeclaringSyntaxReferences) { - var assignsPartitioner = syntaxReference.GetSyntax() + var assignsPartitioner = syntaxReference.GetSyntax(cancellationToken) .DescendantNodes() .OfType() .Any(assignment => assignment.Left switch diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index d418a016e9..873d9182ff 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -13,7 +13,7 @@ Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2 **Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. As with [BRT007](./BRT007.md) and [BRT008](./BRT008.md), existing publications that rely on the current partition assignment can safely ignore (or suppress) this warning instead of applying the fix. -The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block, and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. +The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block, and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. Subclass constructors can only be inspected when the subclass is declared in source — a subclass from a referenced assembly that sets `Partitioner` in its constructor will still be flagged; suppress it there too. ### Example ```csharp diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index 9df7758474..4a7cb81e3f 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -21,7 +21,7 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication()|}; + var publication = new {|#0:KafkaPublication|}(); } } } @@ -55,7 +55,7 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication()|}; + var publication = new {|#0:KafkaPublication|}(); } } } @@ -176,7 +176,7 @@ class TypeName { public void Method() { - var holder = new Holder({|#0:new KafkaPublication()|}); + var holder = new Holder(new {|#0:KafkaPublication|}()); } } } @@ -271,13 +271,13 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new {|#0:KafkaPublication|} { DefaultHeaders = new System.Collections.Generic.Dictionary { ["key"] = new Config { Partitioner = 3 } } - }|}; + }; } } } @@ -413,7 +413,7 @@ class TypeName { public void Method() { - var publication = {|#0:new MyPublication()|}; + var publication = new {|#0:MyPublication|}(); } } } @@ -470,7 +470,7 @@ class TypeName public void Method() { _publication.Partitioner = Partitioner.Murmur2Random; - _publication = {|#0:new KafkaPublication()|}; + _publication = new {|#0:KafkaPublication|}(); } } } @@ -498,7 +498,7 @@ class TypeName { public void Method() { - KafkaPublication publication = {|#0:new()|}; + KafkaPublication publication = {|#0:new|}(); } } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index 2dced6db0b..6ebb2df512 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -23,7 +23,7 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication()|}; + var publication = new {|#0:KafkaPublication|}(); } } } @@ -66,7 +66,7 @@ class TypeName { public void Method() { - var publication = {|#0:new Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication()|}; + var publication = new {|#0:Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication|}(); } } } @@ -109,11 +109,11 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new {|#0:KafkaPublication|} { Topic = new RoutingKey("x"), NumPartitions = 3 - }|}; + }; } } } @@ -164,11 +164,11 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication + var publication = new {|#0:KafkaPublication|} { Topic = new RoutingKey("x"), NumPartitions = 3 // one per shard - }|}; + }; } } } @@ -219,7 +219,7 @@ class TypeName { public void Method() { - KafkaPublication publication = {|#0:new()|}; + KafkaPublication publication = {|#0:new|}(); } } } @@ -268,7 +268,7 @@ class TypeName { public void Method() { - var publication = {|#0:new KafkaPublication { Topic = new RoutingKey("x") }|}; + var publication = new {|#0:KafkaPublication|} { Topic = new RoutingKey("x") }; } } } @@ -314,8 +314,8 @@ class TypeName { public void Method() { - var first = {|#0:new KafkaPublication()|}; - var second = {|#1:new KafkaPublication()|}; + var first = new {|#0:KafkaPublication|}(); + var second = new {|#1:KafkaPublication|}(); } } } @@ -370,4 +370,105 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Partitioner_Is_Missing_On_Empty_Initializer_Should_Add_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new {|#0:KafkaPublication|} { }; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_After_Trailing_Comma() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new {|#0:KafkaPublication|} + { + Topic = new RoutingKey("x"), + NumPartitions = 3, + }; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication + { + Topic = new RoutingKey("x"), + NumPartitions = 3, + Partitioner = Partitioner.Murmur2Random + }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } } From a1f7221fb4a2dff065562bdbc6da14a7fb355beb Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 22:23:28 +0100 Subject: [PATCH 10/12] Add more tests --- .../KafkaPublicationPartitionerAnalyzer.cs | 1 + ...KafkaPublicationPartitionerAnalyzerTest.cs | 81 ++++++++++++++ .../MissingPartitionerCodeFixProviderTest.cs | 104 ++++++++++++++++++ 3 files changed, 186 insertions(+) diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 28e69d04af..2d2109a5b7 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -291,6 +291,7 @@ private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOpe ISimpleAssignmentOperation { Target: ILocalReferenceOperation localReference } => localReference.Local, ISimpleAssignmentOperation { Target: IFieldReferenceOperation fieldReference } => fieldReference.Field, ISimpleAssignmentOperation { Target: IPropertyReferenceOperation propertyReference } => propertyReference.Property, + ISimpleAssignmentOperation { Target: IParameterReferenceOperation parameterReference } => parameterReference.Parameter, _ => null }; diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index 4a7cb81e3f..d527151576 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -682,4 +682,85 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Partitioner_Is_Set_On_Parameter_After_Construction_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method(KafkaPublication publication) + { + publication = new KafkaPublication(); + publication.Partitioner = Partitioner.Murmur2Random; + } + } +} +"""; + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Set_On_Property_After_Construction_Should_Not_Report() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + private KafkaPublication Publication { get; set; } + + public void Method() + { + Publication = new KafkaPublication(); + Publication.Partitioner = Partitioner.Murmur2Random; + } + } +} +"""; + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Target_Typed_New_Is_Created_With_Consistent_Should_Report_Warning() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + KafkaPublication publication = new() + { + {|#0:Partitioner = Partitioner.Consistent|} + }; + } + } +} +"""; + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } } diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index 6ebb2df512..38d0527163 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -471,4 +471,108 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Partitioner_Is_Missing_Should_Append_After_Single_Expression_Trailing_Comma() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new {|#0:KafkaPublication|} { Topic = new RoutingKey("x"), }; + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication { Topic = new RoutingKey("x"), Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } + + [Fact] + public async Task When_Partitioner_Is_Missing_On_Generic_Should_Add_Murmur2Random() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class MyRequest : IRequest + { + public Id Id { get; set; } + public Id? CorrelationId { get; set; } + } + + class TypeName + { + public void Method() + { + var publication = new {|#0:KafkaPublication|}(); + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class MyRequest : IRequest + { + public Id Id { get; set; } + public Id? CorrelationId { get; set; } + } + + class TypeName + { + public void Method() + { + var publication = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } } From 51031c4b7cd15b55ce05edd7373de86373500253 Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Sun, 26 Jul 2026 23:43:31 +0100 Subject: [PATCH 11/12] Apply code revew --- .../MissingPartitionerCodeFixProvider.cs | 71 +++++++++---------- .../Paramore.Brighter.Analyzer.Package.csproj | 4 ++ ... Paramore.Brighter.Analyzer.Package.props} | 0 .../configuration/default.editorconfig | 7 +- .../configuration/none.editorconfig | 7 +- .../KafkaPublicationPartitionerAnalyzer.cs | 27 +++---- .../BrighterAnalyzerGlobals.cs | 5 -- .../Paramore.Brighter.Analyzer.csproj | 3 +- .../KafkaPublicationPartitionerVisitor.cs | 10 +-- .../Operation/RequestTypeAssignmentVisitor.cs | 8 +-- .../SubscriptionConstructorVisitor.cs | 6 +- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 2 +- ...KafkaPublicationPartitionerAnalyzerTest.cs | 32 +++++++++ 13 files changed, 105 insertions(+), 77 deletions(-) rename src/Paramore.Brighter.Analyzer.Package/build/{Paramore.Brighter.Analyzer.props => Paramore.Brighter.Analyzer.Package.props} (100%) diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index 663d8c6311..e27c61c6e3 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -123,75 +123,68 @@ private static InitializerExpressionSyntax AddInitializerExpression( InitializerExpressionSyntax initializer, ExpressionSyntax expression) { - // InitializerExpressionSyntax.AddExpressions inserts the separator comma right - // after the last expression but before its trailing trivia, so the comma ends - // up on the wrong line. Rewire the trivia by hand: the newline + indent that - // follows an existing separator (or the open brace) leads the new expression, - // and the newline before the closing brace moves behind the new expression. - // Comments stay with the expression they document: anything before the final - // newline (e.g. "// one per shard") becomes the separator's trailing trivia. - var expressions = initializer.Expressions; - - // Fold a trailing comma ("{ A = 1, B = 2, }") away first: its trivia - // (typically the newline before the closing brace) moves onto the last - // expression so the branches below see a normal list. - var nodesAndTokens = expressions.GetWithSeparators(); + // Build the new expression list by hand: AddExpressions would insert the + // separator comma right after the last expression but before its trailing + // trivia, so the comma lands on the wrong line and any trailing comment + // (e.g. "// one per shard") would move onto the new expression. + var nodesAndTokens = initializer.Expressions.GetWithSeparators(); + + // A trailing comma ("{ A = 1, B = 2, }") carries the closing-brace trivia: + // drop the comma and move its trivia onto the last expression. if (nodesAndTokens.Count > 0 && nodesAndTokens[nodesAndTokens.Count - 1].IsToken) { - var trailingSeparator = nodesAndTokens[nodesAndTokens.Count - 1].AsToken(); + var trailingComma = nodesAndTokens[nodesAndTokens.Count - 1].AsToken(); nodesAndTokens = nodesAndTokens.RemoveAt(nodesAndTokens.Count - 1); nodesAndTokens = nodesAndTokens.Replace( nodesAndTokens[nodesAndTokens.Count - 1], ((ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode()) - .WithTrailingTrivia(trailingSeparator.TrailingTrivia)); - expressions = SyntaxFactory.SeparatedList(nodesAndTokens); + .WithTrailingTrivia(trailingComma.TrailingTrivia)); } - var lastExpression = expressions.Last(); + var lastExpression = (ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode(); var trailingTrivia = lastExpression.GetTrailingTrivia(); - var beforeEndOfLine = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); var fromEndOfLine = trailingTrivia.SkipWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); - SyntaxTriviaList separatorTrailingTrivia; + var separator = SyntaxFactory.Token(SyntaxKind.CommaToken); ExpressionSyntax newExpression; if (fromEndOfLine.Count == 0) { // Single-line initializer ("{ Topic = x }"): keep it on one line, // with single spaces around the new expression. - separatorTrailingTrivia = beforeEndOfLine.Any(IsComment) - ? SyntaxFactory.TriviaList(beforeEndOfLine) - : default; + var hasComment = beforeEndOfLine.Any(IsComment); + if (hasComment) + { + separator = separator.WithTrailingTrivia(beforeEndOfLine); + } + newExpression = expression .WithLeadingTrivia(SyntaxFactory.TriviaList(SyntaxFactory.Space)) - .WithTrailingTrivia(separatorTrailingTrivia.Count > 0 + .WithTrailingTrivia(hasComment ? SyntaxFactory.TriviaList(SyntaxFactory.Space) : SyntaxFactory.TriviaList(beforeEndOfLine)); } else { - separatorTrailingTrivia = SyntaxFactory.TriviaList(beforeEndOfLine); - var leadingTrivia = expressions.Count > 1 - ? expressions.GetSeparator(expressions.Count - 2).TrailingTrivia + // Multi-line: comments stay on their line attached to the comma, the + // newline + indent that follows an existing separator (or the open + // brace) leads the new expression, and the newline before the closing + // brace moves behind it. + separator = separator.WithTrailingTrivia(beforeEndOfLine); + var leadingTrivia = nodesAndTokens.Count > 1 + ? nodesAndTokens[nodesAndTokens.Count - 2].AsToken().TrailingTrivia : initializer.OpenBraceToken.TrailingTrivia; newExpression = expression .WithLeadingTrivia(leadingTrivia) .WithTrailingTrivia(fromEndOfLine); } - expressions = expressions - .Replace(lastExpression, lastExpression.WithoutTrailingTrivia()) - .Add(newExpression); - - if (separatorTrailingTrivia.Count > 0) - { - nodesAndTokens = expressions.GetWithSeparators(); - var separator = nodesAndTokens[nodesAndTokens.Count - 2].AsToken().WithTrailingTrivia(separatorTrailingTrivia); - nodesAndTokens = nodesAndTokens.Replace(nodesAndTokens[nodesAndTokens.Count - 2], separator); - expressions = SyntaxFactory.SeparatedList(nodesAndTokens); - } - - return initializer.WithExpressions(expressions); + return initializer.WithExpressions( + SyntaxFactory.SeparatedList( + nodesAndTokens + .Replace(nodesAndTokens[nodesAndTokens.Count - 1], lastExpression.WithoutTrailingTrivia()) + .Add(separator) + .Add(newExpression))); } private static bool IsComment(SyntaxTrivia trivia) diff --git a/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj b/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj index e3b77e159f..a36fab7d16 100644 --- a/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj +++ b/src/Paramore.Brighter.Analyzer.Package/Paramore.Brighter.Analyzer.Package.csproj @@ -25,6 +25,10 @@ + + + diff --git a/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.props b/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props similarity index 100% rename from src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.props rename to src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props diff --git a/src/Paramore.Brighter.Analyzer.Package/configuration/default.editorconfig b/src/Paramore.Brighter.Analyzer.Package/configuration/default.editorconfig index c4458e30f1..609f507acb 100644 --- a/src/Paramore.Brighter.Analyzer.Package/configuration/default.editorconfig +++ b/src/Paramore.Brighter.Analyzer.Package/configuration/default.editorconfig @@ -1,5 +1,10 @@ +is_global = true + dotnet_diagnostic.BRT001.severity = warning dotnet_diagnostic.BRT002.severity = warning dotnet_diagnostic.BRT003.severity = warning dotnet_diagnostic.BRT004.severity = warning -dotnet_diagnostic.BRT005.severity = warning \ No newline at end of file +dotnet_diagnostic.BRT005.severity = warning +dotnet_diagnostic.BRT006.severity = warning +dotnet_diagnostic.BRT007.severity = warning +dotnet_diagnostic.BRT008.severity = warning diff --git a/src/Paramore.Brighter.Analyzer.Package/configuration/none.editorconfig b/src/Paramore.Brighter.Analyzer.Package/configuration/none.editorconfig index aef953fc0e..0b5d80477c 100644 --- a/src/Paramore.Brighter.Analyzer.Package/configuration/none.editorconfig +++ b/src/Paramore.Brighter.Analyzer.Package/configuration/none.editorconfig @@ -1,5 +1,10 @@ +is_global = true + dotnet_diagnostic.BRT001.severity = none dotnet_diagnostic.BRT002.severity = none dotnet_diagnostic.BRT003.severity = none dotnet_diagnostic.BRT004.severity = none -dotnet_diagnostic.BRT005.severity = none \ No newline at end of file +dotnet_diagnostic.BRT005.severity = none +dotnet_diagnostic.BRT006.severity = none +dotnet_diagnostic.BRT007.severity = none +dotnet_diagnostic.BRT008.severity = none diff --git a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs index 2d2109a5b7..003949cd5f 100644 --- a/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs +++ b/src/Paramore.Brighter.Analyzer/Analyzers/KafkaPublicationPartitionerAnalyzer.cs @@ -177,7 +177,7 @@ private static Location GetCreationLocation(SyntaxNode creationSyntax) // below KafkaPublication itself are considered; its own Partitioner default is // exactly what BRT006 flags as implicit. private static bool SetsPartitionerInConstructor( - ITypeSymbol type, + ITypeSymbol? type, INamedTypeSymbol kafkaPublicationSymbol, ConcurrentDictionary constructorCheckCache, CancellationToken cancellationToken) @@ -186,11 +186,9 @@ private static bool SetsPartitionerInConstructor( current != null && !SymbolEqualityComparer.Default.Equals(current.OriginalDefinition, kafkaPublicationSymbol); current = current.BaseType) { - if (!constructorCheckCache.TryGetValue(current, out var assigns)) - { - assigns = current.InstanceConstructors.Any(constructor => ConstructorAssignsPartitioner(constructor, cancellationToken)); - constructorCheckCache[current] = assigns; - } + var assigns = constructorCheckCache.GetOrAdd( + current, + symbol => symbol.InstanceConstructors.Any(constructor => ConstructorAssignsPartitioner(constructor, cancellationToken))); if (assigns) { @@ -280,12 +278,12 @@ private static void AnalyzeAssignment( // later in the same block, e.g.: // var publication = new KafkaPublication(); // publication.Partitioner = Partitioner.Murmur2Random; - // Works for locals, fields, properties and parameters. Assignments made - // before the construction, or elsewhere (helper methods, other blocks), - // are not tracked. + // Works for locals, fields, properties and parameters, and sees assignments + // inside nested blocks (if/else/loops). Assignments made before the + // construction, or elsewhere (helper methods, other blocks), are not tracked. private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOperation operation) { - ISymbol symbol = operation.Parent switch + ISymbol? symbol = operation.Parent switch { IVariableInitializerOperation { Parent: IVariableDeclaratorOperation declarator } => declarator.Symbol, ISimpleAssignmentOperation { Target: ILocalReferenceOperation localReference } => localReference.Local, @@ -300,9 +298,6 @@ private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOpe return false; } - // Only the nearest enclosing block is searched; an assignment inside a - // nested block (e.g. an if) still triggers BRT006 — a documented - // limitation (see BRT006.md). var ancestor = operation.Parent; while (ancestor != null && ancestor is not IBlockOperation) { @@ -314,9 +309,7 @@ private static bool HasPartitionerAssignmentAfterConstruction(IObjectCreationOpe return false; } - return block.Operations - .OfType() - .Select(statement => statement.Operation) + return block.Descendants() .OfType() // Field/property targets are compared by symbol, not instance: an // assignment through another object sharing the field (a.Pub vs b.Pub) @@ -328,7 +321,7 @@ assignment.Target is IPropertyReferenceOperation propertyReference && IsReferenceTo(propertyReference.Instance, symbol)); } - private static bool IsReferenceTo(IOperation instance, ISymbol symbol) + private static bool IsReferenceTo(IOperation? instance, ISymbol symbol) { return instance switch { diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index 559316488d..331b4fadf2 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -29,11 +29,6 @@ public class BrighterAnalyzerGlobals public const string PublicationClassName = "Publication"; public const string KafkaPublicationClassName = "KafkaPublication"; public const string BrighterAssembly = "Paramore.Brighter"; - public const string KafkaMessagingGatewayAssembly = "Paramore.Brighter.MessagingGateway.Kafka"; - - // The Kafka namespace happens to equal the assembly name today; keep them as - // separate constants so renaming the assembly can't silently break code that - // needs the namespace (metadata names, generated qualified references). public const string KafkaNamespace = "Paramore.Brighter.MessagingGateway.Kafka"; public const string RequestTypeProperty = "RequestType"; diff --git a/src/Paramore.Brighter.Analyzer/Paramore.Brighter.Analyzer.csproj b/src/Paramore.Brighter.Analyzer/Paramore.Brighter.Analyzer.csproj index 71264c5b3a..94588fea2e 100644 --- a/src/Paramore.Brighter.Analyzer/Paramore.Brighter.Analyzer.csproj +++ b/src/Paramore.Brighter.Analyzer/Paramore.Brighter.Analyzer.csproj @@ -1,4 +1,4 @@ - + $(BrighterNetStandardTargetFrameworks) @@ -6,6 +6,7 @@ Analyzers for the brighter library Analyzer;Scheduler;Message Scheduling;Command Processor;Brighter false + enable true true diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs index 3baa705bbb..bcec77851c 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/KafkaPublicationPartitionerVisitor.cs @@ -41,14 +41,14 @@ public KafkaPublicationPartitionerVisitor(INamedTypeSymbol kafkaPublicationSymbo public bool IsPartitionerAssigned { get; private set; } public bool IsConsistentRandom { get; private set; } public bool IsConsistent { get; private set; } - public string PublicationName { get; private set; } - public Location PartitionerAssignmentLocation { get; private set; } + public string? PublicationName { get; private set; } + public Location? PartitionerAssignmentLocation { get; private set; } public override void VisitObjectCreation(IObjectCreationOperation operation) { if (IsKafkaPublicationType(operation.Type, _kafkaPublicationSymbol)) { - PublicationName = operation.Type.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); + PublicationName = operation.Type!.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); // base walks the children (including the initializer), which drives // VisitSimpleAssignment for any Partitioner assignment. Only descend @@ -86,7 +86,7 @@ public override void VisitSimpleAssignment(ISimpleAssignmentOperation operation) } // Type can be null for erroneous code in the IDE; treat it as no match. - internal static bool IsKafkaPublicationType(ITypeSymbol type, INamedTypeSymbol kafkaPublicationSymbol) + internal static bool IsKafkaPublicationType(ITypeSymbol? type, INamedTypeSymbol kafkaPublicationSymbol) { for (var current = type; current != null; current = current.BaseType) { @@ -99,7 +99,7 @@ internal static bool IsKafkaPublicationType(ITypeSymbol type, INamedTypeSymbol k return false; } - internal static string GetPartitionerValueName(IOperation value, INamedTypeSymbol partitionerEnumSymbol) + internal static string? GetPartitionerValueName(IOperation value, INamedTypeSymbol partitionerEnumSymbol) { // Unwrap an implicit conversion (e.g. enum widening) if present. if (value is IConversionOperation conversion) diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/RequestTypeAssignmentVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/RequestTypeAssignmentVisitor.cs index cc276c4ed1..e65583d6bd 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/RequestTypeAssignmentVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/RequestTypeAssignmentVisitor.cs @@ -1,4 +1,4 @@ -#region License +#region License /* The MIT License (MIT) Copyright © 2026 Aboubakr Nasef @@ -34,10 +34,10 @@ public class RequestTypeAssignmentVisitor : OperationWalker { public bool IsPublicationType { get; private set; } public bool IsRequestTypeAssigned { get; private set; } - public string PublicationName { get; private set; } + public string? PublicationName { get; private set; } public bool IsNotTypeOfIRequest { get; private set; } - public Location TypeOfLocation { get; private set; } - public string TypeOfName { get; private set; } + public Location? TypeOfLocation { get; private set; } + public string? TypeOfName { get; private set; } public override void VisitObjectCreation(IObjectCreationOperation operation) { diff --git a/src/Paramore.Brighter.Analyzer/Visitors/Operation/SubscriptionConstructorVisitor.cs b/src/Paramore.Brighter.Analyzer/Visitors/Operation/SubscriptionConstructorVisitor.cs index 58fd98a830..31114fe9ab 100644 --- a/src/Paramore.Brighter.Analyzer/Visitors/Operation/SubscriptionConstructorVisitor.cs +++ b/src/Paramore.Brighter.Analyzer/Visitors/Operation/SubscriptionConstructorVisitor.cs @@ -1,4 +1,4 @@ -#region License +#region License /* The MIT License (MIT) Copyright © 2026 Aboubakr Nasef @@ -30,7 +30,7 @@ namespace Paramore.Brighter.Analyzer.Visitors.Operation public class SubscriptionConstructorVisitor : OperationWalker { public bool IsMessagePumpDefault { get; private set; } = false; - public string SubscriptionName { get; private set; } + public string? SubscriptionName { get; private set; } public bool IsSubscriptionType { get; private set; } public override void VisitObjectCreation(IObjectCreationOperation operation) @@ -44,7 +44,7 @@ public override void VisitObjectCreation(IObjectCreationOperation operation) } public override void VisitArgument(IArgumentOperation operation) { - if (operation.Value.Type.Name == BrighterAnalyzerGlobals.MessagePumpTypeEnumName && operation.ArgumentKind == ArgumentKind.DefaultValue) + if (operation.Value.Type?.Name == BrighterAnalyzerGlobals.MessagePumpTypeEnumName && operation.ArgumentKind == ArgumentKind.DefaultValue) { IsMessagePumpDefault = true; } diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index 873d9182ff..d185274661 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -13,7 +13,7 @@ Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2 **Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. As with [BRT007](./BRT007.md) and [BRT008](./BRT008.md), existing publications that rely on the current partition assignment can safely ignore (or suppress) this warning instead of applying the fix. -The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block, and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. Subclass constructors can only be inspected when the subclass is declared in source — a subclass from a referenced assembly that sets `Partitioner` in its constructor will still be flagged; suppress it there too. +The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block — including inside nested blocks such as `if` statements — and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. Subclass constructors can only be inspected when the subclass is declared in source — a subclass from a referenced assembly that sets `Partitioner` in its constructor will still be flagged; suppress it there too. ### Example ```csharp diff --git a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs index d527151576..fb8ae5b81b 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/Analyzers/KafkaPublicationPartitionerAnalyzerTest.cs @@ -763,4 +763,36 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Consistent_Is_Set_In_Nested_Block_Should_Report_Only_Warning_At_Assignment() + { + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method(bool legacy) + { + var publication = new KafkaPublication(); + if (legacy) + { + {|#0:publication.Partitioner = Partitioner.Consistent|}; + } + } + } +} +"""; + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult( + KafkaPublicationPartitionerAnalyzer.ConsistentPartitionerRule + ).WithLocation(0) + ); + + await testContext.RunAsync(); + } } From 786036969c3bb899e33346885a15b0f484073740 Mon Sep 17 00:00:00 2001 From: Rafael Lillo <7280959+lillo42@users.noreply.github.com> Date: Mon, 27 Jul 2026 19:17:29 +0100 Subject: [PATCH 12/12] Apply code revew --- .../MissingPartitionerCodeFixProvider.cs | 20 ++++-- ...aramore.Brighter.Analyzer.CodeFixes.csproj | 1 + .../Paramore.Brighter.Analyzer.Package.props | 4 +- .../BrighterAnalyzerGlobals.cs | 2 +- src/Paramore.Brighter.Analyzer/docs/BRT006.md | 6 +- .../MissingPartitionerCodeFixProviderTest.cs | 70 +++++++++++++++++-- 6 files changed, 86 insertions(+), 17 deletions(-) diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs index e27c61c6e3..e97a2bac50 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/CodeFixes/MissingPartitionerCodeFixProvider.cs @@ -107,9 +107,19 @@ private static async Task AddPartitionerAsync( initializer = AddInitializerExpression(objectCreation.Initializer, assignment); } - var newObjectCreation = objectCreation - .WithInitializer(initializer) - .WithAdditionalAnnotations(Formatter.Annotation); + var newObjectCreation = objectCreation.WithInitializer(initializer); + + // Drop a now-redundant empty argument list: with an initializer present, + // new KafkaPublication { ... } reads better than new KafkaPublication() { ... }. + // Keep the argument list's trailing trivia (the space before the brace). + if (newObjectCreation is ObjectCreationExpressionSyntax { ArgumentList.Arguments.Count: 0 } explicitCreation) + { + newObjectCreation = explicitCreation + .WithType(explicitCreation.Type.WithTrailingTrivia(explicitCreation.ArgumentList.GetTrailingTrivia())) + .WithArgumentList(null); + } + + newObjectCreation = newObjectCreation.WithAdditionalAnnotations(Formatter.Annotation); var newRoot = root!.ReplaceNode(objectCreation, newObjectCreation); var formatted = await Formatter.FormatAsync(document.WithSyntaxRoot(newRoot), Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false); @@ -137,11 +147,11 @@ private static InitializerExpressionSyntax AddInitializerExpression( nodesAndTokens = nodesAndTokens.RemoveAt(nodesAndTokens.Count - 1); nodesAndTokens = nodesAndTokens.Replace( nodesAndTokens[nodesAndTokens.Count - 1], - ((ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode()) + ((ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode()!) .WithTrailingTrivia(trailingComma.TrailingTrivia)); } - var lastExpression = (ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode(); + var lastExpression = (ExpressionSyntax)nodesAndTokens[nodesAndTokens.Count - 1].AsNode()!; var trailingTrivia = lastExpression.GetTrailingTrivia(); var beforeEndOfLine = trailingTrivia.TakeWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); var fromEndOfLine = trailingTrivia.SkipWhile(t => !t.IsKind(SyntaxKind.EndOfLineTrivia)).ToList(); diff --git a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj index d3da459921..57966b1e48 100644 --- a/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj +++ b/src/Paramore.Brighter.Analyzer.CodeFixes/Paramore.Brighter.Analyzer.CodeFixes.csproj @@ -3,6 +3,7 @@ $(BrighterNetStandardTargetFrameworks) false + enable diff --git a/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props b/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props index ead6e08f09..8f7bec5577 100644 --- a/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props +++ b/src/Paramore.Brighter.Analyzer.Package/build/Paramore.Brighter.Analyzer.Package.props @@ -1,6 +1,6 @@ - - + + \ No newline at end of file diff --git a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs index 331b4fadf2..6c219b0749 100644 --- a/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs +++ b/src/Paramore.Brighter.Analyzer/BrighterAnalyzerGlobals.cs @@ -24,7 +24,7 @@ THE SOFTWARE. */ namespace Paramore.Brighter.Analyzer; -public class BrighterAnalyzerGlobals +public static class BrighterAnalyzerGlobals { public const string PublicationClassName = "Publication"; public const string KafkaPublicationClassName = "KafkaPublication"; diff --git a/src/Paramore.Brighter.Analyzer/docs/BRT006.md b/src/Paramore.Brighter.Analyzer/docs/BRT006.md index d185274661..95096e6563 100644 --- a/src/Paramore.Brighter.Analyzer/docs/BRT006.md +++ b/src/Paramore.Brighter.Analyzer/docs/BRT006.md @@ -11,9 +11,11 @@ The Brighter team wants users who work with Kafka to make the `Partitioner` choi ## How to fix Set the `Partitioner` explicitly on the `KafkaPublication`. `Partitioner.Murmur2Random` is the recommended value for new publications. -**Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. As with [BRT007](./BRT007.md) and [BRT008](./BRT008.md), existing publications that rely on the current partition assignment can safely ignore (or suppress) this warning instead of applying the fix. +**Changing the partitioner changes runtime behaviour.** Applying the fix to an existing publication moves it from the implicit `ConsistentRandom` default to `Murmur2Random`, which re-partitions the topic — keys will map to different partitions than before. -The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block — including inside nested blocks such as `if` statements — and one made by the constructor of a `KafkaPublication` subclass. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. Subclass constructors can only be inspected when the subclass is declared in source — a subclass from a referenced assembly that sets `Partitioner` in its constructor will still be flagged; suppress it there too. +**Keeping the current partition assignment.** No warning-free value preserves the existing key-to-partition mapping: the only value that matches the implicit default is `Partitioner.ConsistentRandom`, which [BRT007](./BRT007.md) flags — the warning-free values (`Murmur2Random`, `Murmur2`, `Random`) all re-partition the topic. To keep the current assignment while still making the choice visible in code, set `Partitioner = Partitioner.ConsistentRandom` explicitly and suppress BRT007 for that publication. Alternatively, leave it implicit and suppress BRT006. + +The rule recognizes a `Partitioner` assignment made in the object initializer, one made directly on the same instance (local, field, property or parameter) later in the same block — including inside nested blocks such as `if` statements — and one made by the constructor of a `KafkaPublication` subclass. This check is textual, not flow analysis: an assignment on a single conditional branch (`if (useKeys) publication.Partitioner = …`) or inside a loop counts as "set", even on paths where it never runs. Assignments made elsewhere — for example inside a helper method that configures the publication — are not tracked, so the warning may still fire for publications that are configured that way; suppress it in that case. Subclass constructors can only be inspected when the subclass is declared in source — a subclass from a referenced assembly that sets `Partitioner` in its constructor will still be flagged; suppress it there too. ### Example ```csharp diff --git a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs index 38d0527163..171647e95f 100644 --- a/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs +++ b/tests/Paramore.Brighter.Analyzer.Tests/CodeFixes/MissingPartitionerCodeFixProviderTest.cs @@ -1,4 +1,6 @@ +using System.IO; using System.Threading.Tasks; +using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; using Paramore.Brighter.Analyzer.Analyzers; using Paramore.Brighter.Analyzer.CodeFixes; @@ -40,7 +42,7 @@ class TypeName { public void Method() { - var publication = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var publication = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; } } } @@ -80,7 +82,7 @@ class TypeName { public void Method() { - var publication = new Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication() { Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random }; + var publication = new Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication { Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random }; } } } @@ -332,8 +334,8 @@ class TypeName { public void Method() { - var first = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; - var second = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var first = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; + var second = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; } } } @@ -350,8 +352,8 @@ class TypeName { public void Method() { - var first = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; - var second = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var first = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; + var second = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; } } } @@ -561,7 +563,7 @@ class TypeName { public void Method() { - var publication = new KafkaPublication() { Partitioner = Partitioner.Murmur2Random }; + var publication = new KafkaPublication { Partitioner = Partitioner.Murmur2Random }; } } } @@ -575,4 +577,58 @@ public void Method() await testContext.RunAsync(); } + + [Fact] + public async Task When_Partitioner_Is_Ambiguous_With_Confluent_Should_Add_Fully_Qualified_Partitioner() + { + testContext.TestState.AdditionalReferences.Add( + MetadataReference.CreateFromFile( + Path.Combine( + Path.GetDirectoryName(typeof(Paramore.Brighter.MessagingGateway.Kafka.KafkaPublication).Assembly.Location), + "Confluent.Kafka.dll"))); + + testContext.TestCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; +using Confluent.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new {|#0:KafkaPublication|}(); + } + } +} +"""; + + testContext.FixedCode = /* lang=c#-test */ + """ +using Paramore.Brighter; +using Paramore.Brighter.MessagingGateway.Kafka; +using Confluent.Kafka; + +namespace ConsoleApplication1 +{ + class TypeName + { + public void Method() + { + var publication = new KafkaPublication { Partitioner = Paramore.Brighter.MessagingGateway.Kafka.Partitioner.Murmur2Random }; + } + } +} +"""; + + testContext.ExpectedDiagnostics.Add( + new DiagnosticResult(KafkaPublicationPartitionerAnalyzer.MissingPartitionerRule) + .WithLocation(0) + .WithArguments("KafkaPublication") + ); + + await testContext.RunAsync(); + } }