🐛 fix QS.decode: properly account for strictNullHandling when allowEmptyLists
#21
Conversation
WalkthroughThe changes address a bug in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QS
participant Decode
User->>QS: Call decode with strictNullHandling and allowEmptyArrays
QS->>Decode: Pass parameters
Decode->>QS: Return empty list if criteria met
QS->>User: Return parsed result
Assessment against linked issues
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 (
|
techouse
changed the title
strictNullHandling when allowEmptyListsQS.decode: properly account for strictNullHandling when allowEmptyLists
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🚀 Don’t miss a bit, follow what’s new on Codacy. Codacy 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 #21 +/- ##
=======================================
Coverage 97.65% 97.66%
=======================================
Files 14 14
Lines 641 642 +1
=======================================
+ Hits 626 627 +1
Misses 15 15 ☔ View full report in Codecov by Sentry. |
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- lib/src/extensions/decode.dart (1 hunks)
- test/unit/decode_test.dart (1 hunks)
- test/unit/encode_test.dart (1 hunks)
Additional comments not posted (3)
lib/src/extensions/decode.dart (1)
108-109: Review of new condition in_parseObjectmethod.The revised condition checks for both an empty string (
leaf == '') and null values whenstrictNullHandlingis set. This ensures that an empty list is created when these conditions are met, aligning with the expected behaviour described in the PR. This is a crucial fix for the issue described in the linked GitHub issue.The use of a ternary operation here is appropriate and maintains readability while effectively handling multiple conditions. The logic appears sound and should correctly implement the desired functionality of handling empty lists and null values under specific settings.
test/unit/decode_test.dart (1)
263-277: Review of new test case forallowEmptyListsandstrictNullHandling.The test case correctly checks the functionality of decoding an empty list with both
strictNullHandlingandallowEmptyListsset to true. The expected output is an empty list, which aligns with the requirements specified in the linked issue.This test case is well-constructed and directly addresses the bug described. It effectively verifies that the decode function now handles the specific case of an empty list correctly when both options are enabled. The use of
constforDecodeOptionsensures that the options are immutable, which is good practice in test scenarios to prevent unintended side effects.test/unit/encode_test.dart (1)
460-474: Correct implementation of test for handling empty lists with strictNullHandling and allowEmptyListsThis test case successfully verifies the expected behavior when both
strictNullHandlingandallowEmptyListsare enabled. The assertion checks that an empty list is encoded astestEmptyList[], which aligns with the expected output described in the linked issue (#510).The test is well-structured and clearly expresses its intent, making it easy to understand the scenario being tested.
Description
Fix
QS.decodeoutput when bothstrictNullHandlingandallowEmptyListsare set totrue.Fixes ljharb/qs#510
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added new tests.
Checklist: