✨ add key-aware decoding to the query string parser#35
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds DecodeKind enum and integrates key/value-aware decoding into the pipeline. Updates DecodeOptions to support flexible decoder signatures with kind and charset, adjusts decode control flow to pass DecodeKind, broadens dot-decoding in keys, gates list-guard by input type, expands public exports, updates tests, and fixes Makefile target. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant QS as QS.decode
participant P as Parser (extensions/decode)
participant DO as DecodeOptions
participant UD as Utils.decode (fallback)
C->>QS: decode(input, options)
alt input is String
QS->>P: parse query string
else input is Map/other
QS->>P: use pre-tokenised input
end
rect rgba(200,240,255,0.3)
note over P,DO: Key parsing path
P->>DO: decoder(keyPart, charset, kind=key)
alt Custom decoder matches
DO-->>P: decoded key
else Fallbacks fail
DO->>UD: decode(value, charset)
UD-->>DO: decoded key
DO-->>P: decoded key
end
end
rect rgba(220,255,220,0.3)
note over P,DO: Value parsing path
P->>DO: decoder(valPart, charset, kind=value)
alt Custom decoder matches
DO-->>P: decoded value
else Fallbacks fail
DO->>UD: decode(value, charset)
UD-->>DO: decoded value
DO-->>P: decoded value
end
end
P-->>QS: assembled map
QS-->>C: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
techouse
changed the title
techouse
added
enhancement
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #35 +/- ##
==========================================
- Coverage 95.33% 94.84% -0.50%
==========================================
Files 14 14
Lines 815 834 +19
==========================================
+ Hits 777 791 +14
- Misses 38 43 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
Makefile (1)
40-40: Use $(MAKE) when invoking sub-makesThis preserves MAKEFLAGS (e.g., -j, --silent) and is the recommended pattern in Makefiles.
Apply this diff:
- make check_style && make test + $(MAKE) check_style && $(MAKE) testlib/src/enums/decode_kind.dart (1)
15-20: Doc example: import via the public APISince DecodeKind is re-exported, prefer showing imports from the public surface to guide users.
Apply this diff to the doc snippet:
-/// ```dart -/// import 'decode_kind.dart'; -/// -/// DecodeKind k = DecodeKind.key; // decode a key/segment -/// DecodeKind v = DecodeKind.value; // decode a value -/// ``` +/// ```dart +/// import 'package:qs_dart/qs.dart'; // public re-export +/// +/// final DecodeKind k = DecodeKind.key; // decode a key/segment +/// final DecodeKind v = DecodeKind.value; // decode a value +/// ```lib/src/extensions/decode.dart (1)
260-266: Case-insensitive dot decoding: simplify with a single regexMinor readability/perf nit: replace the double replaceAll with a single case-insensitive pattern.
Apply this diff:
- final String decodedRoot = options.decodeDotInKeys - ? cleanRoot.replaceAll('%2E', '.').replaceAll('%2e', '.') - : cleanRoot; + final String decodedRoot = options.decodeDotInKeys + ? cleanRoot.replaceAll(RegExp(r'%2E', caseSensitive: false), '.') + : cleanRoot;lib/src/models/decode_options.dart (2)
72-74: Simplify defaulting logic for allowDots; remove redundant|| falseThe current expression is correct but unnecessarily complex and could be misread due to operator precedence. Make it explicit and concise.
Apply this diff:
- }) : allowDots = allowDots ?? decodeDotInKeys == true || false, + }) : allowDots = allowDots ?? (decodeDotInKeys ?? false),
165-213: Do not swallow user decoder exceptions in the dynamic path; catch only call-shape errorsThe dynamic ladder currently catches all exceptions, which risks hiding genuine errors thrown by user-provided decoders. Restrict catches to NoSuchMethodError/TypeError (mismatched named args/required named param) and let other exceptions propagate. Behaviour in the strongly-typed branches already propagates exceptions.
Apply this diff to narrow the catches:
- // Dynamic callable or class with `call` method - try { - // Try full shape (value, {charset, kind}) - return (d as dynamic)(value, charset: charset, kind: kind); - } catch (_) { - try { - // Try (value, {charset}) - return (d as dynamic)(value, charset: charset); - } catch (_) { - try { - // Try (value, {kind}) - return (d as dynamic)(value, kind: kind); - } catch (_) { - try { - // Try (value) - return (d as dynamic)(value); - } catch (_) { - // Fallback to default - return Utils.decode(value, charset: charset); - } - } - } - } + // Dynamic callable or class with `call` method + try { + // Try full shape (value, {charset, kind}) + return (d as dynamic)(value, charset: charset, kind: kind); + } on NoSuchMethodError catch (_) { + // fall through + } on TypeError catch (_) { + // fall through + } + try { + // Try (value, {charset}) + return (d as dynamic)(value, charset: charset); + } on NoSuchMethodError catch (_) { + // fall through + } on TypeError catch (_) { + // fall through + } + try { + // Try (value, {kind}) + return (d as dynamic)(value, kind: kind); + } on NoSuchMethodError catch (_) { + // fall through + } on TypeError catch (_) { + // fall through + } + try { + // Try (value) + return (d as dynamic)(value); + } on NoSuchMethodError catch (_) { + // Fallback to default + return Utils.decode(value, charset: charset); + } on TypeError catch (_) { + // Fallback to default + return Utils.decode(value, charset: charset); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
Makefile(1 hunks)lib/qs_dart.dart(1 hunks)lib/src/enums/decode_kind.dart(1 hunks)lib/src/extensions/decode.dart(3 hunks)lib/src/models/decode_options.dart(4 hunks)lib/src/qs.dart(3 hunks)test/unit/decode_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (23)
lib/src/enums/decode_kind.dart (1)
27-36: Enum introduction looks goodClear separation between key and value decoding contexts; the inline docs convey the intended behaviour well.
lib/src/qs.dart (2)
16-18: Re-exporting DecodeKind from QS is a good DX improvementThis lets consumers stick to a single import path. No issues spotted.
71-79: Guardrail now applies only to raw string inputs — aligns with option isolationGreat tweak: disabling list parsing based on parameter count only for String inputs avoids surprising behaviour when a pre-tokenised map is supplied.
lib/qs_dart.dart (1)
22-22: Exporting DecodeKind on the package’s public surface is appropriateMatches the re-export in QS and provides consistent access. Looks good.
lib/src/extensions/decode.dart (1)
148-169: Key-aware decoding is correctly plumbedPassing DecodeKind.key for keys and DecodeKind.value for values is the right cut. Also good to decode bare keys with key kind. Ensure DecodeOptions.decoder supports both legacy and new signatures as per PR description.
Would you like me to scan the codebase to confirm DecodeOptions.decoder adapts legacy callables? I can provide a ripgrep script to locate its declaration and usages to double-check signature compatibility.
test/unit/decode_test.dart (15)
2018-2035: Solid coverage: key-aware decoder receives DecodeKind for keys and valuesGood assertion of KEY/ VALUE order; verifies both key and value paths are routed through the decoder with the right kind and that defaults are preserved.
2037-2041: Backwards compatibility preserved for legacy single-arg decodersNice check that classic (value)-only decoders still work and affect both keys and values.
2042-2047: Callable with onlykindnamed arg supportedGood demonstration that a decoder accepting only
DecodeKindcan modify keys while leaving values intact.
2049-2063: Option isolation verified across calls under list guardrailThis ensures any internal guardrail changes don’t mutate the provided options. Thanks for targeting the string-input-only path.
2066-2077: Bare key decoding passes KEY to decoder with strictNullHandlingGood to see the decoder is not invoked for a value in this case and that null is preserved.
2079-2092: Comma splitting emits VALUE per segment, in correct orderOrder-sensitive assertion is helpful for users writing stateful decoders.
2094-2105: Selective key mutation scenario well coveredThis guards against regressions where values might be accidentally passed as KEY or vice versa.
2107-2115: Decoder returning null for VALUE path is respectedEnsures downstream code doesn’t coerce null to empty string unexpectedly.
2117-2124: Decoder not invoked for Map inputGood safety: bypassing the decoder for already-parsed inputs avoids surprising behaviour.
2126-2141: Duplicates path exercises KEY/VALUE sequence twiceNice way to validate per-pair invocation order with the default combine strategy.
2143-2158: Charset sentinel observed by decoder viacharsetparameterGreat to have explicit observation coverage for the charset toggling seen by decoders.
2160-2173: parseLists=false still routes KEY and VALUE correctlyValidates that list parsing choice doesn’t affect decoder kind semantics.
2176-2187: Dynamic fallback: mismatched named params uses value-only invocationThe expectations and the call sink verify that both key and value go through the simplified call. This is the right behaviour for loosely-typed callables.
2189-2195: Dynamic fallback: required named param forces Utils.decodeGood negative path; ensures we do not attempt to satisfy incompatible required named parameters and instead gracefully fall back.
2198-2216: Helper callables (_Loose1, _Loose2) are minimal and purposefulClear intent and comments; they precisely exercise the dynamic invocation ladder.
lib/src/models/decode_options.dart (3)
31-37: New Decoder typedef with key-awareness is clear and future-proofThe unified typedef that includes optional charset and kind aligns with the API expansion and keeps implementations ergonomic.
38-50: Compat typedefs (V1–V4) cover legacy and niche callablesGood call preserving backwards compatibility while enabling progressive enhancement.
232-253: copyWith handles decoder correctly and keeps existing value by defaultGood ergonomics; passing through the existing _decoder unless overridden matches expectations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/src/models/decode_options.dart (2)
31-50: Canonicalise decoder typedefs; avoid duplicated shapes and check the canonical aliasDecoder and DecoderV3 share the exact same signature. Keeping both as independent typedefs risks future drift in comments or parameters. Alias DecoderV3 to Decoder and switch the type check to Decoder to make the “preferred” name authoritative.
Apply:
-/// Full-featured: decoder with [charset] and key/value [kind]. -typedef DecoderV3 = dynamic Function(String? value, - {Encoding? charset, DecodeKind? kind}); +/// Full-featured: decoder with [charset] and key/value [kind]. +typedef DecoderV3 = Decoder;And prefer the canonical alias in the dispatch:
- if (d is DecoderV3) { + if (d is Decoder) { return d(value, charset: charset, kind: kind); }Also applies to: 176-186
165-224: Default to options.charset when none is providedWhen falling back to Utils.decode, charset may be null. Use charset ?? this.charset to maintain consistent decoding per options. This also protects dynamic-call fallbacks that end in the default path.
Apply:
- return Utils.decode(value, charset: charset); + return Utils.decode(value, charset: charset ?? this.charset); @@ - // Fallback to default - return Utils.decode(value, charset: charset); + // Fallback to default + return Utils.decode(value, charset: charset ?? this.charset); @@ - // Fallback to default - return Utils.decode(value, charset: charset); + // Fallback to default + return Utils.decode(value, charset: charset ?? this.charset);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/src/enums/decode_kind.dart(1 hunks)lib/src/models/decode_options.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/src/enums/decode_kind.dart
🔇 Additional comments (5)
lib/src/models/decode_options.dart (5)
55-56: Widened decoder type to Object? enables callable objects—good choiceSwitching the decoder field/ctor/copyWith to Object? to support callable objects (instances with call) matches the PR goal and is idiomatic given the dynamic fallback below. Including _decoder in Equatable props is appropriate.
Also applies to: 72-75, 163-164, 288-306
72-72: Nicely enforces allowDots when decodeDotInKeys is enabledInitialising allowDots to allowDots ?? (decodeDotInKeys ?? false) guarantees dot-notation parsing whenever decodeDotInKeys is true. This aligns with the documented behaviour and avoids invalid states.
165-224: Dispatch order and exception narrowness look pragmaticPrioritising strongly typed decoders (V3/V2/V4/V1) before dynamic invocation is sensible and avoids unnecessary try/catch. Catching NoSuchMethodError and TypeError for call-shape mismatches while letting user-code exceptions bubble up is a good default.
244-265: copyWith mirrors the constructor changes correctlyThe copyWith signature and decoder propagation (decoder ?? _decoder) are consistent with the widened type and preserve existing decoders by default.
4-4: Import path is correct
- pubspec.yaml declares the package name as qs_dart.
- All imports use package:qs_dart/…, and no imports of package:qs/ were found.
No changes required.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/models/decode_options.dart (1)
115-120: Docs contradict themselves aboutdecodeDotInKeysvsallowDots.Earlier docs (Lines 79–83) say
allowDotsis implied and will be treated as true even if passed false. Here, this section says such a combination is invalid and will error. Pick one. Based on the PR summary, implication (no error) is intended.Apply this diff to reconcile:
/// Decode dots that appear in *keys* (e.g., `a.b=c`). /// - /// This explicitly opts into dot‑notation handling and implies [allowDots]. - /// Setting [decodeDotInKeys] to `true` while forcing [allowDots] to `false` - /// is invalid and will cause an error in [QS.decode]. + /// This explicitly opts into dot‑notation handling and implies [allowDots]. + /// If you pass `allowDots: false` alongside `decodeDotInKeys: true`, + /// `allowDots` will still be treated as `true`.
🧹 Nitpick comments (5)
lib/src/models/decode_options.dart (5)
31-40: Consider makingkindnon-nullable in the preferredDecodersignature.As this is the “preferred” signature, a non-nullable
DecodeKind kindbetter signals intent and reduces null-handling inside custom decoders. The defaulting still happens at call sites.Apply this diff:
-typedef Decoder = dynamic Function( - String? value, { - Encoding? charset, - DecodeKind? kind, -}); +typedef Decoder = dynamic Function( + String? value, { + Encoding? charset, + DecodeKind kind, +});
54-54: Narrow thedecoderoption toFunction?to prevent silent misconfiguration.Typing this parameter as
Function?(instead ofObject?) still allows callable classes (call) but catches accidental non-callables (e.g., integers) at compile time rather than silently falling back via exceptions.Apply this diff:
- Object? decoder, + Function? decoder,
160-163: PreferFunction?for_decoderfield.Same rationale as the constructor: keeps the API flexible for callable objects while preventing obvious misconfigurations.
Apply this diff:
- final Object? _decoder; + final Function? _decoder;
164-220: Avoid name shadowing and tighten the dynamic fallback path.The local variable
decodershadows the instance method namedecoder(...), which harms readability. Rename it. Also, the dynamic fallback makes multiple calls guarded by exceptions; that’s acceptable, but consider normalising the decoder once (see follow-up) to avoid per-call exception-driven control flow.Apply this diff to remove shadowing:
dynamic decoder( String? value, { Encoding? charset, DecodeKind kind = DecodeKind.value, }) { - final Object? decoder = _decoder; + final Object? fn = _decoder; - if (decoder == null) { + if (fn == null) { return Utils.decode(value, charset: charset ?? this.charset); } // Prefer strongly-typed variants first - if (decoder is Decoder) return decoder(value, charset: charset, kind: kind); - if (decoder is Decoder1) return decoder(value, charset: charset); - if (decoder is Decoder2) return decoder(value, kind: kind); - if (decoder is Decoder3) return decoder(value); + if (fn is Decoder) return fn(value, charset: charset, kind: kind); + if (fn is Decoder1) return fn(value, charset: charset); + if (fn is Decoder2) return fn(value, kind: kind); + if (fn is Decoder3) return fn(value); // Dynamic callable or class with `call` method try { // Try full shape (value, {charset, kind}) - return (decoder as dynamic)(value, charset: charset, kind: kind); + return (fn as dynamic)(value, charset: charset, kind: kind); } on NoSuchMethodError catch (_) { // fall through } on TypeError catch (_) { // fall through } try { // Try (value, {charset}) - return (decoder as dynamic)(value, charset: charset); + return (fn as dynamic)(value, charset: charset); } on NoSuchMethodError catch (_) { // fall through } on TypeError catch (_) { // fall through } try { // Try (value, {kind}) - return (decoder as dynamic)(value, kind: kind); + return (fn as dynamic)(value, kind: kind); } on NoSuchMethodError catch (_) { // fall through } on TypeError catch (_) { // fall through } try { // Try (value) - return (decoder as dynamic)(value); + return (fn as dynamic)(value); } on NoSuchMethodError catch (_) { // Fallback to default return Utils.decode(value, charset: charset ?? this.charset); } on TypeError catch (_) { // Fallback to default return Utils.decode(value, charset: charset ?? this.charset); } }As a follow-up, consider normalising the decoder at construction time to a single
Decoderwrapper. This removes the need for repeated type checks and exception-driven fallbacks on every scalar decode:// Example approach (outside the diffed range): final class DecodeOptions with EquatableMixin { final Decoder _resolve; // always non-null, wraps Utils.decode by default DecodeOptions({ Function? decoder, /* ... */ }) : _resolve = _normalise(decoder), /* ... */ ; static Decoder _normalise(Function? f) { if (f == null) { return (v, {charset, kind = DecodeKind.value}) => Utils.decode(v, charset: charset ?? utf8); } if (f is Decoder) { return (v, {charset, kind = DecodeKind.value}) => f(v, charset: charset, kind: kind); } if (f is Decoder1) { return (v, {charset, kind = DecodeKind.value}) => f(v, charset: charset); } if (f is Decoder2) { return (v, {charset, kind = DecodeKind.value}) => f(v, kind: kind); } if (f is Decoder3) { return (v, {charset, kind = DecodeKind.value}) => f(v); } // Dynamic fallback once: return (v, {charset, kind = DecodeKind.value}) { try { return (f as dynamic)(v, charset: charset, kind: kind); } catch (_) { try { return (f as dynamic)(v, charset: charset); } catch (_) {} try { return (f as dynamic)(v, kind: kind); } catch (_) {} try { return (f as dynamic)(v); } catch (_) {} return Utils.decode(v, charset: charset ?? utf8); } }; } dynamic decoder(String? value, {Encoding? charset, DecodeKind kind = DecodeKind.value}) => _resolve(value, charset: charset, kind: kind); }
240-261: Type ofdecoderincopyWith+ ability to clear the decoder.
- Mirror the constructor change and narrow to
Function?.- Currently,
copyWith(decoder: null)cannot clear a previously set decoder becausedecoder ?? _decoderretains the old one. Consider a sentinel to allow explicit clearing if needed.Apply this diff:
- Object? decoder, + Function? decoder,Optionally, adopt a sentinel for explicit clearing:
// Outside the diffed range: static const Object _unset = Object(); DecodeOptions copyWith({ /* ... */ Object? decoder = _unset, }) => DecodeOptions( /* ... */ decoder: decoder == _unset ? _decoder : (decoder as Function?), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/src/models/decode_options.dart(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
lib/src/models/decode_options.dart (4)
4-4: LGTM: DecodeKind import correctly integrated.Import aligns with the new key/value-aware decoding path and is used consistently below.
41-49: Back-compat decoder typedefs look solid.Good coverage of legacy shapes and the “kind-only” variant. This will smooth migration for existing consumers.
71-74: LGTM:allowDotsimplied bydecodeDotInKeys.Initialiser captures the implication without leaking option state.
283-302: Verify inclusion of_decoderinpropsis intended.Equatable will treat two otherwise identical options with different function identities as non-equal. This can be desirable (identity-sensitive caching) or surprising (e.g., two equivalent closures created per call). Confirm this matches consumer expectations; otherwise consider excluding
_decoderfrom equality and including a helpful representation intoString()instead.If desired, also make debugging easier by surfacing the decoder’s runtime type in
toString():- ' strictNullHandling: $strictNullHandling\n' + ' strictNullHandling: $strictNullHandling,\n' + ' decoder: ${_decoder?.runtimeType}\n'
…ests for callable decoder objects
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/src/models/decode_options.dart (2)
240-261: Bug:copyWith’sdecoderparameter type is too narrow and inconsistent
copyWithcurrently acceptsdynamic Function(String?)? decoder, but the constructor takesFunction? decoder. This prevents passing decoders with named parameters throughcopyWith, and is inconsistent with the stored type.Apply this diff to align the API and avoid compile-time/type issues:
- dynamic Function(String?)? decoder, + Function? decoder,
283-303: Equatable props omitthrowOnLimitExceededThis affects equality and could cause subtle bugs when caching or comparing options.
Apply this diff to include the missing field:
List<Object?> get props => [ @@ strictDepth, strictNullHandling, + throwOnLimitExceeded, _decoder, ];
🧹 Nitpick comments (4)
test/unit/decode_test.dart (1)
2143-2158: Optional: tighten the charset-sentinel observationYou currently assert the presence of at least one latin1 observation. If you want to be more deterministic (and future-proof accidental changes), consider asserting sequence around the sentinel, e.g., that observations after the sentinel reflect latin1 until overridden again.
lib/src/models/decode_options.dart (3)
52-71: Constructor now acceptsFunction? decoderfor flexibilityThis enables mixed shapes and callable objects. Be aware: you lose some compile-time safety, but tests demonstrate the dynamic ladder compensates adequately.
If you want a middle ground, you could additionally provide a strongly-typed
Decoder?named constructor parameter and keepFunction?as an escape hatch (second param), prioritising the typed one when both are supplied.
164-220: Dynamic invocation ladder is robust; only catches shape errorsYou correctly prefer typed variants and catch only
NoSuchMethodError/TypeErrorfor signature mismatches, allowing user-thrown errors to surface. Sensible defaulting ofcharseton fallback.You can reduce repetition and future mistakes by hoisting
final Encoding _cs = charset ?? this.charset;and using_csin allUtils.decodefallbacks.
263-281:toStringomitsthrowOnLimitExceededIncluding it aids debugging and parity with other options you already print.
Apply this diff to include the field:
String toString() => 'DecodeOptions(\n' @@ - ' strictNullHandling: $strictNullHandling\n' + ' strictNullHandling: $strictNullHandling,\n' + ' throwOnLimitExceeded: $throwOnLimitExceeded\n' ')';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/src/models/decode_options.dart(5 hunks)test/unit/decode_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
test/unit/decode_test.dart (8)
2019-2035: Good coverage: key/value-aware decoder invocation order is validatedThe test asserts KEY then VALUE per pair and exercises the new decoder signature with kind. Looks correct and stable.
2037-2041: Backward compatibility preserved for legacy single-arg decodersNice sanity check that older decoders (value-only) continue to work.
2042-2047: Decoder variant accepting onlykindworks as intendedThis proves the flexible signature. Clear and useful.
2049-2063: Options isolation guardrail validatedThis reliably exercises the “string-input-only” list guard and verifies no leakage across calls. Solid test.
2066-2077: KEY path for bare keys is correctly surfaced to custom decodersSound assertion that only KEY is observed when
strictNullHandlingis true and no value is present.
2079-2092: Comma splitting correctly emits VALUE per segmentThe order expectation (key, then each value segment) is appropriate.
2176-2195: Dynamic fallback tests (_Loose1/_Loose2) cover edge signatures wellGreat use of callable objects to exercise mismatched named parameters and required named-parameter failure paths. Clear signals that the ladder and final fallback work.
Also applies to: 2201-2216
2019-2024: DecodeKind is correctly re-exported bysrc/qs.dartThe file
lib/src/qs.dart(line 17) exportssrc/enums/decode_kind.dart, and your tests importpackage:qs_dart/src/qs.dart, soDecodeKindis available without any additional imports. No changes needed.lib/src/models/decode_options.dart (1)
31-49: Typedef matrix for decoder variants looks goodThe set of Decoder/Decoder1/Decoder2/Decoder3 covers the intended shapes. Returning
dynamicis consistent with existing decode behaviour.
1 participant
This pull request introduces a new
DecodeKindenum to enable key-aware decoding in the query string parser, and refactors the decoding logic and options to support more flexible custom decoder signatures. It also improves percent-decoding for dots in keys, adds comprehensive tests for the new behaviors, and clarifies option handling. These changes make it easier for consumers to write custom decoders that distinguish between keys and values, and ensure backward compatibility with existing decoder signatures.Key-aware decoding and API enhancements
DecodeKindenum (key,value) to distinguish decoding context for keys vs. values, and updated the decoder logic throughout the codebase to pass this context. (lib/src/enums/decode_kind.dart,lib/qs_dart.dart,lib/src/extensions/decode.dart,lib/src/models/decode_options.dart,lib/src/qs.dart) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]DecodeOptions.decoderAPI to support multiple function signatures, including legacy and new key-aware variants, with robust dynamic fallback for callable objects. (lib/src/models/decode_options.dart) [1] [2] [3]Decoding logic improvements
decode.dartto passDecodeKind.keyorDecodeKind.valuefor each token, ensuring custom decoders receive context. (lib/src/extensions/decode.dart) [1] [2]%2Eand%2econsistently. (lib/src/extensions/decode.dart)Option handling and guardrails
allowDotsdefault value to ensure correct option precedence, and improved guardrail for disabling list parsing only for string input. (lib/src/models/decode_options.dart,lib/src/qs.dart) [1] [2]Testing and validation
test/unit/decode_test.dart)Miscellaneous
Makefileto use the correct test target name (testinstead oftests). (Makefile)