Add analyzer and code fix for test methods with out/ref parameters#7108
Add analyzer and code fix for test methods with out/ref parameters#7108Evangelink merged 24 commits intomainfrom
Conversation
Co-authored-by: Evangelink <[email protected]>
Co-authored-by: Evangelink <[email protected]>
|
@copilot please create a different analyzer as current analyzer severity is set to "warning" and so is more tuned for cases that really cause problem at runtime. This case is more of a suggestion so we can be stricter on next breaking change. You should also change the implementation on MSTest to not fail but just report a warning. |
…me warning - Created AvoidOutRefTestMethodParametersAnalyzer with severity Info - Reverted changes to existing MSTEST0003 analyzer - Added runtime warning in TestMethodInfo.InvokeAsync instead of failing - Added comprehensive tests for the new analyzer - Updated all localization files with new resource strings Co-authored-by: Evangelink <[email protected]>
…sage to resources - Changed analyzer category from Design to Usage (more appropriate) - Extracted runtime warning message to Resource.resx for consistency - Updated all runtime resource xlf files with new message Co-authored-by: Evangelink <[email protected]>
I've updated the implementation based on your feedback:
See commits:
The gradual approach allows users to see warnings now and prepare for stricter enforcement in future versions. |
Evangelink
left a comment
There was a problem hiding this comment.
@copilot let's add a codefix that removes the out or/and ref part. The xlf files are auto-generated as part of the build so don't process them manually.
src/Analyzers/MSTest.Analyzers/AvoidOutRefTestMethodParametersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs
Outdated
Show resolved
Hide resolved
…d test method attributes - Added AvoidOutRefTestMethodParametersFixer to remove out/ref modifiers - Updated resource messages per feedback (removed "unsupported", clarified reasoning) - Changed analyzer to use Inherits() to support attributes inheriting from TestMethod - Updated all tests to use code fix verification - Added test for DataTestMethod attribute (inherits from TestMethod) Co-authored-by: Evangelink <[email protected]>
Added code fix in commit 9a75cb2. The |
…mentation - Fixed grammar in message: "for which support will be dropped" - Optimized code fix to filter modifiers in single operation instead of nested WithModifiers calls Co-authored-by: Evangelink <[email protected]>
…dValidator - Removed warning logic from TestMethodInfo.InvokeAsync - Added warning to TestMethodValidator.IsValidTestMethod warnings collection - Method now returns true but adds warning (non-blocking validation) Co-authored-by: Evangelink <[email protected]>
- Added parameters.Length > 0 check before Any() to avoid enumeration when no parameters exist Co-authored-by: Evangelink <[email protected]>
Evangelink
left a comment
There was a problem hiding this comment.
@copilot please fix the indentation of the first <trans-unit section you have added to all .xlf in this PR
| MessageFormat, | ||
| Description, | ||
| Category.Usage, | ||
| DiagnosticSeverity.Info, |
There was a problem hiding this comment.
I think it might be worth having it as a warning, before we ship it.
Test methods with
outorrefparameters are not properly supported by the test framework. This PR adds a new informational analyzer with automatic code fix and runtime warning to help users identify and migrate away from this pattern before support is dropped in a future major version.Example code that triggers the warning:
Changes Made
New Analyzer (MSTEST0060)
AvoidOutRefTestMethodParametersAnalyzeras a separate analyzerInfo(informational suggestion, not warning)Usage[TestMethod](e.g.,[DataTestMethod])Code Fix
AvoidOutRefTestMethodParametersFixeroutandrefmodifiers from test method parametersFindNodeand always apply changes (no unnecessary checks)Runtime Warning
TestMethodValidator.IsValidTestMethodto detect out/ref parameters during test discovery/validationtrue(non-blocking) but adds warning to the warnings collectionResource.UTA_OutRefParametersNotSupportedfor consistency across compile-time and runtimeparameters.Length > 0before LINQ enumeration to avoid unnecessary overheadLocalization
Resources.resxCodeFixResources.resxResource.resxTests
AvoidOutRefTestMethodParametersAnalyzerTests.cswith code fix verification[DataTestMethod]attribute to verify inheritance detectionMigration Path
This approach provides a gradual, non-breaking migration path with automatic fix capability:
Not a breaking change: Existing tests with out/ref parameters continue to run, but users are warned about the pattern during validation and can automatically fix it.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.