Code quality hardening: nullable, analyzers, exception constructors#310
Open
chrisrichards wants to merge 7 commits into
Open
Code quality hardening: nullable, analyzers, exception constructors#310chrisrichards wants to merge 7 commits into
chrisrichards wants to merge 7 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
First Phase 1 nullable pass. Make DbfValue<T>.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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
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
1 participant
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.
Acts on @daiplusplus's review recommendations from #285. This is Phase 0 (infrastructure) — it turns the analysis on and clears the one issue the review called out by name, without attempting the full backlog yet. Tracking plan is included in
docs/code-quality-hardening.md.What's here (one commit per task)
Directory.Build.props— Roslyn/FxCop-style analysis repo-wide (AnalysisMode=All,AnalysisLevel=latest,EnforceCodeStyleInBuild);Nullable=enableandGenerateDocumentationFilescoped to theDbfDataReaderlibrary so test/benchmark projects stay quiet. (recs system.ArgumentException #1, Why not make a DBFConnection and command ? #2, I solved datetime missing lines #5).editorconfig— keeps the new analysis manageable while the backlog is triaged:CS1591silenced (docs are a gradual effort), style diagnostics assuggestion,CA1707silenced undertest/**(underscore test names are intentional).DbfFileFormatException(was empty),CdxException, andSqlParseExceptionnow expose the parameterless /(string)/(string, Exception)constructors. Clears the empty-exception issue the review flagged.docs/code-quality-hardening.md, phased with per-rule warning inventory.Status
Not in this PR (later phases, see the plan)
/// <exception>docs + re-enable CS1591TreatWarningsAsErrorsin CIreadonly structrefinement types (rec #3) are out of scope / optional.🤖 Generated with Claude Code