Merge pull request #396 from MPCoreDeveloper/sonar/mechanical-cleanup

sonar: fix mechanical and new-code issues (batch 1)

Author: MPCoreDeveloper

src/SharpCoreDB/DataStructures/OverflowArena.cs

@@ -121,34 +121,29 @@ public long Write(byte[] payload)
     private bool TryReuseFreeBlock(byte[] payload, out long offset)
     {
         offset = 0;
-        if (!_freeByLength.TryGetValue(payload.Length, out var offsets))
+        if (!_freeByLength.TryGetValue(payload.Length, out var offsets) || offsets.Count == 0)
         {
             return false;
         }
 
-        while (offsets.Count > 0)
-        {
-            offset = offsets[^1];
-            offsets.RemoveAt(offsets.Count - 1);
+        offset = offsets[^1];
+        offsets.RemoveAt(offsets.Count - 1);
 
-            if (_storage.OverwriteRecordAt(_filePath, offset, payload))
+        if (_storage.OverwriteRecordAt(_filePath, offset, payload))
+        {
+            if (offsets.Count == 0)
             {
-                if (offsets.Count == 0)
-                {
-                    _freeByLength.Remove(payload.Length);
-                }
-
-                _cache[offset] = payload;
-                _blockReuses++;
-                return true;
+                _freeByLength.Remove(payload.Length);
             }
 
-            // In-place overwrite refused (e.g. transaction active): keep the block free for a
-            // later write and try the next candidate; if none succeeds we fall back to append.
-            offsets.Add(offset);
-            break;
+            _cache[offset] = payload;
+            _blockReuses++;
+            return true;
         }
 
+        // In-place overwrite refused (e.g. transaction active): keep the block free for a later
+        // write and fall back to appending.
+        offsets.Add(offset);
         offset = 0;
         return false;
     }

src/SharpCoreDB/DataStructures/Table.CRUD.cs

@@ -2272,8 +2272,8 @@ oldHashValues is not null &&
                                     var newPkVal = row[this.Columns[this.PrimaryKeyIndex]]?.ToString() ?? string.Empty;
                                     if (!string.Equals(newPkVal, oldPkValue?.ToString(), StringComparison.Ordinal))
                                     {
-                                        if (!string.IsNullOrEmpty(oldPkValue?.ToString()))
-                                            this.Index.Delete(oldPkValue!.ToString()!);
+                                        if (oldPkValue?.ToString() is { Length: > 0 } oldPkString)
+                                            this.Index.Delete(oldPkString);
                                         if (!string.IsNullOrEmpty(newPkVal))
                                             this.Index.Insert(newPkVal, oldPosition);
                                     }
@@ -2692,12 +2692,9 @@ private bool HasExplicitNamedIndex(string column)
                 // Transactional delete: buffer the physical offsets so the in-place marker is
                 // applied at COMMIT (rollback discards the buffer). Durable in O(delete) — the
                 // flush-time full-file rewrite is no longer needed for transactional deletes.
-                foreach (var position in positions)
+                foreach (var position in positions.Where(static position => position >= 0))
                 {
-                    if (position >= 0)
-                    {
-                        this.storage.BufferTombstoneForCommit(DataFile, position);
-                    }
+                    this.storage.BufferTombstoneForCommit(DataFile, position);
                 }
             }
             else
@@ -2755,10 +2752,10 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
         List<long>? remainingPositions = null;
         if (!this.storage.AreRecordsEncrypted(DataFile))
         {
-            remainingPositions = ScanLegacyPlaintextRemainingKeys(DataFile, pkCol, deletedKeys);
+            remainingPositions = ScanLegacyPlaintextRemainingKeys(DataFile, deletedKeys);
         }
 
