Analyzer: Error when Feature Defaults are used to enable other features #7468
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielmarbach
changed the title
danielmarbach
requested review from
DavidBoike,
andreasohlund and
mattmercurio
DavidBoike
requested changes
Nov 15, 2025
Member
DavidBoike
left a comment
DavidBoike
left a comment
There was a problem hiding this comment.
Initial review done on an iPad on an airplane, no compiler support. 😉
src/NServiceBus.Core.Analyzer/FeatureDefaultsEnableFeatureAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Core.Analyzer/FeatureDefaultsEnableFeatureAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Core.Analyzer/FeatureDefaultsEnableFeatureAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Core.Analyzer.Tests.Common/FeatureDefaultsEnableFeatureAnalyzerTests.cs
Show resolved
Hide resolved
src/NServiceBus.Core.Analyzer.Tests.Common/FeatureDefaultsEnableFeatureFixerTests.cs
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
* Initial plan * Optimize analyzer performance and modernize test code blocks Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: danielmarbach <174258+danielmarbach@users.noreply.github.com>
DavidBoike
reviewed
Nov 15, 2025
src/NServiceBus.Core.Analyzer/FeatureDefaultsEnableFeatureAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/NServiceBus.Core.Analyzer/FeatureDefaultsEnableFeatureAnalyzer.cs
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
|
@danielmarbach I've opened a new pull request, #7476, to work on those changes. Once the pull request is ready, I'll request review from you. |
andreasohlund
approved these changes
Nov 17, 2025
Member
andreasohlund
left a comment
andreasohlund
left a comment
There was a problem hiding this comment.
LGTM (based on very limited analyzer knowledge so I would wait for Davids approval)
DavidBoike
approved these changes
Nov 18, 2025
Member
DavidBoike
left a comment
DavidBoike
left a comment
There was a problem hiding this comment.
LGTM, I can't think of any other tests to add, let's go with it but be sure to smoke test it with the next alpha.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new Roslyn analyzer and code fix to enforce best practices around enabling features. It detects and corrects the use of
EnableFeature<TFeature>()within theDefaultsmethod of a feature, prompting developers to use the protectedEnable<TFeature>()method in the constructor instead.Analyzer and code fix for feature enablement:
FeatureDefaultsEnableFeatureAnalyzerto report diagnostics whenEnableFeature<TFeature>()is called inside aDefaultslambda, enforcing that dependent features should be enabled usingEnable<TFeature>()in the constructor instead.FeatureDefaultsEnableFeatureFixerto automatically moveEnableFeature<TFeature>()calls from withinDefaultslambdas to the feature constructor, optionally removing or updating theDefaultsblock as needed.NSB0019for this rule inDiagnosticIds.Scoped out: