refactor(decode): drop unused accepts(DType) from EncodingDecoder SPI#142
Merged
Conversation
accepts(DType) was a residual of the ADR-0001 read/write runtime split: the
pre-split unified Encoding interface combined encode + accepts + decode, and
the split copied the full method set onto both EncodingDecoder and
EncodingEncoder. accepts is encode-selection semantics ("can this encoding
*encode* the dtype") — the writer's CascadingCompressor and friends still use
EncodingEncoder.accepts, but the reader dispatches decoders purely by
EncodingId (ReadRegistry's Map<EncodingId, EncodingDecoder>) and never calls
it. The method has been dead on the read side since the split; SonarCloud
flagged it as uncovered on BitpackedEncodingDecoder.
Remove accepts from the EncodingDecoder interface and all 33 implementations,
plus the decoder-side accepts assertions in the unit tests (the writer
encoder.accepts assertions stay). Added a DictEncodingDecoderTest#encodingId
test to keep that outer class from collapsing to a checkstyle utility class
once its only non-nested test was removed.
EncodingEncoder.accepts is unchanged.
Verified: full unit suite (all modules) green; reader/writer build clean
(javac -Werror, checkstyle, javadoc).
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
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.
What
EncodingDecoder.accepts(DType)is a residual of the ADR-0001 read/write runtime split. The pre-split unifiedEncodinginterface combinedencode+accepts+decode; the split copied the full method set onto bothEncodingDecoderandEncodingEncoder.acceptsis encode-selection semantics — its javadoc literally said "can this encoding encode the dtype". The writer still uses it (CascadingCompressor,StructEncodingEncoder, …), but the reader dispatches decoders purely byEncodingId(ReadRegistry'sMap<EncodingId, EncodingDecoder>) and never callsaccepts. It has been dead on the read side since the split — which is why SonarCloud flagged it uncovered onBitpackedEncodingDecoder.This is the dead-code (not missing-test) branch of the triage just documented in
docs/testing.md: delete the clause, don't write a test for it.Changes
acceptsfrom theEncodingDecoderinterface and all 33 implementations.acceptsassertions from unit tests. The writerEncodingEncoder.acceptsassertions stay (that method is live).DictEncodingDecoderTest#encodingId_isVortexDictso that outer class keeps a non-nested instance test (removing its only one tripped checkstyle's utility-class rule) — genuine coverage, matches siblings.EncodingEncoder.acceptsis unchanged.Net: 42 files, −265/+3.
Note on the SPI
EncodingDecoderis a public SPI. Dropping a method is technically a source-breaking change for any downstream custom decoder, but the method was never invoked by the read path — implementers were forced to write a body that nothing called.Verification
javac -Werror, Checkstyle, Javadoc)🤖 Generated with Claude Code