Conversation
a75029e to
5e9246b
Compare
…MultipleScope` And similarly for `Assert.MultipleAsync`
5e9246b to
32487ac
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an analyzer and corresponding code fix to replace Assert.Multiple/Assert.MultipleAsync with Assert.EnterMultipleScope, streamlining multiple assertions handling.
- Added new constants, analyzer, and code fix to perform the replacement.
- Updated tests and documentation to validate and explain the new pattern.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/nunit.analyzers/UseAssertEnterMultipleScope/UseAssertEnterMultipleScopeConstants.cs | Defines constants used by the analyzer and code fix. |
| src/nunit.analyzers/UseAssertEnterMultipleScope/UseAssertEnterMultipleScopeAnalyzer.cs | Implements the analyzer that flags the usage of Assert.Multiple/Assert.MultipleAsync. |
| src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs | Introduces a new analyzer identifier for the Assert.EnterMultipleScope rule. |
| src/nunit.analyzers.codefixes/UseAssertEnterMultipleScope/UseAssertEnterMultipleScopeCodeFix.cs | Provides the code fix that replaces the flagged invocations with a using statement for Assert.EnterMultipleScope. |
| src/nunit.analyzers.tests/UseAssertEnterMultipleScope/UseAssertEnterMultipleScopeCodeFixTests.cs | Contains tests verifying the code fix behavior. |
| src/nunit.analyzers.tests/UseAssertEnterMultipleScope/UseAssertEnterMultipleScopeAnalyzerTests.cs | Contains tests ensuring the analyzer correctly identifies the flagged patterns. |
| src/nunit.analyzers.sln | Updates the solution file to include documentation for the new rule. |
| documentation/index.md & documentation/NUnit2056.md | Update project documentation to include the new analyzer and explain its purpose. |
manfred-brands
left a comment
There was a problem hiding this comment.
What is there is great, you have forgotten one use case where users passed an async method to Assert.Multiple.
The use case is treated as non-async code, so the issue is flagged, but the code fix generated invalid code as the return type of the test method needs to be async Task.
manfred-brands
left a comment
There was a problem hiding this comment.
Nice find on using ReplaceNode twice and finding your node using some "SyntaxAnnotation"
I missed last time that FindNearestParentOfType already has a standard equivalent.
One remark about the DescendantNodes being expensive with suggested alternatives.
|
Updated PR with requested changes and also added another test |
manfred-brands
left a comment
There was a problem hiding this comment.
One nitpick, otherwise nothing else.
And similarly for
Assert.MultipleAsync=>Assert.EnterMultipleScope.Fixes #866