-
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ DecodeOptions.strictDepth option to throw when input is beyond depth #22
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DecodeOptions
participant Decoder
User->>DecodeOptions: Set strictDepth to true
User->>Decoder: Call decode with nested input
Decoder->>Decoder: Check if strictDepth is enabled
alt if depth exceeds limit
Decoder-->>User: Throw RangeError
else
Decoder-->>User: Return parsed output
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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 preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
==========================================
+ Coverage 97.66% 97.68% +0.02%
==========================================
Files 14 14
Lines 642 648 +6
==========================================
+ Hits 627 633 +6
Misses 15 15 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
test/unit/models/decode_options_test.dart (1)
Include
strictDepth
in Test Case VerificationThe
strictDepth
property is present in theDecodeOptions
, but it is not currently verified in thecopyWith no modifications
test case. Ensure thatstrictDepth
is included in the test to confirm it remains unchanged whencopyWith
is called without arguments.
- Ensure
strictDepth
is tested incopyWith no modifications
.Analysis chain
Line range hint
14-37
:
Verify inclusion ofstrictDepth
in tests.Ensure that the
strictDepth
property is tested in thecopyWith no modifications
test case to confirm it remains unchanged.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `strictDepth` is included in the `copyWith no modifications` test case. # Test: Search for `strictDepth` in the test case. Expect: `strictDepth` is verified. rg --type dart -A 5 'test\(' test/unit/models/decode_options_test.dart | rg 'strictDepth'Length of output: 89
Script:
#!/bin/bash # Description: Search for `strictDepth` in the entire `decode_options_test.dart` file to verify its presence and usage. # Test: Search for `strictDepth` in the file. Expect: `strictDepth` is present and tested. rg 'strictDepth' test/unit/models/decode_options_test.dartLength of output: 122
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- README.md (1 hunks)
- lib/src/extensions/decode.dart (1 hunks)
- lib/src/models/decode_options.dart (6 hunks)
- test/unit/decode_test.dart (1 hunks)
- test/unit/models/decode_options_test.dart (2 hunks)
Additional comments not posted (11)
test/unit/models/decode_options_test.dart (2)
Line range hint
39-73
:
LGTM!strictDepth
is correctly tested incopyWith modifications
.The test case appropriately verifies the behaviour of
strictDepth
when modified.
Line range hint
75-134
:
LGTM!strictDepth
is correctly included intoString
.The
toString
method and its corresponding test case correctly include and verify thestrictDepth
property.lib/src/extensions/decode.dart (1)
193-199
: Verify clarity ofRangeError
message forstrictDepth
.The error message should clearly indicate the issue and provide guidance on how to address it. Consider refining the message for clarity.
Verification successful
Consider improving the
RangeError
message for clarity.The current error message explains the condition but could be enhanced by suggesting actions, such as checking or adjusting the
depth
orstrictDepth
settings, to help users resolve the issue.
- Current message: "Input depth exceeded depth option of ${options.depth} and strictDepth is true"
- Suggested improvement: "Input depth exceeded the configured depth of ${options.depth}. Consider increasing the depth or disabling strictDepth to avoid this error."
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify clarity and completeness of the `RangeError` message in `_parseKeys`. # Test: Search for `RangeError` message in the code. Expect: Clear and complete message. rg --type dart 'RangeError' lib/src/extensions/decode.dartLength of output: 84
Script:
#!/bin/bash # Description: Retrieve full context of the `RangeError` message in `_parseKeys`. # Test: Search for `RangeError` and include surrounding lines for full context. rg --type dart 'RangeError' -A 3 -B 3 lib/src/extensions/decode.dartLength of output: 362
lib/src/models/decode_options.dart (5)
27-27
: LGTM!strictDepth
property added correctly.The
strictDepth
property is correctly added with a default value offalse
.
106-108
: LGTM!strictDepth
parameter integrated in constructor.The constructor correctly includes the
strictDepth
parameter, allowing it to be set during instantiation.
Line range hint
138-158
:
LGTM!strictDepth
included incopyWith
.The
copyWith
method correctly includes thestrictDepth
parameter, allowing for its modification.
178-178
: LGTM!strictDepth
represented intoString
.The
toString
method correctly includes thestrictDepth
property in its output representation.
198-198
: LGTM!strictDepth
included inprops
.The
props
method correctly includes thestrictDepth
property for equality checks.README.md (1)
135-153
: Documentation is clear and comprehensive.The explanation of the
strictDepth
option and its examples are well-written and provide a clear understanding of the feature.test/unit/decode_test.dart (2)
1627-1679
: Comprehensive throw test cases forstrictDepth
.The test cases effectively cover multiple nesting scenarios, ensuring that
RangeError
is thrown as expected whenstrictDepth
is enabled.
1681-1750
: Well-structured non-throw test cases forstrictDepth
.The test cases comprehensively verify scenarios where no exceptions should be thrown, ensuring the
strictDepth
feature behaves correctly.
Description
This PR adds
DecodeOptions.strictDepth
to enforce throwing when input depth is beyond the depth option.Defaults to
false
.Throws
RangeError
.If depth has been set by the user to
0
, we do not throw, but fallback to the previous behaviour.Type of change
How Has This Been Tested?
Added additional tests
Checklist:
Ref ljharb/qs#511