fix: support optional chaining in environment variable #1730
Conversation
概述遍历在 变更
诗歌
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request introduces support for optional chaining in environment variables within the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (2)
crates/mako/src/visitors/env_replacer.rs (2)
37-50: extract_prop_name 函数逻辑良好，但需注意解释性注释此函数抽取成员属性名时，兼容 Ident 和 Computed 两种情况，能正确处理字符串、数字等多种类型。不过建议补充更多注释，说明遇到其他表达式时返回 None 的原因，方便维护。
52-76: process_chain_expr 函数由于递归逻辑较复杂，建议考虑更清晰的代码结构此函数采用递归的方式遍历可选链，能正确收集成员表达式中的属性名，并与
extract_prop_name配合。但是逻辑嵌套较深，结构可适当拆分，使维护更容易。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/mako/src/visitors/env_replacer.rs(4 hunks)
🔇 Additional comments (8)
crates/mako/src/visitors/env_replacer.rs (8)
10-11: 可选链表达式的导入看起来正确
这里新增了对可选链表达式相关类型的导入，例如 OptChainBase 和 OptChainExpr，满足了处理可选链的需求，整体实现合理。
90-107: 对可选链的处理逻辑
在 visit_mut_expr 中对可选链表达式的处理相当完备，先收集所有属性名后再用 get_define_env 查找对应的环境变量，能够实现正确的替换逻辑。请确保对找不到定义的情况保持原表达式不变，避免引入潜在错误。
548-559: test_optional_chaining_basic 测试用例有效
此测试用例验证了最基本的 A?.B 场景，能够覆盖到可选链的基本替换逻辑，测试充分且符合需求。
561-572: test_optional_chaining_nested 测试用例确保可选链的多层嵌套
用于检测 A?.B?.C 这种更深层可选链的场景，验证了多次递归调用时的表现。该测试对于全面性很有帮助。
574-585: test_optional_chaining_with_computed 测试正确处理 A?.["B"]?.C
验证可选链与计算属性结合时的替换逻辑，有助于确保 Computed 类型属性的准确捕获。
587-598: test_optional_chaining_with_number_computed 测试数字类型索引
测试像 A?.[1]?.B 这样的场景，可覆盖数字索引的可选链处理，为特殊字符索引的正确性提供保证。
600-611: test_optional_chaining_mixed 测试混合形式可选链场景
该测试覆盖了 A?.B?.["C"]?.D 等更复杂场景，验证了多种性质组合在一起时能否正确替换。此类测试极具价值。
613-624: test_optional_chaining_not_defined 测试未定义的可选链路径
此测试用例能有效验证当查找不到对应的 key 时，是否正确保持原表达式不变，符合预期需求。
| @@ -484,6 +545,84 @@ mod tests { | |||
| ); | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
需要补充两类用例：
- optional chain 和 非 optional chain 混用的情况，入 A.B?.C;
- optional chain 作为 computed prop 的情况，入 A[x?.y]
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1730 +/- ##
==========================================
+ Coverage 54.58% 54.79% +0.20%
==========================================
Files 180 180
Lines 17958 18041 +83
==========================================
+ Hits 9803 9885 +82
- Misses 8155 8156 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/mako/src/visitors/env_replacer.rs (3)
38-50: 需要考虑处理私有字段的情况
extract_prop_name方法目前只处理了标识符和计算属性的情况。建议添加对私有字段（Private Fields）的支持。fn extract_prop_name(&self, prop: &MemberProp) -> Option<String> { match prop { MemberProp::Ident(ident) => Some(ident.sym.to_string()), MemberProp::Computed(computed) => match computed.expr.as_ref() { Expr::Lit(Lit::Str(str_lit)) => Some(str_lit.value.to_string()), Expr::Lit(Lit::Num(num_lit)) => Some(num_lit.value.to_string()), _ => None, }, + MemberProp::PrivateName(private) => Some(format!("#{}", private.id.sym.to_string())), _ => None, } }
90-107: 建议优化错误处理和日志记录当前的可选链处理逻辑已经实现得很好，但建议添加以下改进：
- 添加调试日志，便于追踪属性解析过程
- 在环境变量不存在时添加警告日志
Expr::OptChain(OptChainExpr { base, .. }) => { if let OptChainBase::Member(member) = base.as_ref() { let mut parts = Vec::new(); + tracing::debug!("Processing optional chain expression"); if let Some(prop_name) = self.extract_prop_name(&member.prop) { parts.push(prop_name); if self.process_chain_expr(&member.obj, &mut parts) { parts.reverse(); let full_path = parts.join("."); + tracing::debug!("Resolved environment path: {}", full_path); if let Some(env) = self.get_define_env(&full_path) { *expr = env + } else { + tracing::warn!("Environment variable not found: {}", full_path); } } } } }
548-665: 建议添加边缘情况的测试用例当前的测试覆盖了基本场景，但建议添加以下边缘情况的测试：
- 深度嵌套的可选链（超过5层）
- 包含表达式的计算属性，如：
A?.[1 + 2]?.B- 空字符串作为属性名的情况
#[test] fn test_deep_optional_chaining() { assert_eq!( run( r#"log(A?.B?.C?.D?.E?.F)"#, hashmap! { "A.B.C.D.E.F".to_string() => json!(42) } ), "log(42);" ); } #[test] fn test_optional_chaining_with_expression() { assert_eq!( run( r#"log(A?.[1 + 2]?.B)"#, hashmap! { "A.3.B".to_string() => json!(true) } ), "log(A?.[1 + 2]?.B);" ); } #[test] fn test_empty_string_property() { assert_eq!( run( r#"log(A?.[""]?.B)"#, hashmap! { "A..B".to_string() => json!(42) } ), "log(42);" ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/mako/src/visitors/env_replacer.rs(4 hunks)
🔇 Additional comments (1)
crates/mako/src/visitors/env_replacer.rs (1)
69-69: 验证 unresolved_mark 的比较逻辑
需要确保 ident.ctxt.outer() 与 self.unresolved_mark 的比较是正确的。如果 unresolved_mark 设置不当，可能导致标识符处理出现问题。
#!/bin/bash
# 描述：验证 unresolved_mark 的使用情况
# 搜索其他可能影响 unresolved_mark 设置的代码
rg -A 3 "unresolved_mark\s*=|set_unresolved_mark"
# 搜索其他使用 ctxt.outer() 进行比较的地方
ast-grep --pattern 'ctxt.outer() == $_'
Close #1729
Close #788
Summary by CodeRabbit