Skip to content

fix(cosh-ng): swap remaining chunks_exact uses - #2756

Open
jfeng18 wants to merge 1 commit into
alibaba:mainfrom
jfeng18:fix/cosh-ng/as-chunks-followup
Open

fix(cosh-ng): swap remaining chunks_exact uses#2756
jfeng18 wants to merge 1 commit into
alibaba:mainfrom
jfeng18:fix/cosh-ng/as-chunks-followup

Conversation

@jfeng18

@jfeng18 jfeng18 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Why

Clippy 1.98 (rolling stable) ships the new chunks_exact_to_as_chunks lint. Beyond the decode_hex site fixed in #2742, the same lint fires on two more chunks_exact(2) sites. This PR delivers the follow-up promised in the #2742 review discussion (KaiLongZhou's non-blocking observations).

What changed

Swapped .chunks_exact(2) for .as_chunks::<2>().0.iter() (the exact form clippy suggests) in:

  • cosh-core/src/tool/grep.rsdecode_utf16_bytes: both forms iterate full 2-byte units and drop the tail remainder; the odd tail is handled separately via has_partial_unit, unchanged.
  • cosh-shell/src/recommendation/personal_crypto.rs — test-only decode_hex helper; std::str::from_utf8(pair) accepts the &[u8; 2] items via unsize coercion.

Note on the other half of the promised follow-up: the MSRV alignment (observation 1) is already done — 77c4e8b ("feat(cosh-ng): [gateway] add ACP v1 foundation") set rust-version = "1.88" on main, matching the 1.88.0 pinned CI jobs, so no Cargo.toml change is needed here.

Related issue

no-issue: pre-existing code flagged by a new clippy lint, not a user-visible bug. Attribution per repo convention in the commit body: Fixes: c818e11876a3 ("fix(cosh-ng): [core] confine read tools") and Fixes: 8d4d74c12e70 ("feat(cosh-ng): add shell prompt recommendations").

User / Agent impact

None. Semantics-preserving internal refactor; no public API, CLI, or behavior change.

Risk and compatibility

  • Public CLI, API, configuration, or documented behavior changed
  • Privileged or security-sensitive behavior changed
  • Cross-component contract changed
  • Migration or rollback guidance is needed

Low risk: as_chunks stabilized in Rust 1.88; cosh-ng CI pins 1.88.0 (integration/release-build) and 1.97.1 (fast checks) — neither toolchain carries this lint, so the pinned jobs compile and lint unchanged.

