fix: clarify operator grouping and resolve an ambiguous path-string overload#80
Merged
Merged
Conversation
…verload Three nested bitwise/arithmetic expressions (IPv4 octet packing, and Punycode's generalized-integer encoder and bias adaptation) relied on operator precedence alone with no visual grouping cue. Extract the higher-precedence sub-expression into a named local in each case so the intent is unambiguous to both readers and static analysis. Also give ComponentPath.Segments' path-string encoder a distinct name (toSegmentsPathString) instead of overloading it with ComponentPath's dispatcher under the same toUriPathString name. Kotlin resolves extension overloads by the receiver's static type, so keeping the name shared invited a future caller with a ComponentPath-typed (rather than smart-cast Segments-typed) receiver to silently land on the general dispatcher instead of the specific encoder. Pure refactors: no behavior change, no test updates needed.
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.
Summary
Three arithmetic/bitwise expressions leaned on operator precedence alone with no visual grouping cue -- IPv4 octet packing, and Punycode's generalized-integer encoder and bias adaptation. Each nests two operators of different precedence on one line, which reads ambiguously and is exactly the shape CodeQL's "whitespace contradicts operator precedence" check flags. Extracting the higher-precedence sub-expression into a named local fixes the readability issue directly, with identical arithmetic.
Separately,
ComponentPath.SegmentsandComponentPatheach declared an extension function namedtoUriPathString, differing only by receiver type. Kotlin resolves extension overloads by the receiver's static type, not its runtime type, so a caller holding aComponentPath-typed reference to aSegmentsvalue (without a smart cast) always lands on the general dispatcher, never the specific encoder -- correct today only because the dispatcher'swhenbranch calls back into the specific one deliberately. Renaming theSegments-specific one totoSegmentsPathStringremoves the overload entirely so dispatch can no longer depend on which static type a future caller happens to have.All four are pure refactors -- no behavior change, no test updates needed.
Test plan
./gradlew :kuri:jvmTest :kuri:ktlintCheck :kuri:detekt :kuri:apiCheck-- all green, no public API surface change