From 2d3456f0c7e42f057f3b58f0cd7d5bf43c7eec6c Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 10:23:57 +0100 Subject: [PATCH 1/7] Add Directory.Build.props to enable nullable and full analyzers Turn on Roslyn/FxCop-style analysis repo-wide (AnalysisMode=All, AnalysisLevel=latest, EnforceCodeStyleInBuild) so a library with downstream consumers surfaces the issues SonarQube was missing. Scope the noisier library-only settings to the shipping assembly: Nullable reference types and XML documentation generation, so the test and benchmark projects don't drown in warnings. Addresses review recommendations #1, #2 and #5 from PR #285. Co-Authored-By: Claude Opus 4.8 --- Directory.Build.props | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 Directory.Build.props diff --git a/Directory.Build.props b/Directory.Build.props new file mode 100644 index 0000000..1470dad --- /dev/null +++ b/Directory.Build.props @@ -0,0 +1,25 @@ + + + + + latest + + true + latest + All + true + + + + + + enable + + true + + + From 946bfd8dae4c43dd634051ec9e05b5f26fe9576c Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 10:24:10 +0100 Subject: [PATCH 2/7] Add .editorconfig with analyzer severities and conventions Keep the newly-enabled analysis manageable while the backlog is triaged: silence CS1591 (documenting the whole public surface is a gradual effort, the XML file still emits the docs that exist), set style diagnostics to suggestion, and silence CA1707 under test/** where underscore-separated test names are an intentional convention. Co-Authored-By: Claude Opus 4.8 --- .editorconfig | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..5b94f0f --- /dev/null +++ b/.editorconfig @@ -0,0 +1,30 @@ +root = true + +[*] +charset = utf-8 +insert_final_newline = true +trim_trailing_whitespace = true +indent_style = space +indent_size = 4 +end_of_line = lf + +[*.{csproj,props,targets,json,yml,yaml}] +indent_size = 2 + +[*.cs] +# --- Analyzer rule severities --------------------------------------------- +# GenerateDocumentationFile turns on CS1591 for every undocumented public +# member. Documenting the whole public surface is a gradual effort, so keep it +# quiet for now and re-enable when the API is fully documented. The XML doc file +# is still produced for the docs that DO exist (e.g. /// comments). +dotnet_diagnostic.CS1591.severity = none + +# Analyzers should highlight real problems without failing local builds while +# the backlog is triaged; promote individual rules (or set +# TreatWarningsAsErrors) once each is clean. +dotnet_analyzer_diagnostic.category-Style.severity = suggestion + +# Test method names deliberately use underscores (Should_do_x); that's a +# readability convention here, not a naming-guideline violation. +[test/**.cs] +dotnet_diagnostic.CA1707.severity = none From 753bb9985fc36db731ea5dd08d687099e76742be Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 10:24:17 +0100 Subject: [PATCH 3/7] Add standard exception constructors (CA1032) DbfFileFormatException was an empty subclass of Exception, and CdxException and SqlParseException each exposed only their custom constructor. Add the parameterless, (string) and (string, Exception) constructors so all three follow the standard exception pattern, clearing CA1032 and giving consumers the usual construction options. Co-Authored-By: Claude Opus 4.8 --- src/DbfDataReader/Cdx/CdxException.cs | 14 ++++++++++++++ src/DbfDataReader/DbfFileFormatException.cs | 15 ++++++++++++++- src/DbfDataReader/Query/SqlParseException.cs | 14 ++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/DbfDataReader/Cdx/CdxException.cs b/src/DbfDataReader/Cdx/CdxException.cs index 0995d55..c0a36e8 100644 --- a/src/DbfDataReader/Cdx/CdxException.cs +++ b/src/DbfDataReader/Cdx/CdxException.cs @@ -4,6 +4,20 @@ namespace DbfDataReader.Cdx { public class CdxException : Exception { + public CdxException() + { + } + + public CdxException(string message) + : base(message) + { + } + + public CdxException(string message, Exception innerException) + : base(message, innerException) + { + } + public CdxException(CdxErrorCode code) : base($"Invalid CDX index file: {code}") { diff --git a/src/DbfDataReader/DbfFileFormatException.cs b/src/DbfDataReader/DbfFileFormatException.cs index 81ae857..5b485f0 100644 --- a/src/DbfDataReader/DbfFileFormatException.cs +++ b/src/DbfDataReader/DbfFileFormatException.cs @@ -4,5 +4,18 @@ namespace DbfDataReader { public class DbfFileFormatException : Exception { + public DbfFileFormatException() + { + } + + public DbfFileFormatException(string message) + : base(message) + { + } + + public DbfFileFormatException(string message, Exception innerException) + : base(message, innerException) + { + } } -} \ No newline at end of file +} diff --git a/src/DbfDataReader/Query/SqlParseException.cs b/src/DbfDataReader/Query/SqlParseException.cs index 719af70..589ee33 100644 --- a/src/DbfDataReader/Query/SqlParseException.cs +++ b/src/DbfDataReader/Query/SqlParseException.cs @@ -6,6 +6,20 @@ namespace DbfDataReader.Query // ArgumentException for invalid command text keep working public class SqlParseException : ArgumentException { + public SqlParseException() + { + } + + public SqlParseException(string message) + : base(message) + { + } + + public SqlParseException(string message, Exception innerException) + : base(message, innerException) + { + } + public SqlParseException(string message, int position) : base(message) { From f2012553ce4d1255e5abe90cfc8ed0bba1f15d5c Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 10:24:25 +0100 Subject: [PATCH 4/7] Add code quality hardening tracking plan Document the phased plan from the PR #285 review: Phase 0 (infrastructure, done), nullable annotation pass, analyzer cleanup, exception documentation, and enforcing warnings-as-errors in CI, with the current per-rule warning inventory so progress is measurable. Co-Authored-By: Claude Opus 4.8 --- docs/code-quality-hardening.md | 115 +++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 docs/code-quality-hardening.md diff --git a/docs/code-quality-hardening.md b/docs/code-quality-hardening.md new file mode 100644 index 0000000..ce0ac53 --- /dev/null +++ b/docs/code-quality-hardening.md @@ -0,0 +1,115 @@ +# Code Quality Hardening Plan + +Tracks the work from @daiplusplus's review on +[PR #285](https://github.com/yellowfeather/DbfDataReader/pull/285#issuecomment-4952711673): +enable nullable reference types, SDK-style multitargeting, exception +documentation, and full Roslyn/FxCop analysis for a library with downstream +consumers. + +**Status legend:** โœ… done ยท ๐Ÿšง in progress ยท โฌœ not started + +--- + +## Phase 0 โ€” Infrastructure (โœ… done) + +Groundwork so warnings surface without blocking the build. + +- [x] Add `Directory.Build.props` โ€” analyzers repo-wide (`AnalysisMode=All`, + `AnalysisLevel=latest`, `EnforceCodeStyleInBuild`); `Nullable=enable` and + `GenerateDocumentationFile=true` scoped to the `DbfDataReader` library. +- [x] Add `.editorconfig` โ€” formatting conventions; `CS1591` silenced (gradual + doc effort); style diagnostics as `suggestion`; `CA1707` silenced under + `test/**` (underscore test names are intentional). +- [x] Fix empty/incomplete exception classes for **CA1032** โ€” + `DbfFileFormatException`, `CdxException`, `SqlParseException` now expose the + standard constructors. +- [x] Confirm baseline: solution builds with **0 errors**; ~442 warnings on the + library (CA1032 cleared). + +Already in place before the review (no action needed): +- [x] SDK-style `.csproj` with multitargeting (`net10.0;netstandard2.1`). + +--- + +## Phase 1 โ€” Nullable reference types (โฌœ not started) + +~305 nullable warnings on the library, mostly on the public API surface. Do this +before the analyzer pass โ€” it collapses most of the CA1062 count. Work +file-group by file-group so each PR is reviewable. + +Warning inventory (per-build, `net10.0`): + +| Rule | Count | Meaning | +|------|-------|---------| +| CS8603 | 104 | Possible null reference return | +| CS8625 | 94 | Cannot convert null literal to non-nullable reference | +| CS8618 | 58 | Non-nullable field/property uninitialized on exit from ctor | +| CS8765 | 28 | Nullability of override/interface parameter mismatch | +| CS8604 | 20 | Possible null reference argument | +| CS8600 | 14 | Converting null literal or possible null to non-nullable | +| CS8601 | 6 | Possible null reference assignment | +| CS8602 | 2 | Dereference of a possibly null reference | + +Suggested order: +- [ ] Core read path โ€” `DbfTable`, `DbfRecord`, `DbfHeader`, `DbfColumn`, memo readers +- [ ] Value types โ€” `DbfValue*` +- [ ] ADO.NET surface โ€” `DbfDbConnection`, `DbfDbCommand`, `DbfDataReader`, parameter/connection-string types +- [ ] Query engine โ€” `Query/*` (parser, planner, translator, evaluator, readers) +- [ ] CDX index โ€” `Cdx/*` +- [ ] Remove now-redundant manual null-checks flagged as dead by CA1508 + +--- + +## Phase 2 โ€” Analyzer (CA) cleanup (โฌœ not started) + +~137 analyzer warnings. Expect CA1062 to shrink dramatically after Phase 1. +Group by theme; decide keep-vs-suppress per rule in `.editorconfig`. + +| Rule | Count | Theme | Likely action | +|------|-------|-------|---------------| +| CA1062 | 36 | Validate args non-null | Mostly resolved by nullable; `ThrowIfNull` the rest | +| CA1510/CA1512 | 20 | Use `ArgumentNullException.ThrowIfNull` / `ThrowIf*` | Mechanical fix | +| CA1307/CA1305 | 18 | Culture / `StringComparison` on string ops | Fix โ€” correctness for locale-sensitive data | +| CA1859 | 14 | Use concrete types for perf | Fix where hot; judgement call | +| CA1051 | 8 | Do not declare visible instance fields | Review | +| CA2249 | 6 | Prefer `string.Contains` over `IndexOf` | Mechanical | +| CA2201 | 6 | Do not raise reserved exception types (`IndexOutOfRangeException`) | Fix | +| CA1010 | 6 | Collections should implement generic interface | Review | +| CA2100 | 2 | Review SQL string for injection | Review/annotate | +| Others | ~30 | CA1720, CA1065, CA1002, CA2227, CA1870, CA1865, CA1861, CA1819, CA1724, CA1721, CA1508, CA1028, CA1008 | Triage individually | + +- [ ] Mechanical fixes (CA1510/CA1512/CA2249/CA1865) +- [ ] Correctness fixes (CA1307/CA1305/CA2201/CA2100) +- [ ] API/design review (CA1051/CA1002/CA1010/CA1819/CA2227/CA1724/CA1721) +- [ ] Perf fixes where they matter (CA1859/CA1861/CA1870) +- [ ] Record deliberate suppressions with justification in `.editorconfig` + +--- + +## Phase 3 โ€” Exception documentation (โฌœ not started) + +Recommendation #4: `/// ` comments so consumers know what to catch. +`GenerateDocumentationFile` is already on; `CS1591` is currently silenced. + +- [ ] Document thrown exceptions on all public entry points (constructors, + `Read`/`ReadAsync`, `Open`, query execution, connection-string parsing) +- [ ] Add `` docs to the public API +- [ ] Re-enable `CS1591` once the public surface is documented + +--- + +## Phase 4 โ€” Enforce in CI (โฌœ not started) + +Lock the gains in so regressions can't creep back. + +- [ ] Set `TreatWarningsAsErrors=true` for rules that are already clean + (per-rule via `.editorconfig`, or globally once Phases 1โ€“2 land) +- [ ] Confirm the CI build fails on new warnings + +--- + +## Out of scope / optional + +- **`readonly struct` refinement types** (recommendation #3) โ€” large effort, the + reviewer flagged it as tedious. Revisit as a separate initiative if desired, + e.g. for field offsets / record positions / encoding identifiers. From c7dcccb399da9ed77d92ef3a063a499688fe1ebc Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 11:14:40 +0100 Subject: [PATCH 5/7] Annotate core read path and value types for nullable First Phase 1 nullable pass. Make DbfValue.Value a T? so string values and the nullable value-type wrappers (int?, DateTime?, ...) are honestly typed, and flow that through IDbfValue (GetValue/GetFieldType) and DbfRecord's accessors. Mark the memo reference optional throughout (DbfTable.Memo, DbfValueMemo, DbfRecord.CreateDbfValue), the optional encoding/memoStream constructor parameters nullable, and ReadRecord's returns nullable. Resolve dispose-nulled fields (DbfTable.Stream, DbfMemo.BinaryReader) with null! rather than widening them, and initialise Init()-populated properties with null! since MemberNotNull is unavailable on netstandard2.1. BinaryReaderExtensions.ReadString now returns string?. These files are nullable-clean; net10.0 nullable warnings 326 -> 284. All 501 tests pass. Remaining warnings are analyzer (CA) items handled in Phase 2, plus nullable work in the ADO.NET/query/CDX layers. Co-Authored-By: Claude Opus 4.8 --- docs/code-quality-hardening.md | 10 ++++--- src/DbfDataReader/BinaryReaderExtensions.cs | 2 +- src/DbfDataReader/DbfColumn.cs | 5 ++-- src/DbfDataReader/DbfDataReaderOptions.cs | 2 +- src/DbfDataReader/DbfMemo.cs | 3 ++- src/DbfDataReader/DbfMemoFoxPro.cs | 2 +- src/DbfDataReader/DbfRecord.cs | 16 +++++------ src/DbfDataReader/DbfTable.cs | 30 ++++++++++----------- src/DbfDataReader/DbfValue.cs | 6 ++--- src/DbfDataReader/DbfValueMemo.cs | 4 +-- src/DbfDataReader/DbfValueNull.cs | 4 +-- src/DbfDataReader/IDbfValue.cs | 4 +-- 12 files changed, 46 insertions(+), 42 deletions(-) diff --git a/docs/code-quality-hardening.md b/docs/code-quality-hardening.md index ce0ac53..48c739b 100644 --- a/docs/code-quality-hardening.md +++ b/docs/code-quality-hardening.md @@ -31,12 +31,16 @@ Already in place before the review (no action needed): --- -## Phase 1 โ€” Nullable reference types (โฌœ not started) +## Phase 1 โ€” Nullable reference types (๐Ÿšง in progress) ~305 nullable warnings on the library, mostly on the public API surface. Do this before the analyzer pass โ€” it collapses most of the CA1062 count. Work file-group by file-group so each PR is reviewable. +Progress: core read path + value types annotated (net10.0 nullable warnings +326 โ†’ 284; those files are now nullable-clean). Remaining warnings live in the +ADO.NET surface, query engine, and CDX index. + Warning inventory (per-build, `net10.0`): | Rule | Count | Meaning | @@ -51,8 +55,8 @@ Warning inventory (per-build, `net10.0`): | CS8602 | 2 | Dereference of a possibly null reference | Suggested order: -- [ ] Core read path โ€” `DbfTable`, `DbfRecord`, `DbfHeader`, `DbfColumn`, memo readers -- [ ] Value types โ€” `DbfValue*` +- [x] Core read path โ€” `DbfTable`, `DbfRecord`, `DbfHeader`, `DbfColumn`, memo readers +- [x] Value types โ€” `DbfValue*` - [ ] ADO.NET surface โ€” `DbfDbConnection`, `DbfDbCommand`, `DbfDataReader`, parameter/connection-string types - [ ] Query engine โ€” `Query/*` (parser, planner, translator, evaluator, readers) - [ ] CDX index โ€” `Cdx/*` diff --git a/src/DbfDataReader/BinaryReaderExtensions.cs b/src/DbfDataReader/BinaryReaderExtensions.cs index 8fe4e5f..9510c8e 100644 --- a/src/DbfDataReader/BinaryReaderExtensions.cs +++ b/src/DbfDataReader/BinaryReaderExtensions.cs @@ -35,7 +35,7 @@ public static uint ReadBigEndianUInt32(this BinaryReader binaryReader) return BitConverter.ToUInt32(bytes, 0); } - public static string ReadString(this BinaryReader binaryReader, int fieldLength, Encoding encoding) + public static string? ReadString(this BinaryReader binaryReader, int fieldLength, Encoding encoding) { var chars = binaryReader.ReadBytes(fieldLength); if ((chars == null) || (chars.Length == 0)) diff --git a/src/DbfDataReader/DbfColumn.cs b/src/DbfDataReader/DbfColumn.cs index 4d53f92..e4a45cb 100644 --- a/src/DbfDataReader/DbfColumn.cs +++ b/src/DbfDataReader/DbfColumn.cs @@ -53,8 +53,9 @@ private void Read(ReadOnlySpan bytes, Encoding encoding) DecimalCount = decimalCount; } - DataType = GetDataType(ColumnType); - DataTypeName = DataType.ToString(); + var dataType = GetDataType(ColumnType); + DataType = dataType; + DataTypeName = dataType.ToString(); // skip the reserved bytes: // - Int16: reserved1 diff --git a/src/DbfDataReader/DbfDataReaderOptions.cs b/src/DbfDataReader/DbfDataReaderOptions.cs index 8fcef5d..a723a1b 100644 --- a/src/DbfDataReader/DbfDataReaderOptions.cs +++ b/src/DbfDataReader/DbfDataReaderOptions.cs @@ -14,7 +14,7 @@ public DbfDataReaderOptions() } public bool SkipDeletedRecords { get; set; } - public Encoding Encoding { get; set; } + public Encoding? Encoding { get; set; } public StringTrimmingOption StringTrimming { get; set; } public bool ReadFloatsAsDecimals { get; set; } diff --git a/src/DbfDataReader/DbfMemo.cs b/src/DbfDataReader/DbfMemo.cs index a2be71d..6591065 100644 --- a/src/DbfDataReader/DbfMemo.cs +++ b/src/DbfDataReader/DbfMemo.cs @@ -8,6 +8,7 @@ public abstract class DbfMemo : Disposable protected const int BlockHeaderSize = 8; protected const int DefaultBlockSize = 512; + // cleared to null! on Dispose; treated as non-null for the object's usable lifetime protected BinaryReader BinaryReader; protected DbfMemo(string path) @@ -54,7 +55,7 @@ protected override void Dispose(bool disposing) } finally { - BinaryReader = null; + BinaryReader = null!; } } diff --git a/src/DbfDataReader/DbfMemoFoxPro.cs b/src/DbfDataReader/DbfMemoFoxPro.cs index 0dfc6c0..59d764a 100644 --- a/src/DbfDataReader/DbfMemoFoxPro.cs +++ b/src/DbfDataReader/DbfMemoFoxPro.cs @@ -50,7 +50,7 @@ public override string BuildMemo(long startBlock) } value = value.TrimEnd(' '); } - return value; + return value ?? string.Empty; } } } diff --git a/src/DbfDataReader/DbfRecord.cs b/src/DbfDataReader/DbfRecord.cs index 49ad12b..3ba90fc 100644 --- a/src/DbfDataReader/DbfRecord.cs +++ b/src/DbfDataReader/DbfRecord.cs @@ -22,7 +22,7 @@ public class DbfRecord // column-subset parsing (issue #296): when enabled, an ordinal's value is only // readable after it has been parsed for the current row; the stamps guard // against exposing the previous row's content through the reused value objects - private int[] _parsedVersions; + private int[]? _parsedVersions; private int _rowVersion; public DbfRecord(DbfTable dbfTable) @@ -49,7 +49,7 @@ public DbfRecord(DbfTable dbfTable) public IList Values { get; set; } - private IDbfValue CreateDbfValue(DbfColumn dbfColumn, DbfMemo memo) + private IDbfValue CreateDbfValue(DbfColumn dbfColumn, DbfMemo? memo) { IDbfValue value; @@ -277,7 +277,7 @@ private void EnsureParsed(int ordinal) } } - public object GetValue(int ordinal) + public object? GetValue(int ordinal) { EnsureParsed(ordinal); var dbfValue = Values[ordinal]; @@ -330,7 +330,7 @@ private static T GetBoxedValue(IDbfValue dbfValue, int ordinal) catch (InvalidCastException) { throw new InvalidCastException( - $"Unable to cast object of type '{dbfValue.GetValue().GetType().FullName}' to type '{typeof(T).FullName}' at ordinal '{ordinal}'."); + $"Unable to cast object of type '{dbfValue.GetValue()?.GetType().FullName}' to type '{typeof(T).FullName}' at ordinal '{ordinal}'."); } } @@ -340,22 +340,22 @@ private static SqlNullValueException DataIsNull(int ordinal) $"Data is Null. This method or property cannot be called on Null values. Ordinal {ordinal}"); } - public string GetStringValue(int ordinal) + public string? GetStringValue(int ordinal) { EnsureParsed(ordinal); var dbfValue = Values[ordinal]; try { - return (string) dbfValue.GetValue(); + return (string?) dbfValue.GetValue(); } catch (InvalidCastException) { throw new InvalidCastException( - $"Unable to cast object of type '{dbfValue.GetValue().GetType().FullName}' to type '{typeof(string).FullName}' at ordinal '{ordinal}'."); + $"Unable to cast object of type '{dbfValue.GetValue()?.GetType().FullName}' to type '{typeof(string).FullName}' at ordinal '{ordinal}'."); } } - public Type GetFieldType(int ordinal) + public Type? GetFieldType(int ordinal) { var dbfValue = Values[ordinal]; return dbfValue.GetFieldType(); diff --git a/src/DbfDataReader/DbfTable.cs b/src/DbfDataReader/DbfTable.cs index 5e3f200..e31740e 100644 --- a/src/DbfDataReader/DbfTable.cs +++ b/src/DbfDataReader/DbfTable.cs @@ -17,62 +17,60 @@ public class DbfTable : Disposable // streams where the DBF content does not begin at offset zero private readonly long _startOffset; - public DbfTable(string path, Encoding encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false) + public DbfTable(string path, Encoding? encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false) { if (!File.Exists(path)) throw new FileNotFoundException(); Path = path; - CurrentEncoding = encoding; StringTrimming = stringTrimming; ReadFloatsAsDecimals = readFloatsAsDecimals; Stream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - Init(); + Init(encoding); var memoPath = MemoPath(); if (!string.IsNullOrEmpty(memoPath)) Memo = CreateMemo(memoPath); } - public DbfTable(Stream stream, Encoding encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false, bool leaveOpen = false) + public DbfTable(Stream stream, Encoding? encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false, bool leaveOpen = false) : this(stream, null, encoding, stringTrimming, readFloatsAsDecimals, leaveOpen) { } - public DbfTable(Stream stream, Stream memoStream, Encoding encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false, bool leaveOpen = false) + public DbfTable(Stream stream, Stream? memoStream, Encoding? encoding = null, StringTrimmingOption stringTrimming = StringTrimmingOption.Trim, bool readFloatsAsDecimals = false, bool leaveOpen = false) { Path = string.Empty; - CurrentEncoding = encoding; StringTrimming = stringTrimming; ReadFloatsAsDecimals = readFloatsAsDecimals; _leaveOpen = leaveOpen; Stream = stream; _startOffset = stream.CanSeek ? stream.Position : 0; - Init(); + Init(encoding); if (memoStream != null) Memo = CreateMemo(memoStream); } - private void Init() + private void Init(Encoding? encoding) { Header = new DbfHeader(Stream); - CurrentEncoding ??= EncodingProvider.GetEncoding(Header.LanguageDriver); + CurrentEncoding = encoding ?? EncodingProvider.GetEncoding(Header.LanguageDriver); Columns = ReadColumns(Stream); } public string Path { get; } - public Encoding CurrentEncoding { get; private set; } + public Encoding CurrentEncoding { get; private set; } = null!; - public DbfHeader Header { get; private set; } + public DbfHeader Header { get; private set; } = null!; public Stream Stream { get; private set; } - public DbfMemo Memo { get; private set; } + public DbfMemo? Memo { get; private set; } - public IList Columns { get; private set; } + public IList Columns { get; private set; } = null!; public StringTrimmingOption StringTrimming { get; private set; } @@ -97,7 +95,7 @@ protected override void Dispose(bool disposing) } finally { - Stream = null; + Stream = null!; Memo = null; } } @@ -183,13 +181,13 @@ public IList ReadColumns(Stream stream) return columns; } - public DbfRecord ReadRecord() + public DbfRecord? ReadRecord() { var dbfRecord = new DbfRecord(this); return !dbfRecord.Read(Stream) ? null : dbfRecord; } - public async ValueTask ReadRecordAsync(CancellationToken cancellationToken = default) + public async ValueTask ReadRecordAsync(CancellationToken cancellationToken = default) { var dbfRecord = new DbfRecord(this); return !await dbfRecord.ReadAsync(Stream, cancellationToken).ConfigureAwait(false) ? null : dbfRecord; diff --git a/src/DbfDataReader/DbfValue.cs b/src/DbfDataReader/DbfValue.cs index 8e17444..c3b9d7a 100644 --- a/src/DbfDataReader/DbfValue.cs +++ b/src/DbfDataReader/DbfValue.cs @@ -14,20 +14,20 @@ protected DbfValue(int offset, int length) public int Length { get; } - public T Value { get; protected set; } + public T? Value { get; protected set; } public bool IsNull => Value is null; public abstract void Read(ReadOnlySpan bytes); - public object GetValue() + public object? GetValue() { return Value; } public override string ToString() { - return Value == null ? string.Empty : Value.ToString(); + return Value?.ToString() ?? string.Empty; } public Type GetFieldType() diff --git a/src/DbfDataReader/DbfValueMemo.cs b/src/DbfDataReader/DbfValueMemo.cs index 185161a..de80145 100644 --- a/src/DbfDataReader/DbfValueMemo.cs +++ b/src/DbfDataReader/DbfValueMemo.cs @@ -5,9 +5,9 @@ namespace DbfDataReader { public class DbfValueMemo : DbfValueString { - private readonly DbfMemo _memo; + private readonly DbfMemo? _memo; - public DbfValueMemo(int start, int length, DbfMemo memo, Encoding encoding) + public DbfValueMemo(int start, int length, DbfMemo? memo, Encoding encoding) : base(start, length, encoding) { _memo = memo; diff --git a/src/DbfDataReader/DbfValueNull.cs b/src/DbfDataReader/DbfValueNull.cs index 97aafe0..8f40bda 100644 --- a/src/DbfDataReader/DbfValueNull.cs +++ b/src/DbfDataReader/DbfValueNull.cs @@ -16,7 +16,7 @@ public DbfValueNull(int start, int length) public bool IsNull => true; - public object GetValue() + public object? GetValue() { return null; } @@ -36,7 +36,7 @@ public override string ToString() return string.Empty; } - public Type GetFieldType() + public Type? GetFieldType() { return null; } diff --git a/src/DbfDataReader/IDbfValue.cs b/src/DbfDataReader/IDbfValue.cs index e3da005..f5b514f 100644 --- a/src/DbfDataReader/IDbfValue.cs +++ b/src/DbfDataReader/IDbfValue.cs @@ -12,8 +12,8 @@ public interface IDbfValue void Read(ReadOnlySpan bytes); - object GetValue(); + object? GetValue(); - Type GetFieldType(); + Type? GetFieldType(); } } \ No newline at end of file From 8afb680157857c9e273cdd1df619a9611a24ddad Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 11:17:38 +0100 Subject: [PATCH 6/7] =?UTF-8?q?Bumps=20NDbfReader=201.2.2=20=E2=86=92=202.?= =?UTF-8?q?4.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/DbfDataReader.Benchmarks/DbfDataReader.Benchmarks.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/DbfDataReader.Benchmarks/DbfDataReader.Benchmarks.csproj b/test/DbfDataReader.Benchmarks/DbfDataReader.Benchmarks.csproj index 6fdcc45..6f7248f 100644 --- a/test/DbfDataReader.Benchmarks/DbfDataReader.Benchmarks.csproj +++ b/test/DbfDataReader.Benchmarks/DbfDataReader.Benchmarks.csproj @@ -25,7 +25,7 @@ - + From 18beb849a390d60435f417f41c68c50b0349c804 Mon Sep 17 00:00:00 2001 From: Chris Richards Date: Mon, 13 Jul 2026 11:25:27 +0100 Subject: [PATCH 7/7] Annotate the ADO.NET reader surface for nullable Second Phase 1 nullable slice: DbfDataReader, DbfDbConnection, DbfDbParameter, DbfDbParameterCollection and SchemaTableBuilder are now nullable-clean. - DbfDbParameter.Value now matches the base object? (removing a CS8765 override mismatch); ParameterName/SourceColumn get [AllowNull] to mirror the base and are seeded to string.Empty. - DbfDbConnection.ConnectionString becomes an [AllowNull] property over a non-null backing field; the Query/QueryAsync/QueryFirstOrDefault params are object?, and QueryFirstOrDefault returns T?. - DbfDataReader keeps its DbDataReader overrides' declared non-null returns (GetValue/GetString/GetFieldType/GetName) via null-forgiving so the long-standing "null field -> null" behaviour is preserved, and the GetBytes/GetChars/buffer params match the base's nullable buffer. - Parameter-name normalisation flows string? with NotNullIfNotNull. DbfDbCommand and the connection-string builders are deferred to the query-engine slice, where their parameter-value nullability belongs. net10.0 warnings 401 -> 362; all 501 tests pass. Co-Authored-By: Claude Opus 4.8 --- docs/code-quality-hardening.md | 13 ++++++---- src/DbfDataReader/DbfDataReader.cs | 24 ++++++++++--------- src/DbfDataReader/DbfDbConnection.cs | 21 ++++++++++++---- src/DbfDataReader/DbfDbParameter.cs | 9 ++++--- src/DbfDataReader/DbfDbParameterCollection.cs | 8 ++++--- src/DbfDataReader/SchemaTableBuilder.cs | 2 +- 6 files changed, 50 insertions(+), 27 deletions(-) diff --git a/docs/code-quality-hardening.md b/docs/code-quality-hardening.md index 48c739b..453ff57 100644 --- a/docs/code-quality-hardening.md +++ b/docs/code-quality-hardening.md @@ -38,8 +38,13 @@ before the analyzer pass โ€” it collapses most of the CA1062 count. Work file-group by file-group so each PR is reviewable. Progress: core read path + value types annotated (net10.0 nullable warnings -326 โ†’ 284; those files are now nullable-clean). Remaining warnings live in the -ADO.NET surface, query engine, and CDX index. +326 โ†’ 284), then the ADO.NET reader surface (`DbfDataReader`, `DbfDbConnection`, +`DbfDbParameter`, `DbfDbParameterCollection`, `SchemaTableBuilder`) โ€” those files +are now nullable-clean. `DbfDbCommand` and the connection-string builders are +deferred to the query-engine slice: their parameter-value nullability threads +into `QueryPlanner`/`SqlExpressionEvaluator`, so they're best done together. +Remaining warnings live in the query engine, CDX index, `DbfDbCommand`, and the +connection-string builders. Warning inventory (per-build, `net10.0`): @@ -57,8 +62,8 @@ Warning inventory (per-build, `net10.0`): Suggested order: - [x] Core read path โ€” `DbfTable`, `DbfRecord`, `DbfHeader`, `DbfColumn`, memo readers - [x] Value types โ€” `DbfValue*` -- [ ] ADO.NET surface โ€” `DbfDbConnection`, `DbfDbCommand`, `DbfDataReader`, parameter/connection-string types -- [ ] Query engine โ€” `Query/*` (parser, planner, translator, evaluator, readers) +- [x] ADO.NET reader surface โ€” `DbfDataReader`, `DbfDbConnection`, `DbfDbParameter`, `DbfDbParameterCollection`, `SchemaTableBuilder` +- [ ] Query engine โ€” `Query/*` (parser, planner, translator, evaluator, readers) **+ `DbfDbCommand` + connection-string builders** (parameter-value nullability threads through here) - [ ] CDX index โ€” `Cdx/*` - [ ] Remove now-redundant manual null-checks flagged as dead by CA1508 diff --git a/src/DbfDataReader/DbfDataReader.cs b/src/DbfDataReader/DbfDataReader.cs index 06d9d81..0bfffdf 100644 --- a/src/DbfDataReader/DbfDataReader.cs +++ b/src/DbfDataReader/DbfDataReader.cs @@ -54,8 +54,8 @@ public override void Close() } finally { - DbfTable = null; - DbfRecord = null; + DbfTable = null!; + DbfRecord = null!; } } @@ -69,9 +69,9 @@ protected override void Dispose(bool disposing) } } - public DbfRecord ReadRecord() + public DbfRecord? ReadRecord() { - DbfRecord dbfRecord; + DbfRecord? dbfRecord; bool skip; do { @@ -105,7 +105,7 @@ public override byte GetByte(int ordinal) return DbfRecord.GetStructValue(ordinal); } - public override long GetBytes(int ordinal, long dataOffset, byte[] buffer, int bufferOffset, int length) + public override long GetBytes(int ordinal, long dataOffset, byte[]? buffer, int bufferOffset, int length) { throw new NotImplementedException(); } @@ -115,7 +115,7 @@ public override char GetChar(int ordinal) return DbfRecord.GetStructValue(ordinal); } - public override long GetChars(int ordinal, long dataOffset, char[] buffer, int bufferOffset, int length) + public override long GetChars(int ordinal, long dataOffset, char[]? buffer, int bufferOffset, int length) { throw new NotImplementedException(); } @@ -261,12 +261,14 @@ public override int GetValues(object[] values) public override object GetValue(int ordinal) { - return DbfRecord.GetValue(ordinal); + // DbDataReader declares a non-null return, but this reader has always + // surfaced a null field value as null rather than DBNull; preserve that. + return DbfRecord.GetValue(ordinal)!; } public override string GetString(int ordinal) { - return DbfRecord.GetStringValue(ordinal); + return DbfRecord.GetStringValue(ordinal)!; } public override int GetOrdinal(string name) @@ -291,7 +293,7 @@ public override int GetOrdinal(string name) public override string GetName(int ordinal) { var dbfColumn = DbfTable.Columns[ordinal]; - return dbfColumn.ColumnName; + return dbfColumn.ColumnName!; } public override long GetInt64(int ordinal) @@ -321,12 +323,12 @@ public override float GetFloat(int ordinal) public override Type GetFieldType(int ordinal) { - return DbfRecord.GetFieldType(ordinal); + return DbfRecord.GetFieldType(ordinal)!; } public ReadOnlyCollection GetColumnSchema() { - var columns = DbfTable.Columns.Select(c => c as DbColumn).ToList(); + var columns = DbfTable.Columns.Select(c => (DbColumn)c).ToList(); return columns.AsReadOnly(); } diff --git a/src/DbfDataReader/DbfDbConnection.cs b/src/DbfDataReader/DbfDbConnection.cs index 475e820..5f74cba 100644 --- a/src/DbfDataReader/DbfDbConnection.cs +++ b/src/DbfDataReader/DbfDbConnection.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Data; using System.Data.Common; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Text; using System.Threading; @@ -29,7 +30,17 @@ public DbfDbConnection(string dataSource, string serverVersion) public DbfDataReaderOptions Options { get; private set; } = new DbfDataReaderOptions(); - public override string ConnectionString { get; set; } + private string _connectionString = string.Empty; + + // base declares [AllowNull] on the setter; store a non-null value so the getter + // never returns null + [AllowNull] + public override string ConnectionString + { + get => _connectionString; + set => _connectionString = value ?? string.Empty; + } + public override string DataSource { get; } public override string ServerVersion { get; } @@ -104,7 +115,7 @@ protected override DbCommand CreateDbCommand() // Dapper-style typed queries: rows are mapped to T's settable properties by // column name (or to T itself for single-column scalar queries), and param's // properties become named parameters - public List Query(string sql, object param = null) + public List Query(string sql, object? param = null) { using (var command = CreateTypedCommand(sql, param)) using (var reader = command.ExecuteReader()) @@ -123,7 +134,7 @@ public List Query(string sql, object param = null) } } - public async Task> QueryAsync(string sql, object param = null, + public async Task> QueryAsync(string sql, object? param = null, CancellationToken cancellationToken = default) { using (var command = CreateTypedCommand(sql, param)) @@ -143,7 +154,7 @@ public async Task> QueryAsync(string sql, object param = null, } } - public T QueryFirstOrDefault(string sql, object param = null) + public T? QueryFirstOrDefault(string sql, object? param = null) { using (var command = CreateTypedCommand(sql, param)) using (var reader = command.ExecuteReader()) @@ -158,7 +169,7 @@ public T QueryFirstOrDefault(string sql, object param = null) } } - private DbfDbCommand CreateTypedCommand(string sql, object param) + private DbfDbCommand CreateTypedCommand(string sql, object? param) { var command = (DbfDbCommand)CreateCommand(); command.CommandText = sql; diff --git a/src/DbfDataReader/DbfDbParameter.cs b/src/DbfDataReader/DbfDbParameter.cs index 9569a10..afddd21 100644 --- a/src/DbfDataReader/DbfDbParameter.cs +++ b/src/DbfDataReader/DbfDbParameter.cs @@ -1,5 +1,6 @@ using System.Data; using System.Data.Common; +using System.Diagnostics.CodeAnalysis; namespace DbfDataReader { @@ -11,15 +12,17 @@ public class DbfDbParameter : DbParameter public override bool IsNullable { get; set; } - public override string ParameterName { get; set; } + [AllowNull] + public override string ParameterName { get; set; } = string.Empty; public override int Size { get; set; } - public override string SourceColumn { get; set; } + [AllowNull] + public override string SourceColumn { get; set; } = string.Empty; public override bool SourceColumnNullMapping { get; set; } - public override object Value { get; set; } + public override object? Value { get; set; } public override void ResetDbType() { diff --git a/src/DbfDataReader/DbfDbParameterCollection.cs b/src/DbfDataReader/DbfDbParameterCollection.cs index c75c4de..fba42e7 100644 --- a/src/DbfDataReader/DbfDbParameterCollection.cs +++ b/src/DbfDataReader/DbfDbParameterCollection.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Data.Common; +using System.Diagnostics.CodeAnalysis; namespace DbfDataReader { @@ -13,7 +14,7 @@ public class DbfDbParameterCollection : DbParameterCollection public override object SyncRoot => ((ICollection)_parameters).SyncRoot; - public DbfDbParameter AddWithValue(string parameterName, object value) + public DbfDbParameter AddWithValue(string parameterName, object? value) { var parameter = new DbfDbParameter { ParameterName = parameterName, Value = value }; _parameters.Add(parameter); @@ -127,12 +128,13 @@ protected override void SetParameter(string parameterName, DbParameter value) } // parameter names match case-insensitively, with or without a leading '@' - private static bool NamesEqual(string x, string y) + private static bool NamesEqual(string? x, string? y) { return string.Equals(Normalize(x), Normalize(y), StringComparison.OrdinalIgnoreCase); } - internal static string Normalize(string parameterName) + [return: NotNullIfNotNull(nameof(parameterName))] + internal static string? Normalize(string? parameterName) { return parameterName != null && parameterName.StartsWith("@", StringComparison.Ordinal) ? parameterName.Substring(1) diff --git a/src/DbfDataReader/SchemaTableBuilder.cs b/src/DbfDataReader/SchemaTableBuilder.cs index e0773c8..d06b6d1 100644 --- a/src/DbfDataReader/SchemaTableBuilder.cs +++ b/src/DbfDataReader/SchemaTableBuilder.cs @@ -40,7 +40,7 @@ public static DataTable Build(IReadOnlyList columnSchema) foreach (var column in columnSchema) { var row = table.NewRow(); - row[0] = column.ColumnName; + row[0] = column.ColumnName ?? dbNull; row[1] = column.ColumnOrdinal ?? dbNull; row[2] = column.ColumnSize ?? dbNull; row[3] = column.NumericPrecision ?? dbNull;