diff --git a/src/graph/key.rs b/src/graph/key.rs index a91e3c8..a3309f0 100644 --- a/src/graph/key.rs +++ b/src/graph/key.rs @@ -90,6 +90,30 @@ pub enum NodeKey { }, } +/// Node-key type tags exactly as emitted by the [`fmt::Display`] implementation below. +/// Keep in sync with its match arms; `should_detect_every_display_tag_roundtrip` pins +/// the fixed tags (the custom and unresolved formats are intentionally excluded). +const TYPE_TAG_PREFIXES: &[&str] = &[ + "proc", "func", "mapper", "method", "class", "table", "view", "pkg", "trigger", "type", "seq", + "idx", "mview", "syn", "event", "builtin", "javasql", "jsp", "jspsql", +]; + +/// If `target` starts with `:`, return `(tag, rest)`. +/// +/// CLI target parsing uses this so `type:name` node keys resolve as whole keys and are +/// never mistaken for `table.column` targets (#154). +pub fn split_type_prefix(target: &str) -> Option<(&str, &str)> { + let (tag, rest) = target.split_once(':')?; + if rest.is_empty() + || !TYPE_TAG_PREFIXES + .iter() + .any(|t| tag.eq_ignore_ascii_case(t)) + { + return None; + } + Some((tag, rest)) +} + impl fmt::Display for NodeKey { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -354,3 +378,156 @@ impl NodeKey { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn should_detect_known_type_prefix() { + assert_eq!( + split_type_prefix("table:bigfund.mid_yjqs_detail"), + Some(("table", "bigfund.mid_yjqs_detail")) + ); + assert_eq!( + split_type_prefix("table:my_table"), + Some(("table", "my_table")) + ); + assert_eq!( + split_type_prefix("view:public.v1"), + Some(("view", "public.v1")) + ); + assert_eq!( + split_type_prefix("idx:mid_yjqs_detail[pk_mid_yjqs_detail]"), + Some(("idx", "mid_yjqs_detail[pk_mid_yjqs_detail]")) + ); + } + + #[test] + fn should_reject_unknown_or_empty_prefix() { + assert_eq!(split_type_prefix("weird:stuff"), None); + assert_eq!(split_type_prefix("table:"), None); + assert_eq!(split_type_prefix("my_table"), None); + assert_eq!(split_type_prefix("schema.table.column"), None); + } + + #[test] + fn should_detect_type_prefix_case_insensitively() { + assert_eq!( + split_type_prefix("Table:bigfund.mid"), + Some(("Table", "bigfund.mid")) + ); + assert_eq!( + split_type_prefix("VIEW:public.v1"), + Some(("VIEW", "public.v1")) + ); + } + + #[test] + fn should_detect_every_display_tag_roundtrip() { + let cases = [ + format!( + "{}", + NodeKey::Procedure { + schema: Some("s".into()), + package: None, + name: "p".into() + } + ), + format!( + "{}", + NodeKey::Function { + schema: Some("s".into()), + package: None, + name: "f".into() + } + ), + format!( + "{}", + NodeKey::Mapper { + namespace: "n".into(), + statement_id: "q".into() + } + ), + format!( + "{}", + NodeKey::JavaMethod { + fqn: "a.B.c".into() + } + ), + format!("{}", NodeKey::JavaClass { fqn: "a.B".into() }), + format!( + "{}", + NodeKey::Table { + schema: Some("s".into()), + name: "t".into() + } + ), + format!( + "{}", + NodeKey::View { + schema: Some("s".into()), + name: "v".into() + } + ), + format!( + "{}", + NodeKey::Package { + schema: Some("s".into()), + name: "pk".into() + } + ), + format!("{}", NodeKey::Trigger { name: "tg".into() }), + format!( + "{}", + NodeKey::Type { + schema: Some("s".into()), + name: "ty".into() + } + ), + format!( + "{}", + NodeKey::Sequence { + schema: Some("s".into()), + name: "sq".into() + } + ), + format!( + "{}", + NodeKey::Index { + table_name: "t".into(), + name: Some("ix".into()) + } + ), + format!( + "{}", + NodeKey::MaterializedView { + schema: Some("s".into()), + name: "mv".into() + } + ), + format!( + "{}", + NodeKey::Synonym { + schema: Some("s".into()), + name: "sy".into() + } + ), + format!("{}", NodeKey::Event { name: "ev".into() }), + format!("{}", NodeKey::BuiltinFunction { name: "bf".into() }), + format!( + "{}", + NodeKey::JavaSql { + file: "a.java".into(), + line: 1 + } + ), + ]; + for key in &cases { + assert!( + split_type_prefix(key).is_some(), + "tag not detected for Display key: {key}" + ); + } + } +} diff --git a/src/graph/lineage.rs b/src/graph/lineage.rs index 72543bc..9d94cb5 100644 --- a/src/graph/lineage.rs +++ b/src/graph/lineage.rs @@ -1498,13 +1498,22 @@ fn mappings_of_routine(graph: &CodeGraph, routine: NodeIndex) -> Vec Option { +/// other's, so an ambiguous bare name is reported as [`TableLookup::Ambiguous`]. +pub(crate) fn lookup_table_node(graph: &CodeGraph, name: &str) -> TableLookup { let name_of = |idx: &NodeIndex| match &graph[*idx] { crate::graph::Node::Table { schema, name, .. } | crate::graph::Node::View { schema, name, .. } => Some((schema.as_deref(), name.as_str())), @@ -1513,21 +1522,34 @@ fn find_table_node(graph: &CodeGraph, name: &str) -> Option { if let Some((schema, table)) = name.rsplit_once('.') { if !schema.is_empty() { - return graph.node_indices().find(|idx| { - name_of(idx) - .is_some_and(|(s, n)| n.eq_ignore_ascii_case(table) && s == Some(schema)) - }); + return match graph.node_indices().find(|idx| { + name_of(idx).is_some_and(|(s, n)| { + n.eq_ignore_ascii_case(table) && s.is_some_and(|s| eq(s, schema)) + }) + }) { + Some(idx) => TableLookup::Found(idx), + None => TableLookup::Missing, + }; } } let mut matches = graph .node_indices() .filter(|idx| name_of(idx).is_some_and(|(_, n)| n.eq_ignore_ascii_case(name))); - let first = matches.next()?; + let Some(first) = matches.next() else { + return TableLookup::Missing; + }; if matches.next().is_some() { - return None; + return TableLookup::Ambiguous; + } + TableLookup::Found(first) +} + +pub(crate) fn find_table_node(graph: &CodeGraph, name: &str) -> Option { + match lookup_table_node(graph, name) { + TableLookup::Found(idx) => Some(idx), + TableLookup::Ambiguous | TableLookup::Missing => None, } - Some(first) } fn eq(a: &str, b: &str) -> bool { diff --git a/src/main.rs b/src/main.rs index 80f1856..b272249 100644 --- a/src/main.rs +++ b/src/main.rs @@ -378,7 +378,9 @@ enum Commands { /// Table-level and column-level lineage analysis Lineage { - /// Target table name (e.g., "my_table") for table-level, or "table.column" for column-level + /// Target for lineage: "my_table" (table-level), "table.column" or + /// "schema.table.column" (column-level), or a node key like + /// "table:schema.table" (table-level, same grammar as trace/detail) target: String, /// Lineage direction: upstream (who writes/defines), downstream (who consumes), @@ -1556,19 +1558,49 @@ fn cmd_lineage( ); } - // `table` traces the table; `table.column` traces one column through it. Split on the - // last `.` so a schema-qualified `schema.table.column` keeps `schema.table` as the - // table part and only the final component as the column. - let (table_name, column_name) = match target.rsplit_once('.') { - Some((table, column)) if !table.is_empty() && !column.is_empty() => (table, Some(column)), - Some(_) => { - eprintln!( - "Invalid target format: {}. Use 'table' or 'table.column'", - target - ); - return Ok(()); + // Node keys (`table:schema.table`) must not be last-dot-split — the final dot is + // part of the key, not a `table.column` separator. + let (table_name, column_name) = if graph::key::split_type_prefix(target).is_some() { + (target, None) + } else { + match target.rsplit_once('.') { + Some((table, column)) if !table.is_empty() && !column.is_empty() => { + (table, Some(column)) + } + Some(_) => { + eprintln!( + "Invalid target format: {}. Use 'table', 'table.column', or a node key like 'table:schema.table'", + target + ); + return Ok(()); + } + None => (target, None), } - None => (target, None), + }; + + // A missing table half means the split was probably `schema.table`: reinterpret the + // whole target as a table reference. An ambiguous half stops with a qualifier hint. + let (table_name, column_name) = match column_name { + Some(column) => match graph::lineage::lookup_table_node(graph, table_name) { + graph::lineage::TableLookup::Found(_) => (table_name, Some(column)), + graph::lineage::TableLookup::Ambiguous => { + eprintln!( + "error: table '{}' is ambiguous across schemas — qualify it as \ + 'schema.{table_name}' for table-level, or 'schema.{table_name}.{column}' \ + for column-level lineage", + table_name + ); + return Ok(()); + } + graph::lineage::TableLookup::Missing => { + eprintln!( + "note: no table '{}' found — interpreting '{}' as a table reference", + table_name, target + ); + (target, None) + } + }, + None => (table_name, None), }; // Parse direction up front — both the table and column paths need it. `None` means diff --git a/tests/regress_issue_154_lineage_targets.rs b/tests/regress_issue_154_lineage_targets.rs new file mode 100644 index 0000000..016403c --- /dev/null +++ b/tests/regress_issue_154_lineage_targets.rs @@ -0,0 +1,253 @@ +//! Issue #154: lineage target syntax. +//! - Node key `table:schema.table` must use table-level lineage. +//! - Bare `schema.table` falls back to table-level with a transparent note. +//! - Existing `table.column` / `schema.table.column` behavior remains unchanged. + +use std::fs; +use std::path::Path; +use tempfile::TempDir; + +fn codeweb_bin() -> std::path::PathBuf { + let base = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("target"); + let bin_name = if cfg!(windows) { + "codeweb.exe" + } else { + "codeweb" + }; + if let Ok(entries) = std::fs::read_dir(&base) { + for entry in entries.flatten() { + let p = entry.path().join("debug").join(bin_name); + if p.exists() { + return p; + } + } + } + base.join("debug").join(bin_name) +} + +fn run_codeweb_in(cwd: &Path, args: &[&str]) -> std::process::Output { + std::process::Command::new(codeweb_bin()) + .args(args) + .current_dir(cwd) + .output() + .expect("failed to run codeweb") +} + +fn project_with_sql(dir: &TempDir, sql: &str) -> std::path::PathBuf { + let root = dir.path().to_path_buf(); + let src = root.join("src"); + fs::create_dir_all(&src).unwrap(); + fs::write(src.join("t.sql"), sql).unwrap(); + + let out = run_codeweb_in( + &root, + &["init", "issue-154", "--dir", src.to_str().unwrap()], + ); + assert!( + out.status.success(), + "init failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + root +} + +const FIXTURE_SQL: &str = r#" +CREATE SCHEMA bigfund; +CREATE TABLE bigfund.mid_yjqs_detail(id NUMBER, amt NUMBER); +CREATE TABLE bigfund.out_detail(id NUMBER, amt NUMBER); +CREATE PROCEDURE bigfund.prc_load AS BEGIN + UPDATE bigfund.mid_yjqs_detail SET amt = 0; + INSERT INTO bigfund.out_detail SELECT id, amt FROM bigfund.mid_yjqs_detail; +END; +"#; + +#[test] +fn should_treat_nodekey_target_as_table_level_lineage() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "table:bigfund.mid_yjqs_detail", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "lineage failed: {stderr}"); + assert!( + stdout.contains("prc_load"), + "expected table-level lineage mentioning the writer proc, got:\n{stdout}" + ); + assert!( + !stderr.contains("No column lineage"), + "must not hit the column-level branch, stderr:\n{stderr}" + ); +} + +#[test] +fn should_fall_back_to_table_level_for_bare_schema_qualified_target() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "bigfund.mid_yjqs_detail", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "lineage failed: {stderr}"); + assert!( + stdout.contains("prc_load"), + "bare schema.table should fall back to table-level lineage, got:\n{stdout}" + ); + assert!( + stderr.contains("interpreting") && stderr.contains("bigfund.mid_yjqs_detail"), + "fallback must emit a transparent note, stderr:\n{stderr}" + ); +} + +#[test] +fn should_keep_column_level_for_existing_table_and_column() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "bigfund.mid_yjqs_detail.amt", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(out.status.success()); + assert!( + stdout.contains("bigfund.mid_yjqs_detail.amt"), + "column-level root line expected, got:\n{stdout}" + ); +} + +#[test] +fn should_resolve_column_query_with_differently_cased_schema() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "Bigfund.mid_yjqs_detail.amt", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("interpreting"), + "schema casing must not trigger table fallback, stderr:\n{stderr}" + ); + assert!( + stdout.contains("Bigfund.mid_yjqs_detail.amt"), + "column-level root line expected, got:\n{stdout}" + ); +} + +#[test] +fn should_keep_no_column_lineage_hint_when_table_exists_but_column_unknown() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "bigfund.mid_yjqs_detail.nonexistent", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success()); + assert!(stdout.contains("bigfund.mid_yjqs_detail.nonexistent")); + assert!( + stderr.contains("No column lineage"), + "column branch must stay when the table resolves, stderr:\n{stderr}" + ); +} + +#[test] +fn should_report_clean_error_for_unknown_nodekey_target() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql(&tmp, FIXTURE_SQL); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "table:bigfund.missing_table", + "-p", + root.to_str().unwrap(), + ], + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + !stderr.contains("No column lineage"), + "node-key miss must not produce column-branch noise, stderr:\n{stderr}" + ); + assert!( + stderr.contains("No table found matching"), + "expected table-resolution error, stderr:\n{stderr}" + ); +} + +#[test] +fn should_say_ambiguous_when_table_half_is_ambiguous() { + let tmp = TempDir::new().unwrap(); + let root = project_with_sql( + &tmp, + r#" +CREATE SCHEMA bigfund; +CREATE SCHEMA archive; +CREATE TABLE bigfund.mid_yjqs_detail(id NUMBER); +CREATE TABLE archive.mid_yjqs_detail(id NUMBER); +"#, + ); + let out = run_codeweb_in( + &root, + &[ + "lineage", + "mid_yjqs_detail.nonexistent_col", + "-p", + root.to_str().unwrap(), + ], + ); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success()); + assert!( + stderr.contains("ambiguous across schemas"), + "expected ambiguous-schema error, got: {stderr}" + ); + assert!( + stderr.contains("qualify"), + "expected schema qualification hint, got: {stderr}" + ); + assert!( + !stderr.contains("interpreting"), + "ambiguous must not fall back, stderr: {stderr}" + ); + assert!( + !stderr.contains("No table found matching"), + "dead-end fallback must be gone, stderr: {stderr}" + ); + assert!( + stdout.trim().is_empty(), + "no lineage output expected, stdout: {stdout}" + ); +}