Skip to content

Fix #116, json string misjudgment#119

Merged
iAmMccc merged 2 commits into
iAmMccc:mainfrom
faimin:main
Oct 13, 2025
Merged

Fix #116, json string misjudgment#119
iAmMccc merged 2 commits into
iAmMccc:mainfrom
faimin:main

Conversation

@faimin
Copy link
Copy Markdown
Contributor

@faimin faimin commented Oct 11, 2025

Summary by CodeRabbit

  • New Features

    • Improved handling of multiline JSON strings during parsing and decoding.
  • Bug Fixes

    • Parser now attempts to parse any UTF-8 string instead of short-circuiting on non-JSON prefixes; invalid JSON still yields no result.
  • Tests

    • Added tests covering decoding from multiline object and array strings, numeric-string parsing, and nil handling for malformed/partial inputs.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 11, 2025

Walkthrough

Adds a new test file with multiline JSON decoding tests and modifies String.toJSONObject() to remove a pre-check so JSONSerialization always attempts to parse the string.

Changes

Cohort / File(s) Summary
New multiline JSON tests
Example/Tests/MultilineStringTest.swift
Adds User and MultilineStringTest types with two @Test methods covering decoding multiline JSON object and array literals, nil handling for malformed input, and numeric parsing from strings.
Decoder string-to-JSON behavior
Sources/.../JSONDecoder/Decoder/KeysMapper.swift
Removes an early guard in String.toJSONObject(), so the method always passes UTF-8 data to JSONSerialization; parsing failures return nil via the existing try? path.
Xcode project wiring
Example/SmartCodable.xcodeproj/project.pbxproj
Adds MultilineStringTest.swift entries to project file references, build phases, and test target so the new tests are included in the test target.

Sequence Diagram(s)

sequenceDiagram
  actor Test
  participant TestTarget as Tests
  participant Decoder as SmartCodable.decode
  participant StringExt as String.toJSONObject()
  participant JSONSer as JSONSerialization

  Test->>TestTarget: run MultilineStringTest
  TestTarget->>Decoder: decode(multilineString)
  Decoder->>StringExt: toJSONObject()
  note over StringExt: New flow — no early guard\nAlways attempt UTF-8 parse
  StringExt->>JSONSer: parse(data)
  alt Valid JSON
    JSONSer-->>StringExt: JSONObject
    StringExt-->>Decoder: JSONObject
    Decoder-->>TestTarget: Decoded model/array
  else Invalid JSON
    JSONSer--x StringExt: parsing fails (try?)
    StringExt-->>Decoder: nil
    Decoder-->>TestTarget: nil / failed decode
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I nibble lines of curly braces, crunch!
Multiline carrots in a JSON bunch.
Guards hop aside—let parsing flow,
Strings to numbers—watch them grow.
Tests tucked snug in the burrow, go! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title explicitly references the tracked issue and clearly summarizes the primary change—correcting JSON string misjudgment—without extraneous detail, aligning directly with the key modifications to String.toJSONObject and related tests.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 628354e and 5cc6930.

⛔ Files ignored due to path filters (1)
  • Example/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • Example/SmartCodable.xcodeproj/project.pbxproj (4 hunks)
  • Example/Tests/MultilineStringTest.swift (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Example/Tests/MultilineStringTest.swift
⏰ 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: build
🔇 Additional comments (1)
Example/SmartCodable.xcodeproj/project.pbxproj (1)

145-145: LGTM! Test file properly wired into Xcode project.

The changes correctly integrate MultilineStringTest.swift into the Xcode project structure:

  • PBXBuildFile entry created (line 145)
  • PBXFileReference entry created (line 296)
  • File added to Tests group (line 387)
  • File included in test target's Sources build phase (line 1138)

All UUIDs are properly formatted and references are consistent.

Also applies to: 296-296, 387-387, 1138-1138


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a 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

🧹 Nitpick comments (2)
Example/Tests/MultilineStringTest.swift (2)

18-38: Test logic is solid; consider fixing the method name typo.

The test correctly validates both valid multiline JSON parsing and nil handling for malformed input.

However, the method name appears to have a typo: decodeMultilingDictStr should likely be decodeMultilineDict**Str**.

Apply this diff to fix the typo:

-    func decodeMultilingDictStr() {
+    func decodeMultilineDictStr() {

40-53: Array parsing test looks good; same typo in method name.

The test properly validates multiline array parsing and type coercion from string to Int.

The method name has the same typo: decodeMultilingArrayStr should be decodeMultilineArrayStr.

Apply this diff to fix the typo:

-    func decodeMultilingArrayStr() {
+    func decodeMultilineArrayStr() {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f2b7cc0 and 628354e.

📒 Files selected for processing (2)
  • Example/Tests/MultilineStringTest.swift (1 hunks)
  • Sources/SmartCodable/Core/JSONDecoder/Decoder/KeysMapper.swift (0 hunks)
💤 Files with no reviewable changes (1)
  • Sources/SmartCodable/Core/JSONDecoder/Decoder/KeysMapper.swift
🔇 Additional comments (1)
Example/Tests/MultilineStringTest.swift (1)

12-15: LGTM!

The test fixture is well-defined with appropriate default values for testing.

@iAmMccc iAmMccc merged commit 3b9084c into iAmMccc:main Oct 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants