feat: Phase 4a manual-review diagnostics for lossy MockFileData APIs#10
Merged
Conversation
Surface call sites that have no equivalent in Testably.Abstractions.Testing
with discriminating pattern ids so users can locate and migrate them by hand.
The code-fix provider intentionally registers no rewrite for these patterns;
the analyzer's role here is to make the lossy surface discoverable rather
than fix it.
Patterns added:
- MockFileData.AccessControl (Windows FileSecurity, no Testably equivalent)
- MockFileData.AllowedFileShare (no Testably equivalent)
- MockFileData.UnixMode (no Testably equivalent)
- MockFileVersionInfo.ctor (no Testably equivalent)
For the three property patterns the analyzer classifies by property name
before falling back to the generic MockFileDataPropertyRead/Write so the
specific pattern id wins. For MockFileVersionInfo the analyzer extends the
existing ObjectCreation handler. The dispatcher switch in the code-fix
provider has no default case, so unknown pattern ids fall through silently
as required by the verifier ("No code fixes were expected" otherwise).
Playground gains ManualReviewTests.cs as parity baseline; AccessControl
test runtime-skips on non-Windows.
Test Results 6 files ± 0 6 suites ±0 1m 15s ⏱️ +8s Results for commit 9f55589. ± Comparison against base commit fac4dab. This pull request removes 2 and adds 15 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Adds Phase 4a “manual-review” diagnostics to surface lossy System.IO.Abstractions.TestingHelpers APIs that have no Testably.Abstractions.Testing equivalent, intentionally providing no code fix so users can locate and handle these call sites manually.
Changes:
- Extend the analyzer to emit discriminating pattern ids for
MockFileData.AccessControl,MockFileData.AllowedFileShare,MockFileData.UnixMode, andnew MockFileVersionInfo(...). - Add new pattern constants and symbol lookup support (
MockFileVersionInfo) to enable the new classifications. - Update analyzer/code-fix tests and add a playground “parity baseline” test file for manual-review scenarios.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Testably.Abstractions.Migration.Tests/SystemIOAbstractionsCodeFixProviderTests.MockFileDataTests.cs | Adds “no fix expected” tests for newly introduced manual-review diagnostics. |
| Tests/Testably.Abstractions.Migration.Tests/SystemIOAbstractionsAnalyzerTests.cs | Updates analyzer tests to cover the new manual-review diagnostics and constructor pattern. |
| Tests/Testably.Abstractions.Migration.SystemIOAbstractionsPlayground/ManualReviewTests.cs | Adds runtime “manual-review” parity fixtures for lossy APIs (incl. Windows-only AccessControl). |
| Source/Testably.Abstractions.Migration.Analyzers/SystemIOAbstractionsAnalyzer.cs | Implements new manual-review pattern classification for properties and MockFileVersionInfo construction. |
| Source/Testably.Abstractions.Migration.Analyzers/Patterns.cs | Introduces new manual-review pattern ids. |
| Source/Testably.Abstractions.Migration.Analyzers/Common/TestableIoSymbols.cs | Adds MockFileVersionInfo symbol caching for analyzer classification. |
| Source/Testably.Abstractions.Migration.Analyzers.CodeFixers/SystemIOAbstractionsCodeFixProvider.cs | Documents intentional silent fall-through for manual-review patterns (no code-fix registration). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Phase 4a missed lossy writes inside object initializers because the
object-initializer skip ran before the manual-review classification, so
`new MockFileData(...) { AllowedFileShare = X }` (and AccessControl,
UnixMode) was silently dropped from the diagnostic set.
The migratable property writes (Attributes, TextContents, etc.) still
need the skip because they are picked up by the AddFile expansion in
Phase 3.5 — double-flagging would be noise. Manual-review properties
have no AddFile expansion, so this branch must report them or the lossy
call site is invisible.
Reordered the property-reference analyzer so the manual-review
classification fires first; the object-initializer skip now only applies
to the generic read/write fallback.
Tests cover plain writes, object-initializer writes (where both the
AddFile invocation and the property write produce diagnostics on the same
user-visible call site), and HasNoFix for the new write path.
|
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
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.
Surface call sites that have no equivalent in Testably.Abstractions.Testing with discriminating pattern ids so users can locate and migrate them by hand. The code-fix provider intentionally registers no rewrite for these patterns; the analyzer's role here is to make the lossy surface discoverable rather than fix it.
Patterns added:
For the three property patterns the analyzer classifies by property name before falling back to the generic MockFileDataPropertyRead/Write so the specific pattern id wins. For MockFileVersionInfo the analyzer extends the existing ObjectCreation handler. The dispatcher switch in the code-fix provider has no default case, so unknown pattern ids fall through silently as required by the verifier ("No code fixes were expected" otherwise).
Playground gains ManualReviewTests.cs as parity baseline; AccessControl test runtime-skips on non-Windows.