From 1224ea4c758870ffa3bed2ae592eb29c224f4b02 Mon Sep 17 00:00:00 2001 From: Stefan Date: Fri, 2 Oct 2026 09:05:00 +0200 Subject: [PATCH] fix: nullable keys after switch joins, no default-to-0 fix where errors skip, fallible outputs don't shadow, integer and aliased fields in missing cases - keys set by only some switch branches are nullable where branches join, in $nodes and in graph output - the (x ?? 0) fix is not offered in graph table cells and switch conditions, where an error means no match - a graph first-hit row whose output can fail no longer makes later rows unreachable - integer schema fields drop gaps without integers; min/max are read through nullable anyOf - columns over the same field path ((x), x['y']) share one dimension; derived columns overlapping another column are left out of gap checks --- core/engine/src/analysis/nullable.rs | 12 +- core/engine/src/analysis/table/cache.rs | 2 + core/engine/src/analysis/table/missing.rs | 105 +++++- core/engine/src/analysis/table/mod.rs | 1 + core/engine/src/analysis/table/verify.rs | 10 +- core/engine/src/policy/blocks/context.rs | 3 +- .../src/policy/blocks/decision_table.rs | 2 + core/engine/src/workspace/graph/analysis.rs | 299 ++++++++++++++++-- core/expression/src/intellisense/fallible.rs | 107 +++++++ core/expression/src/intellisense/mod.rs | 29 ++ .../src/intellisense/values/cell.rs | 11 + .../src/intellisense/values/value_set.rs | 20 ++ 12 files changed, 561 insertions(+), 40 deletions(-) create mode 100644 core/expression/src/intellisense/fallible.rs diff --git a/core/engine/src/analysis/nullable.rs b/core/engine/src/analysis/nullable.rs index 0fa9d2b0..b9168fc2 100644 --- a/core/engine/src/analysis/nullable.rs +++ b/core/engine/src/analysis/nullable.rs @@ -11,6 +11,12 @@ use crate::workspace::types::{Diagnostic, DiagnosticCode, Span}; pub(crate) struct NullableOperand; +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum OnError { + Raise, + Skip, +} + #[derive(Default)] struct Fallback { operands: Option<(Span, Span)>, @@ -44,8 +50,9 @@ impl NullableOperand { is: &mut IntelliSense, source: &str, unary: bool, + on_error: OnError, ) { - Self::default_operands(diagnostics, is, source, unary); + Self::default_operands(diagnostics, is, source, unary, on_error); Self::fallbacks(diagnostics, is, source, unary); } @@ -54,6 +61,7 @@ impl NullableOperand { is: &mut IntelliSense, source: &str, unary: bool, + on_error: OnError, ) { let requests: Vec<(usize, Span, String, bool)> = diagnostics .iter() @@ -88,7 +96,7 @@ impl NullableOperand { "/" | "%" => found.left, _ => false, }; - if !defaultable { + if !defaultable || on_error == OnError::Skip { continue; } let Some(operand) = AstOps::text(source, found.operand) else { diff --git a/core/engine/src/analysis/table/cache.rs b/core/engine/src/analysis/table/cache.rs index 141e0f61..b6451f4f 100644 --- a/core/engine/src/analysis/table/cache.rs +++ b/core/engine/src/analysis/table/cache.rs @@ -77,6 +77,7 @@ impl VerifyTable<'_> { col.unary.hash(&mut state); col.analyzable.hash(&mut state); col.dated.hash(&mut state); + col.integer.hash(&mut state); col.input.hash(&mut state); col.field.hash(&mut state); col.path.hash(&mut state); @@ -91,6 +92,7 @@ impl VerifyTable<'_> { col.label.hash(&mut state); col.values.hash(&mut state); } + self.fallible.hash(&mut state); self.rules.len().hash(&mut state); for rule in self.rules { let (low, high) = rule.iter().fold((0u64, 0u64), |(low, high), entry| { diff --git a/core/engine/src/analysis/table/missing.rs b/core/engine/src/analysis/table/missing.rs index 2c53e105..599091bb 100644 --- a/core/engine/src/analysis/table/missing.rs +++ b/core/engine/src/analysis/table/missing.rs @@ -1,7 +1,11 @@ use std::sync::Arc; +use std::rc::Rc; + use ahash::HashMap; use serde_json::{Map, Value}; +use zen_expression::intellisense::values::cell::FieldPath; +use zen_expression::intellisense::{IntelliSense, ReadDependency}; use super::cell::CellConstraint; use super::partition::Partition; @@ -19,19 +23,58 @@ struct Dimension { label: Arc, prefer: Option, dated: bool, + integer: bool, input: bool, } +struct Field { + path: Option>>, + reads: Vec>>, +} + +impl Field { + fn of(is: &mut IntelliSense, source: &str) -> Self { + match FieldPath::of(is, source) { + Some(path) => Self { + reads: vec![path.clone()], + path: Some(path), + }, + None => Self { + path: None, + reads: is + .reads(source) + .into_iter() + .filter_map(|read| match read { + ReadDependency::Direct { path, .. } => Some(path), + ReadDependency::Iteration { collection, .. } => Some(collection), + _ => None, + }) + .collect(), + }, + } + } + + fn overlaps(&self, other: &Field) -> bool { + self.reads.iter().any(|a| { + other + .reads + .iter() + .any(|b| a.iter().zip(b).all(|(x, y)| x == y)) + }) + } +} + impl VerifyTable<'_> { pub(super) fn missing( &self, + is: &mut IntelliSense, cells: &[Vec], satisfiable: &[bool], ) -> Option> { if self.rules.is_empty() { return Some(None); } - let dims = self.dimensions(cells); + let dims = self.dimensions(is, cells); if dims.is_empty() || dims.iter().any(|d| d.domain.is_empty()) { return Some(None); } @@ -61,7 +104,21 @@ impl VerifyTable<'_> { dims.iter().map(|d| d.domain.clone()).collect(), (0..cuts.len()).collect(), )?; - let remaining = gaps.out; + let remaining: Vec = gaps + .out + .into_iter() + .filter_map(|mut fragment| { + for (dim, set) in dims.iter().zip(fragment.iter_mut()) { + if dim.integer { + set.numbers = set.numbers.integral(); + } + } + fragment + .iter() + .all(|set| !set.is_empty()) + .then_some(fragment) + }) + .collect(); if remaining.is_empty() { return Some(None); } @@ -74,15 +131,32 @@ impl VerifyTable<'_> { Some(Some(Finding::MissingCases { cases, total })) } - fn dimensions(&self, cells: &[Vec]) -> Vec { - let mut dims: Vec<(Arc, Dimension)> = Vec::new(); + fn dimensions(&self, is: &mut IntelliSense, cells: &[Vec]) -> Vec { + let fields: Vec> = self + .inputs + .iter() + .map(|col| { + col.field + .as_ref() + .filter(|_| col.analyzable) + .map(|field| Field::of(is, field)) + }) + .collect(); + let mut dims: Vec<(Vec>, Dimension)> = Vec::new(); for (idx, col) in self.inputs.iter().enumerate() { - if !col.analyzable { - continue; - } - let Some(field) = col.field.clone() else { + let (Some(field), Some(source)) = (&fields[idx], &col.field) else { continue; }; + let key = match &field.path { + Some(path) => path.clone(), + None if fields.iter().enumerate().any(|(other, f)| { + other != idx && f.as_ref().is_some_and(|f| f.overlaps(field)) + }) => + { + continue + } + None => vec![Rc::from(source.as_ref())], + }; let Some(domain) = col .domain .clone() @@ -90,20 +164,29 @@ impl VerifyTable<'_> { else { continue; }; - match dims.iter_mut().find(|(key, _)| *key == field) { + match dims.iter_mut().find(|(existing, _)| *existing == key) { Some((_, dim)) => { dim.columns.push(idx); dim.domain = dim.domain.intersect(&domain); + dim.integer |= col.integer; + dim.path = dim.path.clone().or_else(|| col.path.clone()); } None => dims.push(( - field, + key, Dimension { columns: vec![idx], domain, - path: col.path.clone(), + path: col.path.clone().or_else(|| { + field + .path + .as_ref() + .filter(|path| path.iter().all(|segment| !segment.contains('.'))) + .map(|path| Arc::from(path.join("."))) + }), label: col.label.clone(), prefer: col.prefer, dated: col.dated, + integer: col.integer, input: col.input, }, )), diff --git a/core/engine/src/analysis/table/mod.rs b/core/engine/src/analysis/table/mod.rs index baadff25..071866a4 100644 --- a/core/engine/src/analysis/table/mod.rs +++ b/core/engine/src/analysis/table/mod.rs @@ -71,6 +71,7 @@ impl TableColumn { unary: field.is_some(), analyzable: field.is_some(), dated, + integer: false, input: true, field: field.map(|f| Arc::from(f.trim())), path: field.filter(|f| Self::is_plain_path(f)).cloned(), diff --git a/core/engine/src/analysis/table/verify.rs b/core/engine/src/analysis/table/verify.rs index 82899041..8fdc4e73 100644 --- a/core/engine/src/analysis/table/verify.rs +++ b/core/engine/src/analysis/table/verify.rs @@ -28,6 +28,7 @@ pub(crate) struct VerifyInput { pub(crate) unary: bool, pub(crate) analyzable: bool, pub(crate) dated: bool, + pub(crate) integer: bool, pub(crate) input: bool, pub(crate) field: Option>, pub(crate) path: Option>, @@ -49,6 +50,7 @@ pub(crate) struct VerifyTable<'a> { pub(crate) inputs: Vec, pub(crate) outputs: Vec, pub(crate) rules: &'a [HashMap, Arc>], + pub(crate) fallible: Vec, } #[derive(Debug, Clone, PartialEq)] @@ -159,7 +161,7 @@ impl VerifyTable<'_> { let mut seen: HashMap<(Vec, Vec), usize> = HashMap::default(); for row in 0..self.rules.len() { if let Some(index) = index.as_mut().filter(|_| row > 0) { - if satisfiable[row - 1] && !reported[row - 1] { + if satisfiable[row - 1] && !reported[row - 1] && !self.can_fail(row - 1) { index.insert(&cells[row - 1], row - 1); } } @@ -212,7 +214,7 @@ impl VerifyTable<'_> { } let mut gaps_incomplete = !coverage; if coverage { - match self.missing(&cells, &satisfiable) { + match self.missing(is, &cells, &satisfiable) { Some(Some(missing)) => findings.push(missing), Some(None) => {} None => gaps_incomplete = true, @@ -273,6 +275,10 @@ impl VerifyTable<'_> { findings } + fn can_fail(&self, row: usize) -> bool { + self.fallible.get(row).copied().unwrap_or(false) + } + fn contributes_collect(&self, row: usize) -> bool { self.outputs .iter() diff --git a/core/engine/src/policy/blocks/context.rs b/core/engine/src/policy/blocks/context.rs index 59e5675f..8cd75498 100644 --- a/core/engine/src/policy/blocks/context.rs +++ b/core/engine/src/policy/blocks/context.rs @@ -8,7 +8,7 @@ use zen_expression::{Isolate, IsolateError}; use super::property_read::ReadFlattener; use super::type_check::TypeCheck; -use crate::analysis::nullable::NullableOperand; +use crate::analysis::nullable::{NullableOperand, OnError}; use crate::analysis::table::{HitMode, VerifyInput, VerifyOutput, VerifyTable}; use crate::policy::ir::PropertyPath; use crate::policy::queries::dependency::{DataModelPaths, PathPrefix}; @@ -502,6 +502,7 @@ impl AnalysisContext { &mut self.intellisense.borrow_mut(), source, matches!(kind, ExpressionKind::Unary), + OnError::Raise, ); } } diff --git a/core/engine/src/policy/blocks/decision_table.rs b/core/engine/src/policy/blocks/decision_table.rs index ffd490af..88a48a25 100644 --- a/core/engine/src/policy/blocks/decision_table.rs +++ b/core/engine/src/policy/blocks/decision_table.rs @@ -477,6 +477,7 @@ impl DecisionTableIr { }) .collect(), rules: &self.rules, + fallible: Vec::new(), }; cx.defer_table_check(table); } @@ -715,6 +716,7 @@ impl DecisionTableIr { inputs: check.inputs.clone(), outputs: check.outputs.clone(), rules: &self.rules, + fallible: Vec::new(), }; table.diagnostics( is, diff --git a/core/engine/src/workspace/graph/analysis.rs b/core/engine/src/workspace/graph/analysis.rs index 5534d495..b0a826a9 100644 --- a/core/engine/src/workspace/graph/analysis.rs +++ b/core/engine/src/workspace/graph/analysis.rs @@ -13,7 +13,7 @@ use zen_types::decision::{ use zen_expression::intellisense::ArmTest; -use crate::analysis::nullable::NullableOperand; +use crate::analysis::nullable::{NullableOperand, OnError}; use crate::analysis::table::{ Bound, HitMode, Interval, NumberSet, PathConstraints, TableColumn, VerifyTable, }; @@ -118,6 +118,30 @@ struct GraphTopology { incoming: IncomingEdges, outgoing: Vec>, order: Option>, + may_skip: Vec, + guaranteed: Vec>, +} + +impl GraphTopology { + fn skips(&self, idx: usize) -> bool { + self.may_skip.get(idx).copied().unwrap_or(false) + } + + fn runs_with(&self, idx: usize, current: usize) -> bool { + !self.skips(idx) + || self + .guaranteed + .get(current) + .is_some_and(|g| g.contains(&idx)) + } + + fn certain_edge(&self, content: &GraphContent, current: usize, edge: usize) -> bool { + let incoming = &self.incoming[current]; + let (pred, handle) = &incoming[edge]; + incoming.len() == 1 + || (self.runs_with(*pred, current) + && !GraphAnalyzer::skippable(content, *pred, handle.as_deref())) + } } impl<'a> GraphAnalyzer<'a> { @@ -175,6 +199,7 @@ impl<'a> GraphAnalyzer<'a> { Self::merged_input(self.content, &topology, &nodes, idx); self.nodes_scope = Self::nodes_scope_of( self.content, + &topology, idx, &ancestor_set, &descendants[idx], @@ -289,14 +314,68 @@ impl<'a> GraphAnalyzer<'a> { )); } + let (may_skip, guaranteed) = match &order { + Some(order) => Self::execution(content, &incoming, order), + None => (Vec::new(), Vec::new()), + }; GraphTopology { node_index, incoming, outgoing, order, + may_skip, + guaranteed, } } + fn execution( + content: &GraphContent, + incoming: &IncomingEdges, + order: &[usize], + ) -> (Vec, Vec>) { + let mut may_skip = vec![false; incoming.len()]; + let mut guaranteed: Vec> = vec![HashSet::default(); incoming.len()]; + for &idx in order { + let edges = &incoming[idx]; + may_skip[idx] = !edges.is_empty() + && edges.iter().all(|(pred, handle)| { + may_skip[*pred] || Self::skippable(content, *pred, handle.as_deref()) + }); + let mut sets = edges.iter().map(|(pred, _)| &guaranteed[*pred]); + let mut runs: HashSet = match sets.next() { + Some(first) => sets.fold(first.clone(), |acc, set| { + acc.intersection(set).copied().collect() + }), + None => HashSet::default(), + }; + runs.insert(idx); + guaranteed[idx] = runs; + } + (may_skip, guaranteed) + } + + fn skippable(content: &GraphContent, source: usize, handle: Option<&str>) -> bool { + let DecisionNodeKind::SwitchNode { content: switch } = &content.nodes[source].kind else { + return false; + }; + let Some(handle) = handle else { + return true; + }; + let Some(position) = switch + .statements + .iter() + .position(|statement| statement.id.as_ref() == handle) + else { + return true; + }; + let always = switch.statements[position].condition.trim().is_empty() + && match switch.hit_policy { + SwitchStatementHitPolicy::Collect => true, + SwitchStatementHitPolicy::First => position == 0, + }; + !always + } + fn topological_order(incoming: &IncomingEdges, outgoing: &[Vec]) -> Option> { let mut indegree: Vec = incoming.iter().map(Vec::len).collect(); let mut queue: VecDeque = indegree @@ -334,6 +413,7 @@ impl<'a> GraphAnalyzer<'a> { fn nodes_scope_of( content: &GraphContent, + topology: &GraphTopology, current: usize, ancestor_set: &HashSet, descendant_set: &HashSet, @@ -350,7 +430,10 @@ impl<'a> GraphAnalyzer<'a> { } let resolved = if ancestor_set.contains(&idx) { match nodes.get(&node.id) { - Some(analysis) => analysis.output.shallow_clone(), + Some(analysis) if topology.runs_with(idx, current) => { + analysis.output.shallow_clone() + } + Some(analysis) => super::wrap_optional(analysis.output.shallow_clone()), None => VariableType::Any, } } else { @@ -375,7 +458,10 @@ impl<'a> GraphAnalyzer<'a> { let mut unchecked = false; let mut open = false; let mut merged: Option = None; - for (pred, handle) in &topology.incoming[idx] { + let mut certain: Vec = Vec::new(); + let mut every: Vec = Vec::new(); + let mut partial = false; + for (edge, (pred, handle)) in topology.incoming[idx].iter().enumerate() { let Some(analysis) = nodes.get(&content.nodes[*pred].id) else { continue; }; @@ -385,16 +471,75 @@ impl<'a> GraphAnalyzer<'a> { .as_ref() .and_then(|h| analysis.branch_outputs.get(h.as_ref())) .unwrap_or(&analysis.output); + match topology.certain_edge(content, idx, edge) { + true => certain.push(branch.shallow_clone()), + false => partial = true, + } + every.push(branch.shallow_clone()); merged = Some(match merged { None => branch.shallow_clone(), Some(acc) => acc.merge(branch), }); } - ( - merged.unwrap_or_else(VariableType::empty_object), - unchecked, - open, - ) + let merged = merged.unwrap_or_else(VariableType::empty_object); + let merged = match partial { + true => Self::present_in( + &merged, + &certain.iter().collect::>(), + Some(&every.iter().collect::>()), + ), + false => merged, + }; + (merged, unchecked, open) + } + + fn present_in( + merged: &VariableType, + certain: &[&VariableType], + every: Option<&[&VariableType]>, + ) -> VariableType { + let VariableType::Object(fields) = merged else { + return merged.shallow_clone(); + }; + let opaque = |t: &&VariableType| !matches!(t, VariableType::Object(_)); + if certain.iter().any(opaque) || every.is_some_and(|every| every.iter().any(opaque)) { + return merged.shallow_clone(); + } + let at = |types: &[&VariableType], key: &Rc| -> Vec { + types + .iter() + .filter_map(|t| match t { + VariableType::Object(object) => { + object.borrow().get(key).map(VariableType::shallow_clone) + } + _ => None, + }) + .collect() + }; + let result = VariableType::empty_object(); + if let VariableType::Object(out) = &result { + let mut out = out.borrow_mut(); + for (key, value) in fields.borrow().iter() { + let present = at(certain, key); + let everywhere = every + .map(|every| (at(every, key), every.len())) + .filter(|(values, len)| values.len() == *len) + .map(|(values, _)| values); + let value = match (present.is_empty(), &everywhere) { + (true, None) => super::wrap_optional(value.shallow_clone()), + _ => Self::present_in( + value, + &present.iter().collect::>(), + everywhere + .as_ref() + .map(|values| values.iter().collect::>()) + .as_deref(), + ), + }; + out.insert(key.clone(), value); + } + } + result } fn reachable_from_inputs( @@ -428,20 +573,32 @@ impl<'a> GraphAnalyzer<'a> { nodes: &HashMap, GraphNodeAnalysis>, ) -> VariableType { let reachable = Self::reachable_from_inputs(content, topology); - let mut terminals: Vec<&GraphNodeAnalysis> = content + let terminals: Vec<(usize, &GraphNodeAnalysis)> = content .nodes .iter() .enumerate() .filter(|(idx, _)| topology.outgoing.get(*idx).is_some_and(Vec::is_empty)) .filter(|(idx, _)| reachable.as_ref().is_none_or(|r| r[*idx])) - .filter_map(|(_, node)| nodes.get(&node.id)) + .filter_map(|(idx, node)| Some((idx, nodes.get(&node.id)?))) .collect(); - let Some(first) = terminals.pop() else { + let Some(((_, first), rest)) = terminals.split_first() else { return VariableType::empty_object(); }; - terminals - .into_iter() - .fold(first.output.shallow_clone(), |acc, t| acc.merge(&t.output)) + let merged = rest + .iter() + .fold(first.output.shallow_clone(), |acc, (_, t)| { + acc.merge(&t.output) + }); + let certain: Vec<&VariableType> = terminals + .iter() + .filter(|(idx, _)| !topology.skips(*idx)) + .map(|(_, t)| &t.output) + .collect(); + let every: Vec<&VariableType> = terminals.iter().map(|(_, t)| &t.output).collect(); + match rest.is_empty() || certain.len() == terminals.len() { + true => merged, + false => Self::present_in(&merged, &certain, Some(&every)), + } } fn graph_input_type(&self) -> VariableType { @@ -915,7 +1072,7 @@ impl<'a> GraphAnalyzer<'a> { continue; } let first = self.diagnostics.len(); - self.check_expression( + self.check_skipping( &node.id, Some(col.id.clone()), Some(target), @@ -926,7 +1083,7 @@ impl<'a> GraphAnalyzer<'a> { checked.insert(key, first..self.diagnostics.len()); } None => { - let resolved = self.check_expression( + let resolved = self.check_skipping( &node.id, Some(col.id.clone()), Some(target.clone()), @@ -962,7 +1119,7 @@ impl<'a> GraphAnalyzer<'a> { } } - self.verify_decision_table(node, content, &input_field_types); + self.verify_decision_table(node, content, &input_field_types, &base_scope); for col in content.inputs.iter() { let Some(field) = &col.field else { @@ -1050,7 +1207,7 @@ impl<'a> GraphAnalyzer<'a> { row: Self::row_key(rule, row_idx), col: col.id.clone(), }; - let resolved = self.check_expression( + let resolved = self.check_skipping( &node.id, Some(col.id.clone()), Some(target.clone()), @@ -1294,11 +1451,11 @@ impl<'a> GraphAnalyzer<'a> { out } - fn schema_number_range( + fn field_schema( &self, content: &DecisionTableContent, field: Option<&str>, - ) -> Option { + ) -> Option<&serde_json::Value> { if !self.preserved_input(content, field) { return None; } @@ -1312,8 +1469,40 @@ impl<'a> GraphAnalyzer<'a> { _ => None, })?; for segment in field.split('.') { - schema = schema.get("properties")?.get(segment)?; + schema = Self::non_null_schema(schema) + .get("properties")? + .get(segment)?; } + Some(Self::non_null_schema(schema)) + } + + fn non_null_schema(schema: &serde_json::Value) -> &serde_json::Value { + let branches = ["anyOf", "oneOf"] + .iter() + .find_map(|key| schema.get(*key).and_then(|v| v.as_array())); + let Some(branches) = branches else { + return schema; + }; + let mut present = branches + .iter() + .filter(|branch| branch.get("type").and_then(|t| t.as_str()) != Some("null")); + match (present.next(), present.next()) { + (Some(branch), None) => branch, + _ => schema, + } + } + + fn schema_integer(schema: &serde_json::Value) -> bool { + match schema.get("type") { + Some(serde_json::Value::String(kind)) => kind == "integer", + Some(serde_json::Value::Array(kinds)) => { + kinds.iter().any(|k| k == "integer") && !kinds.iter().any(|k| k == "number") + } + _ => false, + } + } + + fn schema_number_range(schema: &serde_json::Value) -> Option { let bound = |key: &str| { schema .get(key) @@ -1341,7 +1530,24 @@ impl<'a> GraphAnalyzer<'a> { node: &DecisionNode, content: &DecisionTableContent, input_field_types: &HashMap, VariableType>, + scope: &VariableType, ) { + let intellisense = self.db.graph_intellisense(); + let fallible = match content.hit_policy { + DecisionTableHitPolicy::First => content + .rules + .iter() + .map(|rule| { + content.outputs.iter().any(|col| { + !col.write_path().0.is_empty() + && rule.get(&col.id).is_some_and(|cell| { + !cell.is_empty() && intellisense.borrow_mut().can_fail(cell, scope) + }) + }) + }) + .collect(), + DecisionTableHitPolicy::Collect => Vec::new(), + }; let table = VerifyTable { mode: match content.hit_policy { DecisionTableHitPolicy::First => HitMode::RowFirst, @@ -1358,7 +1564,9 @@ impl<'a> GraphAnalyzer<'a> { input_field_types.get(&col.id), ); input.input = self.preserved_input(content, col.field.as_deref()); - let input = match self.schema_number_range(content, col.field.as_deref()) { + let schema = self.field_schema(content, col.field.as_deref()); + input.integer = schema.is_some_and(Self::schema_integer); + let input = match schema.and_then(Self::schema_number_range) { Some(range) => TableColumn::narrow_numbers(input, range), None => input, }; @@ -1389,8 +1597,8 @@ impl<'a> GraphAnalyzer<'a> { }) .collect(), rules: &content.rules, + fallible, }; - let intellisense = self.db.graph_intellisense(); let diagnostics = table.diagnostics( &mut intellisense.borrow_mut(), |row| Self::row_key(&content.rules[row], row), @@ -1710,7 +1918,7 @@ impl<'a> GraphAnalyzer<'a> { let test = if statement.condition.is_empty() { ArmTest::Default } else { - let resolved = self.check_expression( + let resolved = self.check_skipping( &node.id, Some(statement.id.clone()), None, @@ -2072,6 +2280,48 @@ impl<'a> GraphAnalyzer<'a> { source: &Arc, kind: ExpressionKind, scope: &VariableType, + ) -> VariableType { + self.check( + node_id, + expression_id, + target, + source, + kind, + scope, + OnError::Raise, + ) + } + + fn check_skipping( + &mut self, + node_id: &Arc, + expression_id: Option>, + target: Option, + source: &Arc, + kind: ExpressionKind, + scope: &VariableType, + ) -> VariableType { + self.check( + node_id, + expression_id, + target, + source, + kind, + scope, + OnError::Skip, + ) + } + + #[allow(clippy::too_many_arguments)] + fn check( + &mut self, + node_id: &Arc, + expression_id: Option>, + target: Option, + source: &Arc, + kind: ExpressionKind, + scope: &VariableType, + on_error: OnError, ) -> VariableType { let intellisense = self.db.graph_intellisense(); let analysis = @@ -2101,6 +2351,7 @@ impl<'a> GraphAnalyzer<'a> { &mut intellisense.borrow_mut(), source, matches!(kind, ExpressionKind::Unary), + on_error, ); if self.validate { self.validate_read_paths(node_id, &expression_id, &target, &analysis.reads, scope); diff --git a/core/expression/src/intellisense/fallible.rs b/core/expression/src/intellisense/fallible.rs new file mode 100644 index 00000000..344016e0 --- /dev/null +++ b/core/expression/src/intellisense/fallible.rs @@ -0,0 +1,107 @@ +use crate::intellisense::type_provider::TypesProvider; +use crate::lexer::{ArithmeticOperator, ComparisonOperator, LogicalOperator, Operator}; +use crate::parser::Node; +use crate::variable::VariableType; + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Scalar { + Number, + String, + Bool, +} + +pub(crate) struct Fallible<'t> { + types: &'t TypesProvider, +} + +impl<'t> Fallible<'t> { + pub(crate) fn new(types: &'t TypesProvider) -> Self { + Self { types } + } + + pub(crate) fn safe(&self, node: &Node) -> bool { + match node { + Node::Null + | Node::Bool(_) + | Node::Number(_) + | Node::String(_) + | Node::Identifier(_) + | Node::Root => true, + Node::Parenthesized(inner) => self.safe(inner), + Node::Member { node, property } => { + matches!(property, Node::String(_) | Node::Number(_)) && self.safe(node) + } + Node::Array(items) => items.iter().all(|item| self.safe(item)), + Node::Object(entries) => entries + .iter() + .all(|(key, value)| matches!(key, Node::String(_)) && self.safe(value)), + Node::TemplateString(parts) => parts + .iter() + .all(|part| self.safe(part) && self.scalar(part).is_some()), + Node::Conditional { + condition, + on_true, + on_false, + } => self.typed(condition, Scalar::Bool) && self.safe(on_true) && self.safe(on_false), + Node::Unary { node, operator } => match operator { + Operator::Logical(LogicalOperator::Not) => self.typed(node, Scalar::Bool), + Operator::Arithmetic(ArithmeticOperator::Subtract | ArithmeticOperator::Add) => { + self.typed(node, Scalar::Number) + } + _ => false, + }, + Node::Binary { + left, + operator, + right, + } => match operator { + Operator::Logical(LogicalOperator::NullishCoalescing) + | Operator::Comparison(ComparisonOperator::Equal | ComparisonOperator::NotEqual) => { + self.safe(left) && self.safe(right) + } + Operator::Logical(LogicalOperator::And | LogicalOperator::Or) => { + self.typed(left, Scalar::Bool) && self.typed(right, Scalar::Bool) + } + Operator::Arithmetic( + ArithmeticOperator::Subtract | ArithmeticOperator::Multiply, + ) + | Operator::Comparison( + ComparisonOperator::LessThan + | ComparisonOperator::LessThanOrEqual + | ComparisonOperator::GreaterThan + | ComparisonOperator::GreaterThanOrEqual, + ) => self.typed(left, Scalar::Number) && self.typed(right, Scalar::Number), + Operator::Arithmetic(ArithmeticOperator::Divide | ArithmeticOperator::Modulus) => { + self.typed(left, Scalar::Number) + && matches!(right, Node::Number(divisor) if !divisor.is_zero()) + } + Operator::Arithmetic(ArithmeticOperator::Add) => { + self.safe(left) + && self.safe(right) + && matches!( + (self.scalar(left), self.scalar(right)), + (Some(Scalar::Number), Some(Scalar::Number)) + | (Some(Scalar::String), Some(Scalar::String)) + ) + } + _ => false, + }, + _ => false, + } + } + + fn typed(&self, node: &Node, scalar: Scalar) -> bool { + self.safe(node) && self.scalar(node) == Some(scalar) + } + + fn scalar(&self, node: &Node) -> Option { + match &self.types.get_type(node)?.kind { + VariableType::Number => Some(Scalar::Number), + VariableType::String | VariableType::Const(_) | VariableType::Enum(..) => { + Some(Scalar::String) + } + VariableType::Bool => Some(Scalar::Bool), + _ => None, + } + } +} diff --git a/core/expression/src/intellisense/mod.rs b/core/expression/src/intellisense/mod.rs index 3576f768..2dbbf8ad 100644 --- a/core/expression/src/intellisense/mod.rs +++ b/core/expression/src/intellisense/mod.rs @@ -5,6 +5,7 @@ use crate::intellisense::diagnostic::{ collect_parser_diagnostics, collect_type_diagnostics, compiler_error_to_diagnostic, lexer_error_to_diagnostic, Diagnostic, }; +use crate::intellisense::fallible::Fallible; use crate::intellisense::inspection::{Hover, HoverWord, InspectionResult}; use crate::intellisense::scope::IntelliSenseScope; use crate::intellisense::type_provider::TypesProvider; @@ -24,6 +25,7 @@ pub mod dependency; pub mod diagnostic; mod discriminant; mod entity_flow; +mod fallible; mod inspection; pub(crate) mod scope; pub(crate) mod type_provider; @@ -234,6 +236,33 @@ impl IntelliSense { } } + pub fn can_fail(&mut self, source: &str, data: &VariableType) -> bool { + self.arena.reset(); + let arena = &self.arena; + let Ok(tokens) = self.lexer.tokenize(arena, source) else { + return true; + }; + let Ok(parser) = Parser::try_new(&tokens, arena) else { + return true; + }; + let parser_result = parser.standard().parse(); + let ast = parser_result.root; + if !parser_result.is_complete || ast.has_error() { + return true; + } + let types = TypesProvider::generate( + ast, + IntelliSenseScope { + pointer_data: data.shallow_clone(), + root_data: data.shallow_clone(), + current_data: data.shallow_clone(), + ..Default::default() + }, + self.strict, + ); + !Fallible::new(&types).safe(ast) + } + pub fn with_ast( &mut self, source: &str, diff --git a/core/expression/src/intellisense/values/cell.rs b/core/expression/src/intellisense/values/cell.rs index ef30a1df..ec6d06c8 100644 --- a/core/expression/src/intellisense/values/cell.rs +++ b/core/expression/src/intellisense/values/cell.rs @@ -90,6 +90,17 @@ impl CellConstraint { } } +pub struct FieldPath; + +impl FieldPath { + pub fn of(is: &mut IntelliSense, source: &str) -> Option>> { + is.with_ast(source.trim(), false, |node, _| { + Truth::path(node).map(|path| path.into_iter().map(Rc::from).collect()) + }) + .flatten() + } +} + pub struct Condition; impl Condition { diff --git a/core/expression/src/intellisense/values/value_set.rs b/core/expression/src/intellisense/values/value_set.rs index f4977e2c..4154c94f 100644 --- a/core/expression/src/intellisense/values/value_set.rs +++ b/core/expression/src/intellisense/values/value_set.rs @@ -78,6 +78,15 @@ impl Interval { } } + fn has_integer(&self) -> bool { + let first = match self.lo { + Bound::Unbounded => return !self.is_empty(), + Bound::Inclusive(l) => Some(l.ceil()), + Bound::Exclusive(l) => l.floor().checked_add(Decimal::ONE), + }; + first.is_some_and(|x| self.contains(x)) + } + fn overlaps(&self, other: &Interval) -> bool { let lo = if self.lo.lo_key() >= other.lo.lo_key() { self.lo @@ -182,6 +191,17 @@ impl NumberSet { self.intervals.iter().any(|i| i.contains(x)) } + pub fn integral(&self) -> Self { + Self { + intervals: self + .intervals + .iter() + .filter(|i| i.has_integer()) + .copied() + .collect(), + } + } + fn normalize(&mut self) { self.intervals.retain(|i| !i.is_empty()); self.intervals.sort_by_key(|i| i.lo.lo_key());