From 1bf55fa52d323fdcd7642a0aeff1b740f8e8588a Mon Sep 17 00:00:00 2001 From: andrew Date: Mon, 13 Apr 2026 00:37:44 +0300 Subject: [PATCH] Add query lint and check commands --- crates/omnigraph-cli/src/main.rs | 180 ++++++- crates/omnigraph-cli/tests/cli.rs | 306 +++++++++++ crates/omnigraph-compiler/src/lib.rs | 5 + crates/omnigraph-compiler/src/query/lint.rs | 562 ++++++++++++++++++++ crates/omnigraph-compiler/src/query/mod.rs | 1 + docs/cli.md | 7 + og-cheet-sheet.md | 36 ++ 7 files changed, 1088 insertions(+), 9 deletions(-) create mode 100644 crates/omnigraph-compiler/src/query/lint.rs create mode 100644 og-cheet-sheet.md diff --git a/crates/omnigraph-cli/src/main.rs b/crates/omnigraph-cli/src/main.rs index 3bbe4a0..1ecf23f 100644 --- a/crates/omnigraph-cli/src/main.rs +++ b/crates/omnigraph-cli/src/main.rs @@ -7,9 +7,13 @@ use clap::{Arg, ArgAction, Args, CommandFactory, FromArgMatches, Parser, Subcomm use color_eyre::eyre::{Result, bail}; use omnigraph::db::{Omnigraph, ReadTarget, RunId, SnapshotId}; use omnigraph::loader::LoadMode; -use omnigraph_compiler::json_params_to_param_map; use omnigraph_compiler::query::parser::parse_query; -use omnigraph_compiler::{JsonParamMode, ParamMap, SchemaMigrationPlan, SchemaMigrationStep}; +use omnigraph_compiler::schema::parser::parse_schema; +use omnigraph_compiler::{ + JsonParamMode, ParamMap, QueryLintOutput, QueryLintQueryKind, QueryLintSchemaSource, + QueryLintSeverity, QueryLintStatus, SchemaMigrationPlan, SchemaMigrationStep, build_catalog, + json_params_to_param_map, lint_query_file, +}; use omnigraph_server::api::{ BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput, BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput, @@ -104,6 +108,11 @@ enum Command { #[command(subcommand)] command: SchemaCommand, }, + /// Query validation and linting + Query { + #[command(subcommand)] + command: QueryCommand, + }, /// Show repo snapshot Snapshot { /// Repo URI @@ -296,6 +305,26 @@ enum SchemaCommand { }, } +#[derive(Debug, Subcommand)] +enum QueryCommand { + /// Validate queries and report higher-level drift warnings + #[command(visible_alias = "check")] + Lint { + /// Repo URI + uri: Option, + #[arg(long)] + target: Option, + #[arg(long)] + config: Option, + #[arg(long)] + query: PathBuf, + #[arg(long)] + schema: Option, + #[arg(long)] + json: bool, + }, +} + #[derive(Debug, Subcommand)] enum RunCommand { /// List transactional runs @@ -476,6 +505,70 @@ fn print_schema_apply_human(output: &SchemaApplyOutput) { } } +fn query_kind_label(kind: QueryLintQueryKind) -> &'static str { + match kind { + QueryLintQueryKind::Read => "read", + QueryLintQueryKind::Mutation => "mutation", + } +} + +fn severity_label(severity: QueryLintSeverity) -> &'static str { + match severity { + QueryLintSeverity::Error => "ERROR", + QueryLintSeverity::Warning => "WARN ", + QueryLintSeverity::Info => "INFO ", + } +} + +fn print_query_lint_human(output: &QueryLintOutput) { + for result in &output.results { + match result.status { + QueryLintStatus::Ok => { + println!( + "OK query `{}` ({})", + result.name, + query_kind_label(result.kind) + ); + } + QueryLintStatus::Error => { + println!( + "ERROR query `{}`: {}", + result.name, + result.error.as_deref().unwrap_or("unknown error") + ); + } + } + + for warning in &result.warnings { + println!("WARN query `{}`: {}", result.name, warning); + } + } + + for finding in &output.findings { + println!("{} {}", severity_label(finding.severity), finding.message); + } + + println!( + "INFO Lint complete: {} queries processed ({} error(s), {} warning(s), {} info item(s))", + output.queries_processed, output.errors, output.warnings, output.infos + ); +} + +fn finish_query_lint(output: &QueryLintOutput, json: bool) -> Result<()> { + if json { + print_json(output)?; + } else { + print_query_lint_human(output); + } + + if output.status == QueryLintStatus::Error { + io::stdout().flush()?; + std::process::exit(1); + } + + Ok(()) +} + fn ensure_local_repo_parent(uri: &str) -> Result<()> { if !uri.contains("://") { fs::create_dir_all(uri)?; @@ -735,18 +828,30 @@ fn resolve_read_target( )) } +fn resolve_query_path( + config: &OmnigraphConfig, + explicit_query: Option<&PathBuf>, + alias_query: Option<&str>, +) -> Result { + explicit_query + .map(PathBuf::from) + .or_else(|| alias_query.map(PathBuf::from)) + .ok_or_else(|| { + color_eyre::eyre::eyre!("exactly one of --query or --alias must be provided") + }) + .and_then(|query_path| config.resolve_query_path(&query_path)) +} + fn resolve_query_source( config: &OmnigraphConfig, explicit_query: Option<&PathBuf>, alias_query: Option<&str>, ) -> Result { - let query_path = explicit_query - .map(PathBuf::from) - .or_else(|| alias_query.map(PathBuf::from)) - .ok_or_else(|| { - color_eyre::eyre::eyre!("exactly one of --query or --alias must be provided") - })?; - Ok(fs::read_to_string(config.resolve_query_path(&query_path)?)?) + Ok(fs::read_to_string(resolve_query_path( + config, + explicit_query, + alias_query, + )?)?) } fn parse_alias_value(value: &str) -> Value { @@ -1312,6 +1417,47 @@ fn query_params_from_json( .map_err(|err| color_eyre::eyre::eyre!(err.to_string())) } +async fn execute_query_lint( + config: &OmnigraphConfig, + cli_uri: Option, + cli_target: Option<&str>, + schema_path: Option<&PathBuf>, + query_path: &PathBuf, +) -> Result { + let resolved_query_path = resolve_query_path(config, Some(query_path), None)?; + let query_source = fs::read_to_string(&resolved_query_path)?; + let query_path = resolved_query_path.to_string_lossy().into_owned(); + + if let Some(schema_path) = schema_path { + let schema_source = fs::read_to_string(schema_path)?; + let schema = + parse_schema(&schema_source).map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?; + let catalog = + build_catalog(&schema).map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?; + return Ok(lint_query_file( + &catalog, + &query_source, + query_path, + QueryLintSchemaSource::file(schema_path.to_string_lossy().into_owned()), + )); + } + + let has_repo_target = + cli_uri.is_some() || cli_target.is_some() || config.cli_target_name().is_some(); + if !has_repo_target { + bail!("query lint requires --schema or a resolvable repo target"); + } + + let uri = resolve_local_uri(config, cli_uri, cli_target, "query lint")?; + let db = Omnigraph::open(&uri).await?; + Ok(lint_query_file( + db.catalog(), + &query_source, + query_path, + QueryLintSchemaSource::repo(uri), + )) +} + async fn execute_read( uri: &str, query_source: &str, @@ -1858,6 +2004,22 @@ async fn main() -> Result<()> { } } }, + Command::Query { command } => match command { + QueryCommand::Lint { + uri, + target, + config, + query, + schema, + json, + } => { + let config = load_cli_config(config.as_ref())?; + let output = + execute_query_lint(&config, uri, target.as_deref(), schema.as_ref(), &query) + .await?; + finish_query_lint(&output, json)?; + } + }, Command::Snapshot { uri, target, diff --git a/crates/omnigraph-cli/tests/cli.rs b/crates/omnigraph-cli/tests/cli.rs index b6925e5..b34793a 100644 --- a/crates/omnigraph-cli/tests/cli.rs +++ b/crates/omnigraph-cli/tests/cli.rs @@ -537,6 +537,312 @@ fn schema_apply_rejects_when_non_main_branch_exists() { assert!(stderr.contains("schema apply requires a repo with only main")); } +#[test] +fn query_lint_json_with_schema_reports_warnings() { + let temp = tempdir().unwrap(); + let schema_path = temp.path().join("schema.pg"); + let query_path = temp.path().join("queries.gq"); + write_file( + &schema_path, + r#" +node Policy { + slug: String @key + name: String? + effectiveTo: DateTime? +} +"#, + ); + write_query_file( + &query_path, + r#" +query update_policy($slug: String, $name: String) { + update Policy set { name: $name } where slug = $slug +} +"#, + ); + + let output = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); + + assert_eq!(payload["status"], "ok"); + assert_eq!(payload["schema_source"]["kind"], "file"); + assert_eq!(payload["queries_processed"], 1); + assert_eq!(payload["warnings"], 1); + assert_eq!(payload["findings"][0]["code"], "L201"); + assert_eq!( + payload["findings"][0]["message"], + "Policy.effectiveTo exists in schema but no update query sets it" + ); +} + +#[test] +fn query_check_alias_matches_lint_output() { + let temp = tempdir().unwrap(); + let schema_path = temp.path().join("schema.pg"); + let query_path = temp.path().join("queries.gq"); + write_file( + &schema_path, + r#" +node Person { + name: String +} +"#, + ); + write_query_file( + &query_path, + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + + let lint_output = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + let check_output = output_success( + cli() + .arg("query") + .arg("check") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path) + .arg("--json"), + ); + + assert_eq!(stdout_string(&lint_output), stdout_string(&check_output)); +} + +#[test] +fn query_lint_can_use_local_repo_via_positional_uri() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + let query_path = temp.path().join("queries.gq"); + init_repo(&repo); + write_query_file( + &query_path, + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + + let output = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--json") + .arg(&repo), + ); + let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); + + assert_eq!(payload["status"], "ok"); + assert_eq!(payload["schema_source"]["kind"], "repo"); + assert_eq!( + payload["schema_source"]["uri"].as_str(), + Some(repo.to_string_lossy().as_ref()) + ); +} + +#[test] +fn query_lint_can_resolve_repo_and_query_from_config() { + let temp = tempdir().unwrap(); + let repo = repo_path(temp.path()); + let config_path = temp.path().join("omnigraph.yaml"); + init_repo(&repo); + write_query_file( + &temp.path().join("queries.gq"), + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + write_config(&config_path, &local_yaml_config(&repo)); + + let output = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg("queries.gq") + .arg("--config") + .arg(&config_path) + .arg("--json"), + ); + let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); + + assert_eq!(payload["status"], "ok"); + assert_eq!(payload["schema_source"]["kind"], "repo"); + assert_eq!( + payload["schema_source"]["uri"].as_str(), + Some(repo.to_string_lossy().as_ref()) + ); +} + +#[test] +fn query_lint_rejects_http_targets_without_schema() { + let temp = tempdir().unwrap(); + let query_path = temp.path().join("queries.gq"); + write_query_file( + &query_path, + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + + let output = output_failure( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("http://127.0.0.1:8080"), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("query lint is only supported against local repo URIs in this milestone") + ); +} + +#[test] +fn query_lint_requires_schema_or_resolvable_repo_target() { + let temp = tempdir().unwrap(); + let query_path = temp.path().join("queries.gq"); + write_query_file( + &query_path, + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} +"#, + ); + + let output = output_failure( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("query lint requires --schema or a resolvable repo target") + ); +} + +#[test] +fn query_lint_human_output_reports_warnings() { + let temp = tempdir().unwrap(); + let schema_path = temp.path().join("schema.pg"); + let query_path = temp.path().join("queries.gq"); + write_file( + &schema_path, + r#" +node Policy { + slug: String @key + name: String? + effectiveTo: DateTime? +} +"#, + ); + write_query_file( + &query_path, + r#" +query update_policy($slug: String, $name: String) { + update Policy set { name: $name } where slug = $slug +} +"#, + ); + + let output = output_success( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path), + ); + let stdout = stdout_string(&output); + + assert!(stdout.contains("OK query `update_policy` (mutation)")); + assert!( + stdout.contains("WARN Policy.effectiveTo exists in schema but no update query sets it") + ); + assert!(stdout.contains( + "INFO Lint complete: 1 queries processed (0 error(s), 1 warning(s), 0 info item(s))" + )); +} + +#[test] +fn query_lint_human_output_reports_strict_validation_errors() { + let temp = tempdir().unwrap(); + let schema_path = temp.path().join("schema.pg"); + let query_path = temp.path().join("queries.gq"); + write_file( + &schema_path, + r#" +node Policy { + slug: String @key + name: String? +} +"#, + ); + write_query_file( + &query_path, + r#" +query bad_update($slug: String) { + update Policy set { priority_level: "high" } where slug = $slug +} +"#, + ); + + let output = output_failure( + cli() + .arg("query") + .arg("lint") + .arg("--query") + .arg(&query_path) + .arg("--schema") + .arg(&schema_path), + ); + let stdout = stdout_string(&output); + + assert!(stdout.contains("ERROR query `bad_update`:")); + assert!(stdout.contains("Policy")); + assert!(stdout.contains( + "INFO Lint complete: 1 queries processed (1 error(s), 0 warning(s), 0 info item(s))" + )); +} + #[test] fn load_json_outputs_summary_for_main_branch() { let temp = tempdir().unwrap(); diff --git a/crates/omnigraph-compiler/src/lib.rs b/crates/omnigraph-compiler/src/lib.rs index 3c63367..0b71ebd 100644 --- a/crates/omnigraph-compiler/src/lib.rs +++ b/crates/omnigraph-compiler/src/lib.rs @@ -20,6 +20,11 @@ pub use catalog::schema_plan::{ pub use ir::ParamMap; pub use ir::lower::{lower_mutation_query, lower_query}; pub use query::ast::Literal; +pub use query::lint::{ + QueryLintFinding, QueryLintOutput, QueryLintQueryKind, QueryLintQueryResult, + QueryLintSchemaSource, QueryLintSchemaSourceKind, QueryLintSeverity, QueryLintStatus, + lint_query_file, +}; pub use query_input::{ JsonParamMode, RunInputError, RunInputResult, ToParam, find_named_query, json_params_to_param_map, diff --git a/crates/omnigraph-compiler/src/query/lint.rs b/crates/omnigraph-compiler/src/query/lint.rs new file mode 100644 index 0000000..9805449 --- /dev/null +++ b/crates/omnigraph-compiler/src/query/lint.rs @@ -0,0 +1,562 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use serde::Serialize; + +use crate::catalog::Catalog; +use crate::query::ast::{Mutation, QueryDecl}; +use crate::query::parser::parse_query; +use crate::query::typecheck::typecheck_query_decl; + +const PARSE_ERROR_CODE: &str = "Q000"; +const L201_CODE: &str = "L201"; +const HARDCODED_MUTATION_WARNING: &str = + "mutation declares no params; hardcoded mutations are easy to miss"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum QueryLintStatus { + Ok, + Error, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum QueryLintSeverity { + Error, + Warning, + Info, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum QueryLintQueryKind { + Read, + Mutation, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum QueryLintSchemaSourceKind { + File, + Repo, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct QueryLintSchemaSource { + pub kind: QueryLintSchemaSourceKind, + #[serde(skip_serializing_if = "Option::is_none")] + pub path: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub uri: Option, +} + +impl QueryLintSchemaSource { + pub fn file(path: impl Into) -> Self { + Self { + kind: QueryLintSchemaSourceKind::File, + path: Some(path.into()), + uri: None, + } + } + + pub fn repo(uri: impl Into) -> Self { + Self { + kind: QueryLintSchemaSourceKind::Repo, + path: None, + uri: Some(uri.into()), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct QueryLintQueryResult { + pub name: String, + pub kind: QueryLintQueryKind, + pub status: QueryLintStatus, + #[serde(skip_serializing_if = "Vec::is_empty", default)] + pub warnings: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct QueryLintFinding { + pub severity: QueryLintSeverity, + pub code: String, + pub message: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub type_name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub property: Option, + #[serde(skip_serializing_if = "Vec::is_empty", default)] + pub query_names: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct QueryLintOutput { + pub status: QueryLintStatus, + pub schema_source: QueryLintSchemaSource, + pub query_path: String, + pub queries_processed: usize, + pub errors: usize, + pub warnings: usize, + pub infos: usize, + pub results: Vec, + pub findings: Vec, +} + +#[derive(Debug, Default)] +struct UpdateCoverage { + query_names: BTreeSet, + assigned_properties: BTreeSet, +} + +pub fn lint_query_file( + catalog: &Catalog, + query_source: &str, + query_path: impl Into, + schema_source: QueryLintSchemaSource, +) -> QueryLintOutput { + let query_path = query_path.into(); + match parse_query(query_source) { + Ok(parsed) => { + let queries_processed = parsed.queries.len(); + let mut results = Vec::with_capacity(queries_processed); + let mut coverage = BTreeMap::::new(); + + for query in &parsed.queries { + let kind = query_kind(query); + let warnings = per_query_warnings(query); + match typecheck_query_decl(catalog, query) { + Ok(_) => { + collect_update_coverage(query, &mut coverage); + results.push(QueryLintQueryResult { + name: query.name.clone(), + kind, + status: QueryLintStatus::Ok, + warnings, + error: None, + }); + } + Err(err) => { + results.push(QueryLintQueryResult { + name: query.name.clone(), + kind, + status: QueryLintStatus::Error, + warnings, + error: Some(err.to_string()), + }); + } + } + } + + let mut findings = lint_update_coverage(catalog, &coverage); + findings.sort_by(findings_cmp); + + let errors = results + .iter() + .filter(|result| result.status == QueryLintStatus::Error) + .count() + + findings + .iter() + .filter(|finding| finding.severity == QueryLintSeverity::Error) + .count(); + let warnings = results + .iter() + .map(|result| result.warnings.len()) + .sum::() + + findings + .iter() + .filter(|finding| finding.severity == QueryLintSeverity::Warning) + .count(); + let infos = findings + .iter() + .filter(|finding| finding.severity == QueryLintSeverity::Info) + .count(); + + QueryLintOutput { + status: if errors == 0 { + QueryLintStatus::Ok + } else { + QueryLintStatus::Error + }, + schema_source, + query_path, + queries_processed, + errors, + warnings, + infos, + results, + findings, + } + } + Err(err) => QueryLintOutput { + status: QueryLintStatus::Error, + schema_source, + query_path, + queries_processed: 0, + errors: 1, + warnings: 0, + infos: 0, + results: Vec::new(), + findings: vec![QueryLintFinding { + severity: QueryLintSeverity::Error, + code: PARSE_ERROR_CODE.to_string(), + message: err.to_string(), + type_name: None, + property: None, + query_names: Vec::new(), + }], + }, + } +} + +fn query_kind(query: &QueryDecl) -> QueryLintQueryKind { + if query.mutations.is_empty() { + QueryLintQueryKind::Read + } else { + QueryLintQueryKind::Mutation + } +} + +fn per_query_warnings(query: &QueryDecl) -> Vec { + if query.mutations.is_empty() || !query.params.is_empty() { + return Vec::new(); + } + vec![HARDCODED_MUTATION_WARNING.to_string()] +} + +fn collect_update_coverage(query: &QueryDecl, coverage: &mut BTreeMap) { + for mutation in &query.mutations { + if let Mutation::Update(update) = mutation { + let entry = coverage.entry(update.type_name.clone()).or_default(); + entry.query_names.insert(query.name.clone()); + for assignment in &update.assignments { + entry + .assigned_properties + .insert(assignment.property.clone()); + } + } + } +} + +fn lint_update_coverage( + catalog: &Catalog, + coverage: &BTreeMap, +) -> Vec { + let mut type_names = catalog.node_types.keys().cloned().collect::>(); + type_names.sort(); + + let mut findings = Vec::new(); + for type_name in type_names { + let Some(type_coverage) = coverage.get(&type_name) else { + continue; + }; + if type_coverage.query_names.is_empty() { + continue; + } + + let node_type = &catalog.node_types[&type_name]; + let key_properties = node_type.key.clone().unwrap_or_default(); + + let mut property_names = node_type.properties.keys().cloned().collect::>(); + property_names.sort(); + + for property_name in property_names { + let property = &node_type.properties[&property_name]; + if !property.nullable { + continue; + } + if key_properties.iter().any(|key| key == &property_name) { + continue; + } + if node_type.embed_sources.contains_key(&property_name) { + continue; + } + if type_coverage.assigned_properties.contains(&property_name) { + continue; + } + + findings.push(QueryLintFinding { + severity: QueryLintSeverity::Warning, + code: L201_CODE.to_string(), + message: format!( + "{}.{} exists in schema but no update query sets it", + type_name, property_name + ), + type_name: Some(type_name.clone()), + property: Some(property_name), + query_names: type_coverage.query_names.iter().cloned().collect(), + }); + } + } + findings +} + +fn findings_cmp(left: &QueryLintFinding, right: &QueryLintFinding) -> std::cmp::Ordering { + severity_rank(left.severity) + .cmp(&severity_rank(right.severity)) + .then_with(|| left.type_name.cmp(&right.type_name)) + .then_with(|| left.property.cmp(&right.property)) + .then_with(|| left.message.cmp(&right.message)) +} + +fn severity_rank(severity: QueryLintSeverity) -> u8 { + match severity { + QueryLintSeverity::Error => 0, + QueryLintSeverity::Warning => 1, + QueryLintSeverity::Info => 2, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::build_catalog; + use crate::schema::parser::parse_schema; + + fn catalog(schema: &str) -> Catalog { + let schema = parse_schema(schema).unwrap(); + build_catalog(&schema).unwrap() + } + + #[test] + fn parse_failure_returns_structured_error_output() { + let output = lint_query_file( + &catalog("node Person { name: String }"), + "query broken(", + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert_eq!(output.status, QueryLintStatus::Error); + assert_eq!(output.queries_processed, 0); + assert_eq!(output.errors, 1); + assert!(output.results.is_empty()); + assert_eq!(output.findings.len(), 1); + assert_eq!(output.findings[0].severity, QueryLintSeverity::Error); + assert_eq!(output.findings[0].code, PARSE_ERROR_CODE); + } + + #[test] + fn mixed_valid_and_invalid_queries_preserve_per_query_results() { + let output = lint_query_file( + &catalog( + r#" +node Person { + slug: String @key + name: String? +} +"#, + ), + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} + +query bad_update($slug: String) { + update Person set { missing: "nope" } where slug = $slug +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert_eq!(output.queries_processed, 2); + assert_eq!(output.results[0].name, "list_people"); + assert_eq!(output.results[0].status, QueryLintStatus::Ok); + assert_eq!(output.results[1].name, "bad_update"); + assert_eq!(output.results[1].status, QueryLintStatus::Error); + assert!( + output.results[1] + .error + .as_deref() + .unwrap_or_default() + .contains("has no property") + ); + } + + #[test] + fn hardcoded_mutation_warning_only_fires_for_mutation_queries() { + let output = lint_query_file( + &catalog( + r#" +node Person { + slug: String @key + name: String? +} +"#, + ), + r#" +query list_people() { + match { $p: Person } + return { $p.name } +} + +query insert_person() { + insert Person { slug: "p1", name: "P1" } +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert!(output.results[0].warnings.is_empty()); + assert_eq!( + output.results[1].warnings, + vec![HARDCODED_MUTATION_WARNING.to_string()] + ); + assert_eq!(output.warnings, 1); + } + + #[test] + fn l201_warns_for_nullable_uncovered_update_fields() { + let output = lint_query_file( + &catalog( + r#" +node Policy { + slug: String @key + name: String? + effectiveTo: DateTime? +} +"#, + ), + r#" +query update_policy($slug: String, $name: String) { + update Policy set { name: $name } where slug = $slug +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert_eq!(output.findings.len(), 1); + assert_eq!(output.findings[0].code, L201_CODE); + assert_eq!( + output.findings[0].message, + "Policy.effectiveTo exists in schema but no update query sets it" + ); + assert_eq!(output.findings[0].query_names, vec!["update_policy"]); + } + + #[test] + fn l201_does_not_fire_without_valid_update_queries() { + let output = lint_query_file( + &catalog( + r#" +node Policy { + slug: String @key + effectiveTo: DateTime? +} +"#, + ), + r#" +query insert_policy($slug: String) { + insert Policy { slug: $slug } +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert!(output.findings.is_empty()); + } + + #[test] + fn l201_excludes_embed_target_properties() { + let output = lint_query_file( + &catalog( + r#" +node Doc { + slug: String @key + body: String? + summary: String? + embedding: Vector(3)? @embed(body) +} +"#, + ), + r#" +query update_doc($slug: String, $body: String) { + update Doc set { body: $body } where slug = $slug +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert_eq!(output.findings.len(), 1); + assert_eq!(output.findings[0].property.as_deref(), Some("summary")); + } + + #[test] + fn l201_excludes_key_properties_even_if_catalog_is_modified() { + let mut catalog = catalog( + r#" +node Policy { + slug: String @key + name: String? +} +"#, + ); + catalog + .node_types + .get_mut("Policy") + .unwrap() + .properties + .get_mut("slug") + .unwrap() + .nullable = true; + + let output = lint_query_file( + &catalog, + r#" +query update_policy($slug: String, $name: String) { + update Policy set { name: $name } where slug = $slug +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert!( + output + .findings + .iter() + .all(|finding| finding.property.as_deref() != Some("slug")) + ); + } + + #[test] + fn findings_and_query_names_are_deterministic() { + let output = lint_query_file( + &catalog( + r#" +node Policy { + slug: String @key + c_field: String? + b_field: String? + a_field: String? +} +"#, + ), + &r#" +query update_b($slug: String) { + update Policy set { a_field: "x" } where slug = $slug +} + +query update_a($slug: String) { + update Policy set { a_field: "x" } where slug = $slug +} +"#, + "/tmp/queries.gq", + QueryLintSchemaSource::file("/tmp/schema.pg"), + ); + + assert_eq!(output.findings.len(), 2); + assert_eq!(output.findings[0].property.as_deref(), Some("b_field")); + assert_eq!(output.findings[1].property.as_deref(), Some("c_field")); + assert_eq!(output.findings[0].query_names, vec!["update_a", "update_b"]); + assert_eq!(output.findings[1].query_names, vec!["update_a", "update_b"]); + } +} diff --git a/crates/omnigraph-compiler/src/query/mod.rs b/crates/omnigraph-compiler/src/query/mod.rs index 7592221..792a274 100644 --- a/crates/omnigraph-compiler/src/query/mod.rs +++ b/crates/omnigraph-compiler/src/query/mod.rs @@ -1,3 +1,4 @@ pub mod ast; +pub mod lint; pub mod parser; pub mod typecheck; diff --git a/docs/cli.md b/docs/cli.md index afeae98..36d54a4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -47,6 +47,9 @@ and configure the matching `bearer_token_env` in `omnigraph.yaml`. ## Runs, Policy, And Diagnostics ```bash +omnigraph query lint --query ./queries.gq --schema ./schema.pg --json +omnigraph query check --query ./queries.gq ./repo.omni --json + omnigraph schema plan --schema ./next.pg ./repo.omni --json omnigraph schema apply --schema ./next.pg ./repo.omni --json omnigraph policy validate --config ./omnigraph.yaml @@ -59,6 +62,10 @@ omnigraph run publish --uri ./repo.omni --json omnigraph run abort --uri ./repo.omni --json ``` +`query lint` and `query check` are the same command surface. In v1, repo-backed +lint uses local or `s3://` repo URIs; HTTP targets are only supported when you +also pass `--schema`. + ## Config `omnigraph.yaml` lets the CLI and server share named targets, defaults, and diff --git a/og-cheet-sheet.md b/og-cheet-sheet.md new file mode 100644 index 0000000..8ae6f5c --- /dev/null +++ b/og-cheet-sheet.md @@ -0,0 +1,36 @@ +# Omnigraph Cheat Sheet + +## Local Query Validation + +Use an explicit schema file: + +```bash +omnigraph query lint --query ./queries.gq --schema ./schema.pg --json +omnigraph query check --query ./queries.gq --schema ./schema.pg +``` + +Use a local or `s3://` repo target: + +```bash +omnigraph query lint --query ./queries.gq ./repo.omni --json +omnigraph query check --query ./queries.gq s3://bucket/repo +``` + +Use `omnigraph.yaml` target resolution: + +```bash +omnigraph query lint --query ./queries.gq --target local --config ./omnigraph.yaml +``` + +## What It Checks + +- parses every query in the file +- typechecks each query against the resolved schema +- warns on hardcoded mutation queries with no params +- warns when nullable node fields have no update-query coverage + +## Current Limits + +- repo-backed lint is local/S3-only in v1 +- HTTP targets need `--schema` +- exit code is nonzero only when there are strict validation errors