Skip to content

Move write state types out of generic converters and rename#6572

Merged
NinoFloris merged 2 commits into
mainfrom
writestate-changes
May 13, 2026
Merged

Move write state types out of generic converters and rename#6572
NinoFloris merged 2 commits into
mainfrom
writestate-changes

Conversation

@NinoFloris
Copy link
Copy Markdown
Member

Should reduce generic duplication a bit, and with file types we don't give up localized visibility either.

Copilot AI review requested due to automatic review settings May 13, 2026 12:34
@NinoFloris NinoFloris requested review from roji and vonzshik as code owners May 13, 2026 12:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors converter write-state handling by moving write-state wrapper types out of generic/nested converter types and renaming the late-bound object converter/provider to “LateBinding…”, aiming to reduce generic duplication while keeping visibility localized via file types.

Changes:

  • Rename ObjectConverter/LateBoundTypeInfoProvider to LateBindingConverter/LateBindingTypeInfoProvider and update references/comments.
  • Extract/rename several write-state wrapper types into file-scoped types (e.g., range/composite/multirange/hstore).
  • Update tests and internal comments to reflect the new late-binding naming and write-state flow.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Npgsql.Tests/WriteStateTests.cs Updates late-binding naming in test comments; documents write-state flow.
src/Npgsql/NpgsqlParameter.cs Updates internal comment to reference LateBindingConverter.
src/Npgsql/Internal/TypeInfoMapping.cs Updates internal comment to reference LateBindingTypeInfoProvider.
src/Npgsql/Internal/ResolverFactories/AdoTypeInfoResolverFactory.cs Updates object-array provider mapping to instantiate LateBindingTypeInfoProvider.
src/Npgsql/Internal/Converters/RangeConverter.cs Renames/extracts range write-state wrapper to file-scoped RangeWriteState.
src/Npgsql/Internal/Converters/MultirangeConverter.cs Renames/extracts multirange write-state wrapper type (currently introduced with invalid syntax).
src/Npgsql/Internal/Converters/LateBindingConverter.cs Renames converter/provider and extracts write-state wrapper to file-scoped LateBindingWriteState.
src/Npgsql/Internal/Converters/HstoreConverter.cs Renames/extracts hstore write-state wrapper type (currently introduced with invalid syntax).
src/Npgsql/Internal/Converters/CompositeConverter.cs Renames/extracts composite field/write-state wrappers to file-scoped types.
Comments suppressed due to low confidence (2)

src/Npgsql/Internal/Converters/HstoreConverter.cs:156

  • file sealed class HstoreWriteState : MultiWriteState; is not valid C# syntax for a class declaration (classes require a body). This will fail compilation; use an explicit empty body (e.g. { }) or another valid type form (e.g. a record if appropriate).
}

file sealed class HstoreWriteState : MultiWriteState;

src/Npgsql/Internal/Converters/MultirangeConverter.cs:146

  • file sealed class MultirangeWriteState : MultiWriteState; appears to be an invalid class declaration (missing body) and will not compile. If the intent is an empty derived type just for distinguishing write state, declare it with an empty body (e.g. { }).
}

file sealed class MultirangeWriteState : MultiWriteState;


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/Npgsql.Tests/WriteStateTests.cs Outdated
@NinoFloris NinoFloris merged commit ca4eea3 into main May 13, 2026
17 checks passed
@NinoFloris NinoFloris deleted the writestate-changes branch May 13, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants