feat(lsp): support constant event names but ignore constants in invoke calls#75
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for resolving event names and arguments defined as constants in Rust, JavaScript, and TypeScript files by extracting constant definitions from the AST. Feedback on the changes highlights critical compilation errors in several parser files due to type mismatches in unwrap_or calls when resolving constants. Additionally, it is recommended to optimize performance in utils.rs by utilizing std::sync::LazyLock to compile tree-sitter queries only once rather than on every file parse.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| .utf8_text(content.as_bytes()) | ||
| .unwrap_or_default() | ||
| .to_string(); | ||
| } else if resolved_event.starts_with('"') | ||
| && resolved_event.ends_with('"') | ||
| && resolved_event.len() >= 2 | ||
| { | ||
| resolved_event = resolved_event[1..resolved_event.len() - 1].to_string(); | ||
| } | ||
| } else { | ||
| if let Some(resolved) = constants.get(&resolved_event) { | ||
| resolved_event.clone_from(resolved); | ||
| } else { | ||
| let lookup_key = if resolved_event.contains("::") { |
There was a problem hiding this comment.
This code will fail to compile because unwrap_or expects &str but receives &String (&resolved_event), and the if and else branches return incompatible types (&str vs &String). Additionally, if a constant cannot be resolved, we should clear the event name to avoid indexing the unresolved identifier name as a literal event name.
} else {
let mut resolved = false;
if let Some(resolved_val) = constants.get(&resolved_event) {
resolved_event.clone_from(resolved_val);
resolved = true;
} else {
let lookup_key = if resolved_event.contains("::") {
resolved_event.split("::").last().unwrap_or(resolved_event.as_str())
} else {
resolved_event.as_str()
};
if let Some(resolved_val) = constants.get(lookup_key) {
resolved_event.clone_from(resolved_val);
resolved = true;
}
}
if !resolved {
resolved_event.clear();
}
}| || resolved_arg.starts_with('\'') && resolved_arg.ends_with('\'') | ||
| || resolved_arg.starts_with('`') && resolved_arg.ends_with('`')) | ||
| && resolved_arg.len() >= 2 | ||
| { | ||
| resolved_arg = resolved_arg[1..resolved_arg.len() - 1].to_string(); | ||
| } | ||
| } | ||
| } else { | ||
| if let Some(resolved) = constants.get(&resolved_arg) { | ||
| resolved_arg.clone_from(resolved); | ||
| } else { | ||
| let lookup_key = if resolved_arg.contains('.') { | ||
| resolved_arg.split('.').next_back().unwrap_or(&resolved_arg) | ||
| } else { |
There was a problem hiding this comment.
This code will fail to compile because unwrap_or expects &str but receives &String (&resolved_arg), and the if and else branches return incompatible types (&str vs &String). Additionally, if a constant cannot be resolved, we should clear the argument to avoid indexing the unresolved identifier name as a literal event name.
} else {
let mut resolved = false;
if let Some(resolved_val) = constants.get(&resolved_arg) {
resolved_arg.clone_from(resolved_val);
resolved = true;
} else {
let lookup_key = if resolved_arg.contains('.') {
resolved_arg.split('.').next_back().unwrap_or(resolved_arg.as_str())
} else {
resolved_arg.as_str()
};
if let Some(resolved_val) = constants.get(lookup_key) {
resolved_arg.clone_from(resolved_val);
resolved = true;
}
}
if !resolved {
resolved_arg.clear();
}
}| resolved_arg = fragment.utf8_text(bytes).unwrap_or_default().to_string(); | ||
| } else { | ||
| // fallback: strip first and last quotes | ||
| if (resolved_arg.starts_with('"') && resolved_arg.ends_with('"') | ||
| || resolved_arg.starts_with('\'') && resolved_arg.ends_with('\'') | ||
| || resolved_arg.starts_with('`') && resolved_arg.ends_with('`')) | ||
| && resolved_arg.len() >= 2 | ||
| { | ||
| resolved_arg = resolved_arg[1..resolved_arg.len() - 1].to_string(); | ||
| } | ||
| } | ||
| } else { | ||
| if let Some(resolved) = constants.get(&resolved_arg) { | ||
| resolved_arg.clone_from(resolved); |
There was a problem hiding this comment.
This code will fail to compile because unwrap_or expects &str but receives &String (&resolved_arg), and the if and else branches return incompatible types (&str vs &String). Additionally, if a constant cannot be resolved, we should clear the argument to avoid indexing the unresolved identifier name as a literal event name.
} else {
let mut resolved = false;
if let Some(resolved_val) = constants.get(&resolved_arg) {
resolved_arg.clone_from(resolved_val);
resolved = true;
} else {
let lookup_key = if resolved_arg.contains('.') {
resolved_arg.split('.').next_back().unwrap_or(resolved_arg.as_str())
} else {
resolved_arg.as_str()
};
if let Some(resolved_val) = constants.get(lookup_key) {
resolved_arg.clone_from(resolved_val);
resolved = true;
}
}
if !resolved {
resolved_arg.clear();
}
}| .utf8_text(bytes) | ||
| .unwrap_or_default() | ||
| .to_string(); | ||
| } else if resolved_name.starts_with('"') | ||
| && resolved_name.ends_with('"') | ||
| && resolved_name.len() >= 2 | ||
| { | ||
| resolved_name = resolved_name[1..resolved_name.len() - 1].to_string(); | ||
| } | ||
| } else { | ||
| if let Some(resolved) = constants.get(&resolved_name) { | ||
| resolved_name.clone_from(resolved); | ||
| } else { | ||
| let lookup_key = if resolved_name.contains("::") { |
There was a problem hiding this comment.
This code will fail to compile because unwrap_or expects &str but receives &String (&resolved_name), and the if and else branches return incompatible types (&str vs &String). Additionally, if a constant cannot be resolved, we should clear the event name to avoid indexing the unresolved identifier name as a literal event name.
} else {
let mut resolved = false;
if let Some(resolved_val) = constants.get(&resolved_name) {
resolved_name.clone_from(resolved_val);
resolved = true;
} else {
let lookup_key = if resolved_name.contains("::") {
resolved_name.split("::").last().unwrap_or(resolved_name.as_str())
} else {
resolved_name.as_str()
};
if let Some(resolved_val) = constants.get(lookup_key) {
resolved_name.clone_from(resolved_val);
resolved = true;
}
}
if !resolved {
resolved_name.clear();
}
}| pub fn extract_rust_constants( | ||
| root: tree_sitter::Node<'_>, | ||
| content: &str, | ||
| ) -> std::collections::HashMap<String, String> { | ||
| let mut constants = std::collections::HashMap::new(); | ||
| let ts_lang = tree_sitter_rust::LANGUAGE.into(); | ||
| let query_str = r" | ||
| (const_item | ||
| name: (identifier) @name | ||
| value: (string_literal | ||
| (string_content) @value) | ||
| ) | ||
| (static_item | ||
| name: (identifier) @name | ||
| value: (string_literal | ||
| (string_content) @value) | ||
| ) | ||
| "; | ||
| if let Ok(query) = Query::new(&ts_lang, query_str) { | ||
| let mut cursor = QueryCursor::new(); | ||
| let bytes = content.as_bytes(); | ||
| let mut matches = cursor.matches(&query, root, bytes); | ||
| let name_idx = query.capture_index_for_name("name"); | ||
| let value_idx = query.capture_index_for_name("value"); | ||
| while let Some(m) = matches.next() { | ||
| if let (Some(name_cap), Some(val_cap)) = | ||
| (find_capture(m, name_idx), find_capture(m, value_idx)) | ||
| { | ||
| let name = name_cap | ||
| .node | ||
| .utf8_text(bytes) | ||
| .unwrap_or_default() | ||
| .to_string(); | ||
| let value = val_cap |
There was a problem hiding this comment.
Compiling tree-sitter queries on every single file parse is inefficient. We can use std::sync::LazyLock to compile the queries exactly once on first use, which significantly improves performance.
pub fn extract_rust_constants(
root: tree_sitter::Node<'_>,
content: &str,
) -> std::collections::HashMap<String, String> {
static RUST_CONSTANTS_QUERY: std::sync::LazyLock<Query> = std::sync::LazyLock::new(|| {
let ts_lang = tree_sitter_rust::LANGUAGE.into();
Query::new(
&ts_lang,
r"
(const_item
name: (identifier) @name
value: (string_literal
(string_content) @value)
)
(static_item
name: (identifier) @name
value: (string_literal
(string_content) @value)
)
",
)
.expect("Failed to parse Rust constants query")
});
let mut constants = std::collections::HashMap::new();
let mut cursor = QueryCursor::new();
let bytes = content.as_bytes();
let mut matches = cursor.matches(&RUST_CONSTANTS_QUERY, root, bytes);
let name_idx = RUST_CONSTANTS_QUERY.capture_index_for_name("name");
let value_idx = RUST_CONSTANTS_QUERY.capture_index_for_name("value");
while let Some(m) = matches.next() {
if let (Some(name_cap), Some(val_cap)) = (find_capture(m, name_idx), find_capture(m, value_idx)) {
let name = name_cap.node.utf8_text(bytes).unwrap_or_default().to_string();
let value = val_cap.node.utf8_text(bytes).unwrap_or_default().to_string();
constants.insert(name, value);
}
}
constants
}| .to_string(); | ||
| constants.insert(name, value); | ||
| } | ||
| } | ||
| } | ||
| constants | ||
| } | ||
|
|
||
| /// Extract constant definitions from a JavaScript/TypeScript AST root node | ||
| #[must_use] | ||
| pub fn extract_js_constants( | ||
| root: tree_sitter::Node<'_>, | ||
| content: &str, | ||
| is_javascript: bool, | ||
| ) -> std::collections::HashMap<String, String> { | ||
| let mut constants = std::collections::HashMap::new(); | ||
| let ts_lang: tree_sitter::Language = if is_javascript { | ||
| tree_sitter_javascript::LANGUAGE.into() | ||
| } else { | ||
| tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into() | ||
| }; | ||
| let query_str = if is_javascript { | ||
| r" | ||
| (variable_declarator | ||
| name: (identifier) @name | ||
| value: (string (string_fragment) @value) | ||
| ) | ||
| (variable_declarator | ||
| name: (identifier) @object_name | ||
| value: (object | ||
| (pair | ||
| key: (property_identifier) @prop_name | ||
| value: (string (string_fragment) @value) | ||
| ) | ||
| ) | ||
| ) | ||
| " | ||
| } else { | ||
| r" | ||
| (variable_declarator | ||
| name: (identifier) @name | ||
| value: [ | ||
| (string (string_fragment) @value) | ||
| (as_expression (string (string_fragment) @value)) | ||
| (type_assertion (string (string_fragment) @value)) | ||
| ] | ||
| ) | ||
| (variable_declarator | ||
| name: (identifier) @object_name | ||
| value: (object | ||
| (pair | ||
| key: (property_identifier) @prop_name | ||
| value: [ | ||
| (string (string_fragment) @value) | ||
| (as_expression (string (string_fragment) @value)) | ||
| (type_assertion (string (string_fragment) @value)) | ||
| ] | ||
| ) | ||
| ) | ||
| ) | ||
| " | ||
| }; | ||
| match Query::new(&ts_lang, query_str) { | ||
| Ok(query) => { | ||
| let mut cursor = QueryCursor::new(); | ||
| let bytes = content.as_bytes(); | ||
| let mut matches = cursor.matches(&query, root, bytes); | ||
| let name_idx = query.capture_index_for_name("name"); | ||
| let object_name_idx = query.capture_index_for_name("object_name"); | ||
| let prop_name_idx = query.capture_index_for_name("prop_name"); | ||
| let value_idx = query.capture_index_for_name("value"); | ||
|
|
||
| while let Some(m) = matches.next() { | ||
| if let Some(val_cap) = find_capture(m, value_idx) { | ||
| let value = val_cap | ||
| .node | ||
| .utf8_text(bytes) | ||
| .unwrap_or_default() | ||
| .to_string(); | ||
|
|
||
| if let Some(name_cap) = find_capture(m, name_idx) { | ||
| let name = name_cap | ||
| .node | ||
| .utf8_text(bytes) | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
Compiling tree-sitter queries on every single file parse is inefficient. We can use std::sync::LazyLock to compile the queries exactly once on first use, which significantly improves performance.
pub fn extract_js_constants(
root: tree_sitter::Node<'_>,
content: &str,
is_javascript: bool,
) -> std::collections::HashMap<String, String> {
static JS_CONSTANTS_QUERY: std::sync::LazyLock<Query> = std::sync::LazyLock::new(|| {
let ts_lang = tree_sitter_javascript::LANGUAGE.into();
Query::new(
&ts_lang,
r"
(variable_declarator
name: (identifier) @name
value: (string (string_fragment) @value)
)
(variable_declarator
name: (identifier) @object_name
value: (object
(pair
key: (property_identifier) @prop_name
value: (string (string_fragment) @value)
)
)
)
",
)
.expect("Failed to parse JS constants query")
});
static TS_CONSTANTS_QUERY: std::sync::LazyLock<Query> = std::sync::LazyLock::new(|| {
let ts_lang = tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into();
Query::new(
&ts_lang,
r"
(variable_declarator
name: (identifier) @name
value: [
(string (string_fragment) @value)
(as_expression (string (string_fragment) @value))
(type_assertion (string (string_fragment) @value))
]
)
(variable_declarator
name: (identifier) @object_name
value: (object
(pair
key: (property_identifier) @prop_name
value: [
(string (string_fragment) @value)
(as_expression (string (string_fragment) @value))
(type_assertion (string (string_fragment) @value))
]
)
)
)
",
)
.expect("Failed to parse TS constants query")
});
let mut constants = std::collections::HashMap::new();
let query = if is_javascript {
&JS_CONSTANTS_QUERY
} else {
&TS_CONSTANTS_QUERY
};
let mut cursor = QueryCursor::new();
let bytes = content.as_bytes();
let mut matches = cursor.matches(query, root, bytes);
let name_idx = query.capture_index_for_name("name");
let object_name_idx = query.capture_index_for_name("object_name");
let prop_name_idx = query.capture_index_for_name("prop_name");
let value_idx = query.capture_index_for_name("value");
while let Some(m) = matches.next() {
if let Some(val_cap) = find_capture(m, value_idx) {
let value = val_cap.node.utf8_text(bytes).unwrap_or_default().to_string();
if let Some(name_cap) = find_capture(m, name_idx) {
let name = name_cap.node.utf8_text(bytes).unwrap_or_default().to_string();
constants.insert(name, value);
} else if let (Some(obj_cap), Some(prop_cap)) = (find_capture(m, object_name_idx), find_capture(m, prop_name_idx)) {
let obj_name = obj_cap.node.utf8_text(bytes).unwrap_or_default();
let prop_name = prop_cap.node.utf8_text(bytes).unwrap_or_default();
let full_key = format!("{obj_name}.{prop_name}");
constants.insert(full_key, value.clone());
constants.insert(prop_name.to_string(), value);
}
}
}
constants
}…exing
Add a two-pass workspace scan: Pass 1 collects all constants from every
file and stores them in the ProjectIndex before any parsing begins.
Pass 2 runs full parsing with the complete global constants map so
cross-file constant references in event names are resolved correctly.
- Add `rust_constants` / `js_constants` DashMaps to ProjectIndex with
per-file reverse indexes for stale removal
- Add `collect_constants_from_file` in file_processor for the first pass
- Thread `global_constants: &HashMap<String, String>` through
`parse`, `parse_rust_full`, `parse_frontend`, and
`extract_rust_findings`; local constants take priority, globals fill gaps
- Guard `did_open`/`did_change` handlers with `constants_ready` AtomicBool
so files opened during startup are not re-indexed before Pass 1 completes
- Trigger `code_lens_refresh` after Pass 2 so editors pick up resolved names
No description provided.