Validation

  • Discriminating lint evidence (ECS Linux, stable 1.98 clippy, -p cosh-core -p cosh-shell --all-targets --keep-going):
    • pre-fix on the main baseline (with query.rs temporarily mirroring the fix(cosh-ng): swap chunks_exact for as_chunks #2742 patch so cosh-platform compiles and cosh-core gets reached): the lint fires at grep.rs:511 and personal_crypto.rs:259
    • post-fix (this PR): both target sites disappear from the lint output; exit code still non-zero only because of the out-of-scope sites below
  • ECS, RUSTUP_TOOLCHAIN=1.89.0: cargo clippy -p cosh-core -p cosh-shell --all-targets -- -D warnings — exit 0
  • ECS, RUSTUP_TOOLCHAIN=1.88.0: cargo check -p cosh-core -p cosh-shell — exit 0 (MSRV floor confirmed; the two pre-existing fs2 unlock() future-incompat warnings in extension/ are unrelated to this change)
  • ECS, 1.89.0: cargo test -p cosh-core --lib — 124 passed / 0 failed; cargo test -p cosh-shell --lib — 1347 passed / 0 failed
  • Local cargo fmt --all -- --check with both 1.89.0 and stable 1.98 — clean
  • Known out-of-scope remainder: personal_crypto.rs:66 (SHA256_BLOCK_BYTES) and :172 (chunks_exact(4)) also trip the same lint on clippy 1.98; they are not the chunks_exact(2) sites called out in the review and the pinned CI toolchains do not carry the lint. Left for a later pass if fast checks ever move to a lint-carrying toolchain (same for query.rs, owned by fix(cosh-ng): swap chunks_exact for as_chunks #2742).

Documentation and rollback

None. Revert the single commit to restore the previous form.

Clippy 1.98 raises chunks_exact_to_as_chunks on the two remaining
chunks_exact(2) sites; swap both to as_chunks::<2>().0.iter(), the
exact form clippy suggests (follow-up to alibaba#2742).

Fixes: c818e11 ("fix(cosh-ng): [core] confine read tools")
Fixes: 8d4d74c ("feat(cosh-ng): add shell prompt recommendations")
Assisted-by: Qoder
Signed-off-by: Jiangtian Feng <jiangtianf97@163.com>

@qoderai qoderai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本次修改主要是将若干 chunks_exact(2) 换成 as_chunks::<2>().0.iter() 以满足新 clippy lint 要求,作用范围仅在 cosh-core 和 cosh-shell 内部,不涉及对外行为。

我重点核对了:

  • decode_utf16_bytes / decode_hex 对奇数字节尾部的处理是否保持一致;
  • PR 描述与实际 diff 是否完全一致,避免遗漏 chunks_exact(2) 站点;
  • MSRV 及 clippy lint 行为与使用的 as_chunks 写法是否兼容。

🤖 Generated by QoderView workflow run

Comment thread src/cosh-ng/crates/cosh-core/src/tool/grep.rs
Comment thread src/cosh-ng/crates/cosh-shell/src/recommendation/personal_crypto.rs
@KaiLongZhou

Copy link
Copy Markdown
Collaborator

Code Review — PR #2756

结论:LGTM ✅ — 未发现阻塞问题


语义等价性验证

Site 1 — cosh-core/src/tool/grep.rs:511 (decode_utf16_bytes)

chunks_exact(2) as_chunks::<2>().0.iter()
Iterator item &[u8] (len=2) &[u8; 2]
Closure 中 bytes[0], bytes[1] ✅ 正常工作 ✅ 正常工作
尾部余量处理 丢弃,由 has_partial_unit 处理 相同 — .0 同样丢弃余量

语义完全等价。read_utf16_unit 接收 [u8; 2] 值类型,两种情况下 [bytes[0], bytes[1]] 构造结果一致。

Site 2 — cosh-shell/src/recommendation/personal_crypto.rs:259 (test decode_hex)

chunks_exact(2) as_chunks::<2>().0.iter()
Iterator item &[u8] (len=2) &[u8; 2]
from_utf8(pair) &[u8] → 直接匹配 &[u8; 2] → unsize coercion 到 &[u8]

语义完全等价。std::str::from_utf8 签名接受 &[u8]&[u8; 2] 通过 unsized coercion 自动转换。

MSRV 兼容性

as_chunks::<N>() 在 Rust 1.88 稳定。当前 rust-version = "1.88" (77c4e8b),CI pinned 1.88.0 / 1.97.1 — 兼容无问题。

Out-of-scope 站点

PR 描述正确记录了以下已知未修复的 chunks_exact 站点:

这些不是本 PR 的范围,且当前 CI pinned toolchains 不携带该 lint。

P0 / P1 未发现

变更最小化、目标明确、语义等价,测试覆盖充分(cosh-core 124 passed, cosh-shell 1347 passed)。

@KaiLongZhou

Copy link
Copy Markdown
Collaborator

code-reviewer-check: 审核通过 ✅

已回读 pr-2756 分支代码,逐条验证 code-reviewer 的 review 结论:

  1. grep.rs 语义等价 ✅ — as_chunks::<2>().0.iter() 产出 &[u8; 2][bytes[0], bytes[1]] 构造 [u8; 2] 传入 read_utf16_unit,类型匹配正确
  2. personal_crypto.rs 语义等价 ✅ — from_utf8(pair) 通过 unsize coercion 接受 &[u8; 2]
  3. 尾部余量处理不变 ✅ — as_chunks.0 丢弃余量,与原 chunks_exact(2) 行为一致
  4. MSRV 兼容 ✅ — as_chunks stabilized in 1.88,匹配 rust-version = "1.88"
  5. Out-of-scope 站点记录准确

未发现遗漏或误判,建议合并。

@KaiLongZhou

Copy link
Copy Markdown
Collaborator

Review Check: LGTM ✅

code-reviewer 的 review 结论经逐项代码回读验证,全部正确:

  • 语义等价性chunks_exact(2)as_chunks::<2>().0.iter() 两处替换均为语义保持的等价变换(&[u8]&[u8; 2],索引行为和 unsized coercion 均兼容)
  • MSRVas_chunks 稳定于 Rust 1.88.0,与 rust-version = "1.88" 匹配
  • 范围完整性:cosg-ng 中所有 chunks_exact 调用点已核实,本 PR 覆盖全部 chunks_exact(2) 站点
  • Commit 质量:单 commit,+6/-2,diff 最小化,归因完备

结论:可以合并,无阻塞问题。

@KaiLongZhou KaiLongZhou left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — review check passed. Semantic equivalence, MSRV compatibility, scope completeness, and commit quality all verified against source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants