fix(sheets): add missing JSON unmarshal error handling in dry-run mode

[KhanCold]

Summary

Two DryRun functions in the sheets shortcuts called json.Unmarshal without checking the return value. This looks like a bug, but Validate already parses and validates the same --style / --data JSON before DryRun runs, so the error is structurally impossible at this point.

Use _ = assignment + comment to silence the unchecked-error lint warning and make the safety invariant explicit to future readers.

Changes

• SheetSetStyle.DryRun: document why json.Unmarshal of --style can be safely ignored
• SheetBatchSetStyle.DryRun: document why json.Unmarshal of --data can be safely ignored (via validateBatchStyleData())

Note

The initial attempt added if err != nil { return common.FlagErrorf(...) } inside DryRun, but DryRun must return *common.DryRunAPI (not error), so that would not compile. The correct fix is to acknowledge the already-validated invariant instead.

Summary by CodeRabbit

• Style
  • Internal code cleanup to improve maintainability in sheet functionality.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 525b4568-480f-4634-aeef-65f6566d7d8e

📥 Commits

Reviewing files that changed from the base of the PR and between 890cf52 and bb9bd12.

📒 Files selected for processing (1)

• shortcuts/sheets/lark_sheets_cell_style_and_merge.go

✅ Files skipped from review due to trivial changes (1)

• shortcuts/sheets/lark_sheets_cell_style_and_merge.go

---

📝 Walkthrough

Walkthrough

Two DryRun handlers now explicitly assign the result of json.Unmarshal to _ and add comments indicating validation already parses/validates the --style and --data JSON; no new error handling was introduced.

Changes

DryRun explicit json.Unmarshal discard

Layer / File(s)	Summary
Assign json.Unmarshal to _ in DryRun shortcuts/sheets/lark_sheets_cell_style_and_merge.go	SheetSetStyle DryRun now assigns json.Unmarshal result to _ for --style with a comment that Validate already parses it; SheetBatchSetStyle DryRun does the same for --data (no new error handling added).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

• fangshuyu-768

Poem

🐰 I nibble code at break of dawn,
Swallow JSON bits then carry on.
A quiet discard, a tiny sigh,
Comments say validation did apply.
Hooray for tidy, hop—goodbye!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding explicit error handling (via discarding with comment) for json.Unmarshal calls in DryRun functions.
Description check	✅ Passed	The description follows the template with all required sections: Summary explains the problem and solution clearly, Changes lists the two functions modified, and includes a Note clarifying the compilation issue and rationale. Test Plan and Related Issues sections are omitted but non-critical.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

This PR aims to make sheets +set-style and sheets +batch-set-style dry-run behavior consistent with real execution by handling JSON unmarshal failures for --style/--data instead of silently proceeding.

Changes:

• Add JSON unmarshal error checks in SheetSetStyle.DryRun for --style.
• Add JSON unmarshal error checks in SheetBatchSetStyle.DryRun for --data.

Comments suppressed due to low confidence (1)

shortcuts/sheets/lark_sheets_cell_style_and_merge.go:171

• DryRun functions must return *common.DryRunAPI, but this branch returns common.FlagErrorf (an error) on JSON parse failure, which will not compile. Validate already calls validateBatchStyleData and rejects invalid --data before dry-run runs, so either remove this error-return path or adjust the DryRun/runner API to allow returning an error.

		if err := json.Unmarshal([]byte(runtime.Str("data")), &data); err != nil {
			return common.FlagErrorf("--data must be valid JSON: %v", err)
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fangshuyu-768]

This PR does not compile. The DryRun function signature returns *common.DryRunAPI, but common.FlagErrorf returns error. The type mismatch causes build failures:

shortcuts/sheets/lark_sheets_cell_style_and_merge.go:99:11: cannot use common.FlagErrorf("--style must be valid JSON: %v", err) (value of type error) as *cmdutil.DryRunAPI value in return statement
shortcuts/sheets/lark_sheets_cell_style_and_merge.go:170:11: cannot use common.FlagErrorf("--data must be valid JSON: %v", err) (value of type error) as *cmdutil.DryRunAPI value in return statement

Since Validate already guarantees valid JSON before DryRun is called, a possible fix is to explicitly discard the error with a comment:

// Safe to ignore: Validate already verified JSON validity
_ = json.Unmarshal([]byte(runtime.Str("style")), &style)

[KhanCold]

@fangshuyu-768 Thanks for the detailed feedback. You were absolutely right — the original commit still had the if err != nil { return common.FlagErrorf(...) } paths that don't compile because DryRun returns *common.DryRunAPI, not error.

I've amended the commit to use _ = json.Unmarshal(...) with explicit comments documenting the safety invariant (Validate already guarantees valid JSON before DryRun runs). Force-pushed as bb9bd12. PTAL, thanks!

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.92%. Comparing base (898e0ee) to head (bb9bd12).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #935   +/-   ##
=======================================
  Coverage   65.92%   65.92%           
=======================================
  Files         523      523           
  Lines       49692    49692           
=======================================
  Hits        32758    32758           
  Misses      14134    14134           
  Partials     2800     2800           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@bb9bd12bd365670cbb90ddc7298ffdb1aa5afb3e

🧩 Skill update

npx skills add KhanCold/cli#fix/dryrun-missing-json-error-handling -y -g