diff --git a/crates/codegraph-core/src/ast_analysis/engine.rs b/crates/codegraph-core/src/ast_analysis/engine.rs index fddfed711..0b302012b 100644 --- a/crates/codegraph-core/src/ast_analysis/engine.rs +++ b/crates/codegraph-core/src/ast_analysis/engine.rs @@ -11,16 +11,48 @@ use crate::ast_analysis::cfg::{build_function_cfg, get_cfg_rules}; use crate::ast_analysis::complexity::{compute_all_metrics, lang_rules}; use crate::ast_analysis::dataflow::extract_dataflow; use crate::domain::parser::LanguageKind; +use crate::extractors::julia::signature_call; +use crate::extractors::r_lang::assigned_function_name; use crate::shared::constants::MAX_WALK_DEPTH; use crate::types::{DataflowResult, FunctionCfgResult, FunctionComplexityResult}; -/// Extract the name of a function/method node via the "name" field. -fn function_name(node: &Node, source: &[u8]) -> String { +/// Fallback name lookup via the node's own direct `name` field, for +/// languages whose function nodes actually carry one. +fn generic_function_name(node: &Node, source: &[u8]) -> String { node.child_by_field_name("name") .map(|n| n.utf8_text(source).unwrap_or("").to_string()) .unwrap_or_else(|| "".to_string()) } +/// Extract the name of a function/method node. +/// +/// Most languages' function nodes carry a direct `name` field, but Julia and +/// R do not (issue #2471) — their real extractors (`extractors/julia.rs`, +/// `extractors/r_lang.rs`) already resolve names correctly for these +/// languages; this reuses that exact logic instead of re-deriving it, so the +/// two can't silently drift apart on what counts as a function's name. +/// +/// R is never allowed to fall through to `generic_function_name`: confirmed +/// by direct inspection that tree-sitter-r's grammar *does* define a `name` +/// field on `function_definition` — but it points at the literal `function` +/// keyword token, not an identifier (R has no named-function-definition +/// syntax at all; every function is an anonymous expression that only +/// acquires a name via assignment). Falling through there wouldn't report +/// "" as this issue originally assumed — it would report the +/// literal string "function" for every unnamed R function, which is a more +/// actively misleading result than a missing name. +fn function_name(node: &Node, source: &[u8], lang_id: &str) -> String { + match lang_id { + "julia" => signature_call(node) + .and_then(|call_sig| call_sig.child(0)) + .and_then(|name_node| name_node.utf8_text(source).ok()) + .map(|s| s.to_string()) + .unwrap_or_else(|| generic_function_name(node, source)), + "r" => assigned_function_name(node, source).unwrap_or_else(|| "".to_string()), + _ => generic_function_name(node, source), + } +} + /// Collect all function/method nodes from the AST using a DFS walk. /// Uses the complexity rules' `function_nodes` list to identify function node types. fn collect_function_nodes<'a>( @@ -87,7 +119,7 @@ pub fn analyze_complexity_standalone( .into_iter() .filter_map(|node| { let metrics = compute_all_metrics(&node, source_bytes, lang_id)?; - let name = function_name(&node, source_bytes); + let name = function_name(&node, source_bytes, lang_id); let line = node.start_position().row as u32 + 1; let column = Some(node.start_position().column as u32); let end_line = Some(node.end_position().row as u32 + 1); @@ -132,7 +164,7 @@ pub fn build_cfg_standalone( .into_iter() .filter_map(|node| { let cfg = build_function_cfg(&node, lang_id, source_bytes)?; - let name = function_name(&node, source_bytes); + let name = function_name(&node, source_bytes, lang_id); let line = node.start_position().row as u32 + 1; let column = Some(node.start_position().column as u32); let end_line = Some(node.end_position().row as u32 + 1); @@ -191,4 +223,56 @@ mod tests { assert!(a.is_some(), "expected a result at line 2, column 10"); assert!(b.is_some(), "expected a result at line 2, column 25"); } + + // #2471: Julia and R function_definition nodes carry no direct `name` + // field, unlike most other languages this crate supports — the generic + // function_name() fallback used to silently report a wrong name for + // every function in these two languages (Julia: ""; R: + // actively worse — the literal string "function", since tree-sitter-r's + // grammar happens to define a `name` field pointing at the keyword + // token itself, not an identifier). + // + // Only `analyze_complexity_standalone` is exercised here for Julia/R — + // `get_cfg_rules` (ast_analysis/cfg.rs) has no entry for either language + // at all, so `build_cfg_standalone` correctly returns zero results for + // both regardless of this fix; that's a separate, pre-existing, and + // out-of-scope gap (CFG support for Julia/R hasn't been built yet), not + // something this issue's name-resolution fix touches. + #[test] + fn analyze_complexity_standalone_resolves_julia_function_name() { + let results = analyze_complexity_standalone( + "function greet(name)\n return \"hello, \" * name\nend", + "test.jl", + Some("julia"), + ); + assert_eq!(results.len(), 1); + assert_eq!(results[0].name, "greet"); + } + + #[test] + fn analyze_complexity_standalone_resolves_r_function_name() { + let results = analyze_complexity_standalone( + "greet <- function(name) {\n return(paste(\"hello,\", name))\n}", + "test.r", + Some("r"), + ); + assert_eq!(results.len(), 1); + assert_eq!(results[0].name, "greet"); + } + + #[test] + fn analyze_complexity_standalone_r_anonymous_function_reports_anonymous_not_the_function_keyword( + ) { + // A function_definition with no enclosing name-assigning + // binary_operator (an inline callback) has no name to resolve — must + // report "", NOT fall through to generic_function_name's + // child_by_field_name("name") lookup, which for R returns the + // literal "function" keyword token rather than None (confirmed by + // direct AST inspection — R's grammar defines that field, just not + // with the meaning this generic fallback assumes). + let results = + analyze_complexity_standalone("lapply(x, function(y) y + 1)", "test.r", Some("r")); + assert_eq!(results.len(), 1); + assert_eq!(results[0].name, ""); + } } diff --git a/crates/codegraph-core/src/extractors/julia.rs b/crates/codegraph-core/src/extractors/julia.rs index 169c8992d..31048e619 100644 --- a/crates/codegraph-core/src/extractors/julia.rs +++ b/crates/codegraph-core/src/extractors/julia.rs @@ -88,7 +88,7 @@ fn handle_module_def(node: &Node, source: &[u8], symbols: &mut FileSymbols) -> O /// would silently match the first body call_expression and mis-record the /// function name. Callers must therefore treat a missing `signature` as a /// parser/grammar mismatch worth investigating, not as a routine code path. -fn signature_call<'a>(node: &Node<'a>) -> Option> { +pub(crate) fn signature_call<'a>(node: &Node<'a>) -> Option> { if let Some(sig) = find_child(node, "signature") { return find_child(&sig, "call_expression"); } diff --git a/crates/codegraph-core/src/extractors/r_lang.rs b/crates/codegraph-core/src/extractors/r_lang.rs index 32f0fa925..87b5d58ee 100644 --- a/crates/codegraph-core/src/extractors/r_lang.rs +++ b/crates/codegraph-core/src/extractors/r_lang.rs @@ -106,6 +106,42 @@ fn is_program_level(node: &Node) -> bool { .unwrap_or(false) } +/// Resolve the name of a `function_definition` node from its enclosing +/// `binary_operator` assignment (`name <- function(...) {...}`) — R's +/// `function_definition` node itself carries no `name` field, unlike most +/// other languages this crate supports (issue #2471). Mirrors the same +/// operator-set and identifier-kind validation `handle_binary_op` above +/// already applies when it independently discovers the same shape top-down +/// (parent -> child); this is the bottom-up direction (child -> parent), +/// needed by the standalone complexity/CFG analysis in `ast_analysis::engine`, +/// which is handed a bare `function_definition` node with no assignment +/// context threaded through. +/// +/// Returns `None` when the function isn't the RHS of a valid name-assigning +/// binary_operator — e.g. an anonymous function expression passed inline to +/// `lapply(x, function(y) y + 1)` — matching `handle_binary_op`'s own silent +/// no-op for those same shapes. +pub(crate) fn assigned_function_name(function_def: &Node, source: &[u8]) -> Option { + let parent = function_def.parent()?; + if parent.kind() != "binary_operator" { + return None; + } + let lhs = parent + .child_by_field_name("lhs") + .or_else(|| parent.child(0))?; + let op = parent + .child_by_field_name("operator") + .or_else(|| parent.child(1))?; + let op_text = node_text(&op, source); + if op_text != "<-" && op_text != "=" && op_text != "<<-" { + return None; + } + if lhs.kind() != "identifier" { + return None; + } + Some(node_text(&lhs, source).to_string()) +} + fn extract_r_params(func_def: &Node, source: &[u8]) -> Vec { let mut params = Vec::new(); let params_node = match func_def.child_by_field_name("parameters") { diff --git a/tests/unit/native-analysis.test.ts b/tests/unit/native-analysis.test.ts index 7586f8db8..6ae3a957f 100644 --- a/tests/unit/native-analysis.test.ts +++ b/tests/unit/native-analysis.test.ts @@ -87,15 +87,12 @@ function complex(x) { it('works for Julia, including the &&/|| operator-by-text fix (#2312)', () => { const source = `function classify(x)\n if x > 0 && x < 10\n return 1\n elseif x < 0\n return -1\n else\n return 0\n end\nend\n`; const results = native.analyzeComplexity(source, 'test.jl', 'julia'); - // NOTE: `name` resolves to "" here — this standalone helper's - // generic `function_name` only checks a direct `name` field, which - // Julia's `function_definition` doesn't have (the name is nested under - // `signature` → `call_expression`, see `extractors/julia.rs`). This is - // a pre-existing gap shared by every language with the same shape - // (e.g. R) and is unrelated to complexity/Halstead correctness — out - // of scope for issue #2312. expect(results.length).toBe(1); const classify = results[0]!; + // #2471: `name` resolves the real function name via `signature_call` + // (extractors/julia.rs), not the generic direct-`name`-field fallback + // Julia's `function_definition` doesn't have. + expect(classify.name).toBe('classify'); // if: +1 cog, +1 cyc; &&: +1 cog, +1 cyc; elseif: +1 cog, +1 cyc; else: +1 cog expect(classify.complexity.cognitive).toBe(4); expect(classify.complexity.cyclomatic).toBe(4); @@ -104,6 +101,18 @@ function complex(x) { expect(classify.complexity.halstead!.n1).toBeGreaterThanOrEqual(3); }); + it('resolves the real function name for R (#2471)', () => { + // R's `function_definition` has no `name` field at all — worse than + // Julia's case, the generic fallback's `child_by_field_name("name")` + // actively matches tree-sitter-r's grammar's own field pointing at the + // literal `function` keyword token, so the pre-fix result was the + // string "function", not merely "". + const source = `greet <- function(name) {\n if (nchar(name) > 0) {\n return(paste("hello,", name))\n }\n return("hello")\n}`; + const results = native.analyzeComplexity(source, 'test.r', 'r'); + expect(results.length).toBe(1); + expect(results[0]!.name).toBe('greet'); + }); + it('works for Solidity, including the else-if transparent-wrapper fix (#2312)', () => { const source = 'contract C { function f(int x, int y) public { if (x > 0) { x = 1; } else if (y > 0) { x = 2; } else { x = 3; } } }';