From eb328f8b50011b86aaef3bc4c834bff5f510602f Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Fri, 4 Sep 2026 17:28:44 +0300 Subject: [PATCH] refactor: make the unreachable-surface guard see every observable property EveryModelProperty_IsEitherWrittenOrShown detects [ObservableProperty] fields with the prefix [^\n]*\n?, which is wrong in both directions. It skips forward: for the single-line form followed by a plain private field, the prefix eats the attribute's own line, crosses the newline and captures the NEXT field. Four such mis-captures exist in ViewModels/ today, each hiding the real property beside it. And one newline is not enough for the multi-attribute form, so every property with a [NotifyPropertyChangedFor] beside it was invisible -- 23 of the 187 declarations in Models/, six on DiskHealthReport alone. The floor is what made that survivable: 164 of 187 matched, and the assertion was >= 40. Re-measured and raised to 180. A mutation proof shows the greedy prefix now reddens, and that with the old floor of 40 it stays GREEN -- the regex was the defect, the floor value is what let it live. The corrected guard finds no new dead model surface, so this is a coverage fix rather than a bug hunt with a result. Also removes three view-model properties from the hand-measured list in #2100: SpeedStatus (declared, never assigned, never read), TotalFolders (a full recursive Sum per scan, read nowhere) and CurrentFolder (read nowhere, and the next line already puts the same value into the bound StatusMessage). The assertion on TotalFolders' default -- which is what made it look exercised -- went with it. --- .../SysManager.Tests/ArchitectureTests.cs | 27 ++++++++++++++++--- .../DiskAnalyzerViewModelTests.cs | 4 ++- .../ViewModels/DiskAnalyzerViewModel.cs | 6 ----- .../ViewModels/SpeedTestViewModel.cs | 1 - 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/SysManager/SysManager.Tests/ArchitectureTests.cs b/SysManager/SysManager.Tests/ArchitectureTests.cs index 4a92426b..53141c0a 100644 --- a/SysManager/SysManager.Tests/ArchitectureTests.cs +++ b/SysManager/SysManager.Tests/ArchitectureTests.cs @@ -1844,9 +1844,13 @@ public void EveryModelProperty_IsEitherWrittenOrShown() } } - Assert.True(checkedProperties >= 40, - $"only {checkedProperties} model properties were inspected — the detection is probably no " - + "longer matching the [ObservableProperty] declarations."); + // Measured: 187 today. The floor is set just under that rather than at a token value, because the + // defect it has to catch is precisely a pattern that still matches MOST declarations — the previous + // prefix saw 164 of these 187 and its floor of 40 reported nothing wrong for as long as it shipped. + Assert.True(checkedProperties >= 180, + $"only {checkedProperties} model properties were inspected, out of 187 measured — the detection " + + "is no longer matching every [ObservableProperty] declaration. A pattern that matches most of " + + "them still leaves the rest unguarded, so fix this before trusting a pass."); Assert.True(dead.Count == 0, "these model properties are never written and never shown, so they can only ever present an " @@ -1858,7 +1862,22 @@ public void EveryModelProperty_IsEitherWrittenOrShown() private static partial Regex TypeDeclaration(); /// An [ObservableProperty] backing field, capturing the field name without its underscore. - [GeneratedRegex(@"\[ObservableProperty\][^\n]*\n?\s*(?:private|internal)\s+[\w\?<>,\[\]\. ]+?\s+_(\w+)\s*[;=]", + /// + /// The prefix was [^\n]*\n?, which is wrong in both directions. + /// It let the match run to the end of the attribute's own line and then across ONE newline, so for + /// the single-line form followed by a plain field — + /// [ObservableProperty] private string _label = ""; then private string? _target; — it walked + /// past _label and captured _target, a field that is not observable at all. Four such + /// mis-captures exist in ViewModels/ today. + /// And one newline is not enough for the multi-attribute form, so every property carrying a + /// [NotifyPropertyChangedFor] beside it was invisible: 23 of the 187 properties in + /// Models/, including six on DiskHealthReport alone. The guard's floor of 40 could never + /// reveal that, since 164 clears it comfortably. + /// Now: optional whitespace, then any number of further attributes, then the field. Nothing on the + /// attribute's own line can be skipped over. + /// + [GeneratedRegex(@"\[ObservableProperty\]\s*(?:\[[^\]]*\]\s*)*(?:private|internal)\s+" + + @"[\w\?<>,\[\]\. ]+?\s+_(\w+)\s*[;=]", RegexOptions.Compiled)] private static partial Regex ObservablePropertyField(); diff --git a/SysManager/SysManager.Tests/DiskAnalyzerViewModelTests.cs b/SysManager/SysManager.Tests/DiskAnalyzerViewModelTests.cs index 62a7ae41..cf9763d3 100644 --- a/SysManager/SysManager.Tests/DiskAnalyzerViewModelTests.cs +++ b/SysManager/SysManager.Tests/DiskAnalyzerViewModelTests.cs @@ -30,11 +30,13 @@ private static DiskAnalyzerViewModel NewVm() [Fact] public void Constructor_InitialState_IsCorrect() { + // TotalFolders was asserted here too. It was computed from a full recursive + // Entries.Sum(e => e.FolderCount) on every scan and read by nothing — no binding, no other + // code — and this assertion on its default value is what made it look exercised. var vm = NewVm(); Assert.False(vm.IsBusy); Assert.Equal(0, vm.TotalSize); Assert.Equal(0, vm.TotalFiles); - Assert.Equal(0, vm.TotalFolders); Assert.Equal(0, vm.EntryCount); Assert.Empty(vm.Entries); Assert.Contains("Select", vm.ScanSummary); diff --git a/SysManager/SysManager/ViewModels/DiskAnalyzerViewModel.cs b/SysManager/SysManager/ViewModels/DiskAnalyzerViewModel.cs index a7330f55..87962e95 100644 --- a/SysManager/SysManager/ViewModels/DiskAnalyzerViewModel.cs +++ b/SysManager/SysManager/ViewModels/DiskAnalyzerViewModel.cs @@ -44,9 +44,7 @@ public sealed partial class DiskAnalyzerViewModel : ViewModelBase public bool HasTrend => !string.IsNullOrEmpty(TrendSummary); [ObservableProperty] private long _totalSize; [ObservableProperty] private int _totalFiles; - [ObservableProperty] private int _totalFolders; [ObservableProperty] private int _entryCount; - [ObservableProperty] private string _currentFolder = ""; // Distinguishes the un-run state from a completed zero-result scan so the big empty-state overlay // doesn't tell the user to "pick a folder and analyze" right after they did exactly that. Set true @@ -153,7 +151,6 @@ private async Task AnalyzeAsync() Entries.Clear(); TotalSize = 0; TotalFiles = 0; - TotalFolders = 0; EntryCount = 0; UpdateDriveInfo(); @@ -162,7 +159,6 @@ private async Task AnalyzeAsync() { var progress = new Progress(p => { - CurrentFolder = p.CurrentFolder; StatusMessage = $"Scanning folder {p.FoldersScanned}: {p.CurrentFolder}"; }); @@ -173,7 +169,6 @@ private async Task AnalyzeAsync() EntryCount = Entries.Count; TotalSize = Entries.Sum(e => e.SizeBytes); TotalFiles = Entries.Sum(e => e.FileCount); - TotalFolders = Entries.Sum(e => e.FolderCount); ScanSummary = EntryCount == 0 ? "No subfolders found." @@ -201,7 +196,6 @@ private async Task AnalyzeAsync() { IsBusy = false; IsProgressIndeterminate = false; - CurrentFolder = ""; } } diff --git a/SysManager/SysManager/ViewModels/SpeedTestViewModel.cs b/SysManager/SysManager/ViewModels/SpeedTestViewModel.cs index 335c90ca..f9fdac0b 100644 --- a/SysManager/SysManager/ViewModels/SpeedTestViewModel.cs +++ b/SysManager/SysManager/ViewModels/SpeedTestViewModel.cs @@ -45,7 +45,6 @@ public sealed partial class SpeedTestViewModel : ViewModelBase "New York, US (ID: 10390)", }; [ObservableProperty] private int _speedProgress; - [ObservableProperty] private string _speedStatus = ""; [ObservableProperty] private string _httpStatus = ""; [ObservableProperty] private string _ooklaStatus = ""; [ObservableProperty] private bool _isSpeedTesting;