-        remainingPositions ??= ScanLegacyRemainingKeysViaStorage(DataFile, pkCol, deletedKeys);
+        remainingPositions ??= ScanLegacyRemainingKeysViaStorage(DataFile, deletedKeys);
         if (remainingPositions is { Count: > 0 })
         {
             TombstoneDeletedPositions(remainingPositions.ToArray());
@@ -2772,7 +2769,7 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
     /// <see langword="null"/> when the raw layout could not be parsed safely (the caller then falls
     /// back to the storage-layer scan, which understands per-record encryption).
     /// </summary>
-    private List<long>? ScanLegacyPlaintextRemainingKeys(string dataFile, string pkCol, HashSet<string> deletedKeys)
+    private List<long>? ScanLegacyPlaintextRemainingKeys(string dataFile, HashSet<string> deletedKeys)
     {
         var matches = new List<long>();
         try
@@ -2822,7 +2819,7 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
                     break;
                 }
 
-                if (TryReadPrimaryKeyFromLegacyRecord(recordData, pkCol, out var pkStr) && deletedKeys.Contains(pkStr))
+                if (TryReadPrimaryKeyFromLegacyRecord(recordData, out var pkStr) && deletedKeys.Contains(pkStr))
                 {
                     matches.Add(position);
                 }
@@ -2843,12 +2840,12 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
     /// plaintext raw scan is unavailable): iterates the records through
     /// <c>storage.ReadAllRecords</c>, which decrypts payloads and already skips tombstone markers.
     /// </summary>
-    private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, string pkCol, HashSet<string> deletedKeys)
+    private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, HashSet<string> deletedKeys)
     {
         var matches = new List<long>();
-        foreach (var (recordOffset, recordData) in this.storage!.ReadAllRecords(dataFile))
+        foreach (var (recordOffset, recordData) in this.storage.ReadAllRecords(dataFile))
         {
-            if (TryReadPrimaryKeyFromLegacyRecord(recordData, pkCol, out var pkStr) && deletedKeys.Contains(pkStr))
+            if (TryReadPrimaryKeyFromLegacyRecord(recordData, out var pkStr) && deletedKeys.Contains(pkStr))
             {
                 matches.Add(recordOffset);
             }
@@ -2861,7 +2858,7 @@ private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, string pkC
     /// Walks a legacy variable-length record and returns the value of the primary-key column
     /// (the same layout walk used by the reopen index rebuild).
     /// </summary>
-    private bool TryReadPrimaryKeyFromLegacyRecord(byte[] recordData, string pkCol, out string? pkValue)
+    private bool TryReadPrimaryKeyFromLegacyRecord(byte[] recordData, out string? pkValue)
     {
         pkValue = null;
         try

src/SharpCoreDB/Database/Core/Database.Core.cs

@@ -35,7 +35,7 @@ public partial class Database : IDatabase, IDisposable, IAsyncDisposable
     // Diagnostics: number of DELETE statements routed through the canonical structured batch path
     // (proves the scanner/batch fast paths are engaged; 0 means everything fell back to regex).
     private static long _canonicalDeleteStatements;
-    public long CanonicalDeleteStatementsParsed => Interlocked.Read(ref _canonicalDeleteStatements);
+    public static long CanonicalDeleteStatementsParsed => Interlocked.Read(ref _canonicalDeleteStatements);
 
     private readonly IServiceProvider _serviceProvider;
     private readonly IStorage storage;

tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs

@@ -299,12 +299,12 @@ public void CanonicalBatchDelete_EngagesStructuredPath()
             InsertDocs(db, 1, 50);
             db.Flush();
 
-            var before = ((SharpCoreDB.Database)db).CanonicalDeleteStatementsParsed;
+            var before = SharpCoreDB.Database.CanonicalDeleteStatementsParsed;
             db.ExecuteBatchSQL(BuildDeletes(1, 10));
             db.Flush();
 
             Assert.True(
-                ((SharpCoreDB.Database)db).CanonicalDeleteStatementsParsed >= before + 10,
+                SharpCoreDB.Database.CanonicalDeleteStatementsParsed >= before + 10,
                 "canonical DELETE statements must flow through the structured batch path");
             Assert.Equal(40, db.ExecuteQuery("SELECT id FROM docs").Count);
         }

tests/SharpCoreDB.Tests/FixedWidthRecordLayoutTests.cs

@@ -58,7 +58,7 @@ public void RoundTrip_AllColumnTypes_PointAndFullScan()
             Assert.Single(row);
             Assert.Equal("beta", row[0]["name"]);
             Assert.Equal(2.5, Convert.ToDouble(row[0]["score"]));
-            Assert.Equal(false, Convert.ToBoolean(row[0]["flag"]));
+            Assert.False(Convert.ToBoolean(row[0]["flag"]));
 
             var all = db.ExecuteQuery("SELECT * FROM t ORDER BY id");
             Assert.Equal(2, all.Count);