diff --git a/changelog.d/1861.breaking.md b/changelog.d/1861.breaking.md new file mode 100644 index 0000000000..ef60c3f176 --- /dev/null +++ b/changelog.d/1861.breaking.md @@ -0,0 +1,17 @@ +Several stdlib functions now declare element-kind constraints on array parameters, enabling the compiler to detect element-type mismatches at compile time and automatically infer call-site infallibility. + +**Before:** passing a string-literal array required `!` because the compiler assumed it could fail: +``` +join!(["sources", "transforms", "sinks"], separator: ", ") +``` + +**After:** the compiler proves the elements are strings, so `!` is unnecessary (and now an error): +``` +join(["sources", "transforms", "sinks"], separator: ", ") +``` + +Passing the wrong element type (e.g. `join([1, 2, 3])`) is now a hard compile error instead of a runtime failure. + +Affected functions: `join`, `contains_all`, `tally`, `encode_key_value`, `encode_logfmt`, `ip_cidr_contains`, `parse_groks`. + +authors: pront diff --git a/docs/generated/encode_key_value.json b/docs/generated/encode_key_value.json index 7dc9f8d9a3..1f238f5e6a 100644 --- a/docs/generated/encode_key_value.json +++ b/docs/generated/encode_key_value.json @@ -65,7 +65,7 @@ }, { "title": "Encode with default delimiters (fields ordering)", - "source": "encode_key_value!(\n {\n \"ts\": \"2021-06-05T17:20:00Z\",\n \"msg\": \"This is a message\",\n \"lvl\": \"info\",\n \"log_id\": 12345\n },\n [\"ts\", \"lvl\", \"msg\"]\n)\n", + "source": "encode_key_value(\n {\n \"ts\": \"2021-06-05T17:20:00Z\",\n \"msg\": \"This is a message\",\n \"lvl\": \"info\",\n \"log_id\": 12345\n },\n [\"ts\", \"lvl\", \"msg\"]\n)\n", "return": "ts=2021-06-05T17:20:00Z lvl=info msg=\"This is a message\" log_id=12345" }, { @@ -75,7 +75,7 @@ }, { "title": "Encode with default delimiters (nested fields ordering)", - "source": "encode_key_value!(\n {\n \"agent\": {\"name\": \"foo\"},\n \"log\": {\"file\": {\"path\": \"my.log\"}},\n \"event\": \"log\"\n },\n [\"event\", \"log.file.path\", \"agent.name\"])\n", + "source": "encode_key_value(\n {\n \"agent\": {\"name\": \"foo\"},\n \"log\": {\"file\": {\"path\": \"my.log\"}},\n \"event\": \"log\"\n },\n [\"event\", \"log.file.path\", \"agent.name\"])\n", "return": "event=log log.file.path=my.log agent.name=foo" }, { @@ -90,7 +90,7 @@ } ], "notices": [ - "If `fields_ordering` is specified then the function is fallible else it is infallible." + "This function is fallible if `fields_ordering` contains non-string elements." ], "pure": true } diff --git a/docs/generated/encode_logfmt.json b/docs/generated/encode_logfmt.json index e9d642881b..2986d6f520 100644 --- a/docs/generated/encode_logfmt.json +++ b/docs/generated/encode_logfmt.json @@ -38,7 +38,7 @@ }, { "title": "Encode to logfmt (fields ordering)", - "source": "encode_logfmt!({\"ts\": \"2021-06-05T17:20:00Z\", \"msg\": \"This is a message\", \"lvl\": \"info\", \"log_id\": 12345}, [\"ts\", \"lvl\", \"msg\"])", + "source": "encode_logfmt({\"ts\": \"2021-06-05T17:20:00Z\", \"msg\": \"This is a message\", \"lvl\": \"info\", \"log_id\": 12345}, [\"ts\", \"lvl\", \"msg\"])", "return": "ts=2021-06-05T17:20:00Z lvl=info msg=\"This is a message\" log_id=12345" }, { @@ -48,7 +48,7 @@ }, { "title": "Encode to logfmt (nested fields ordering)", - "source": "encode_logfmt!({\"agent\": {\"name\": \"foo\"}, \"log\": {\"file\": {\"path\": \"my.log\"}}, \"event\": \"log\"}, [\"event\", \"log.file.path\", \"agent.name\"])", + "source": "encode_logfmt({\"agent\": {\"name\": \"foo\"}, \"log\": {\"file\": {\"path\": \"my.log\"}}, \"event\": \"log\"}, [\"event\", \"log.file.path\", \"agent.name\"])", "return": "event=log log.file.path=my.log agent.name=foo" } ], diff --git a/docs/generated/join.json b/docs/generated/join.json index 511475caa1..c821213b1d 100644 --- a/docs/generated/join.json +++ b/docs/generated/join.json @@ -29,12 +29,12 @@ "examples": [ { "title": "Join array (no separator)", - "source": "join!([\"bring\", \"us\", \"together\"])", + "source": "join([\"bring\", \"us\", \"together\"])", "return": "bringustogether" }, { "title": "Join array (comma separator)", - "source": "join!([\"sources\", \"transforms\", \"sinks\"], separator: \", \")", + "source": "join([\"sources\", \"transforms\", \"sinks\"], separator: \", \")", "return": "sources, transforms, sinks" } ], diff --git a/docs/generated/tally.json b/docs/generated/tally.json index 65ba0d469d..5d8c51151b 100644 --- a/docs/generated/tally.json +++ b/docs/generated/tally.json @@ -21,7 +21,7 @@ "examples": [ { "title": "tally", - "source": "tally!([\"foo\", \"bar\", \"foo\", \"baz\"])", + "source": "tally([\"foo\", \"bar\", \"foo\", \"baz\"])", "return": { "foo": 2, "bar": 1, diff --git a/lib/tests/tests/rfcs/8381/convert_object_to_specific_string_format.vrl b/lib/tests/tests/rfcs/8381/convert_object_to_specific_string_format.vrl index df5d709cd0..91f1de3c05 100644 --- a/lib/tests/tests/rfcs/8381/convert_object_to_specific_string_format.vrl +++ b/lib/tests/tests/rfcs/8381/convert_object_to_specific_string_format.vrl @@ -4,4 +4,4 @@ strings = [] for_each(.) -> |key, value| { strings = push(strings, key + "=" + encode_json(value)) } -"{" + join!(strings, ",") + "}" +"{" + join(strings, ",") + "}" diff --git a/src/compiler/function.rs b/src/compiler/function.rs index 3a90ad3a35..b729ffec15 100644 --- a/src/compiler/function.rs +++ b/src/compiler/function.rs @@ -262,6 +262,18 @@ pub struct Parameter { /// error. pub kind: u16, + /// For array parameters, the type kind(s) allowed for the array's elements. + /// + /// When [`kind`] includes [`kind::ARRAY`] and this is not [`kind::ANY`], the + /// compiler restricts the array's element kind accordingly: + /// - If the argument's element kind is a subset, the call is infallible. + /// - If the argument's element kind is unknown or a superset, the call is + /// automatically marked fallible. + /// + /// Defaults to [`kind::ANY`] (no element-type constraint). Ignored when + /// [`kind`] does not include [`kind::ARRAY`]. + pub element_kind: u16, + /// Whether or not this is a required parameter. /// /// If it isn't, the function can be called without errors, even if the @@ -292,6 +304,7 @@ impl Parameter { Self { keyword, kind, + element_kind: kind::ANY, required: true, description, default: None, @@ -305,6 +318,7 @@ impl Parameter { Self { keyword, kind, + element_kind: kind::ANY, required: false, description, default: None, @@ -326,55 +340,67 @@ impl Parameter { self } + /// For an array parameter, restrict the kind of its elements. + /// + /// The compiler uses this to infer call-site fallibility: a subset match is + /// infallible; a superset or unknown match is fallible; a disjoint match is + /// a compile error. Ignored when [`kind`] does not include [`kind::ARRAY`]. + #[must_use] + pub const fn with_element_kind(mut self, element_kind: u16) -> Self { + self.element_kind = element_kind; + self + } + #[allow(arithmetic_overflow)] #[must_use] pub fn kind(&self) -> Kind { - let mut kind = Kind::never(); - - let n = self.kind; - - if (n & kind::BYTES) == kind::BYTES { - kind.add_bytes(); - } - - if (n & kind::INTEGER) == kind::INTEGER { - kind.add_integer(); - } - - if (n & kind::FLOAT) == kind::FLOAT { - kind.add_float(); - } + let mut kind = kind_from_bits(self.kind); - if (n & kind::BOOLEAN) == kind::BOOLEAN { - kind.add_boolean(); + if (self.kind & kind::ARRAY) == kind::ARRAY && self.element_kind != kind::ANY { + let element = kind_from_bits(self.element_kind); + kind.add_array(Collection::from_unknown(element)); } - if (n & kind::OBJECT) == kind::OBJECT { - kind.add_object(Collection::any()); - } - - if (n & kind::ARRAY) == kind::ARRAY { - kind.add_array(Collection::any()); - } - - if (n & kind::TIMESTAMP) == kind::TIMESTAMP { - kind.add_timestamp(); - } - - if (n & kind::REGEX) == kind::REGEX { - kind.add_regex(); - } - - if (n & kind::NULL) == kind::NULL { - kind.add_null(); - } + kind + } +} - if (n & kind::UNDEFINED) == kind::UNDEFINED { - kind.add_undefined(); - } +#[allow(arithmetic_overflow)] +fn kind_from_bits(n: u16) -> Kind { + let mut kind = Kind::never(); - kind + if (n & kind::BYTES) == kind::BYTES { + kind.add_bytes(); + } + if (n & kind::INTEGER) == kind::INTEGER { + kind.add_integer(); } + if (n & kind::FLOAT) == kind::FLOAT { + kind.add_float(); + } + if (n & kind::BOOLEAN) == kind::BOOLEAN { + kind.add_boolean(); + } + if (n & kind::OBJECT) == kind::OBJECT { + kind.add_object(Collection::any()); + } + if (n & kind::ARRAY) == kind::ARRAY { + kind.add_array(Collection::any()); + } + if (n & kind::TIMESTAMP) == kind::TIMESTAMP { + kind.add_timestamp(); + } + if (n & kind::REGEX) == kind::REGEX { + kind.add_regex(); + } + if (n & kind::NULL) == kind::NULL { + kind.add_null(); + } + if (n & kind::UNDEFINED) == kind::UNDEFINED { + kind.add_undefined(); + } + + kind } // ----------------------------------------------------------------------------- diff --git a/src/docs/mod.rs b/src/docs/mod.rs index cc7bc4d02f..dcac21639e 100644 --- a/src/docs/mod.rs +++ b/src/docs/mod.rs @@ -130,6 +130,7 @@ pub fn build_function_doc(func: &dyn Function) -> FunctionDoc { description, default, enum_variants, + element_kind: _, } = param; let name = keyword.trim().to_string(); diff --git a/src/stdlib/contains_all.rs b/src/stdlib/contains_all.rs index f655aebcb8..aff77173b7 100644 --- a/src/stdlib/contains_all.rs +++ b/src/stdlib/contains_all.rs @@ -46,7 +46,8 @@ impl Function for ContainsAll { "substrings", kind::ARRAY, "An array of substrings to search for in `value`.", - ), + ) + .with_element_kind(kind::BYTES), Parameter::optional( "case_sensitive", kind::BOOLEAN, @@ -114,11 +115,11 @@ impl FunctionExpression for ContainsAllFn { contains_all(&value, substrings, case_sensitive) } - fn type_def(&self, state: &TypeState) -> TypeDef { - let substring_type_def = self.substrings.type_def(state); - let collection = substring_type_def.as_array().expect("must be an array"); - let bytes_collection = Collection::from_unknown(Kind::bytes()); - TypeDef::boolean().maybe_fallible(bytes_collection.is_superset(collection).is_err()) + fn type_def(&self, _state: &TypeState) -> TypeDef { + // Element-kind constraint on `substrings` is declared via + // `Parameter::with_element_kind`, which drives compiler-level + // fallibility inference at the call site. + TypeDef::boolean() } } @@ -138,11 +139,16 @@ mod tests { tdef: TypeDef::boolean().infallible(), } + // The function body's `type_def` is now unconditionally infallible; + // element-kind fallibility for `substrings` is inferred by the compiler + // at the call site via `Parameter::with_element_kind`. See the + // `compiler_flags_non_bytes_element_as_fallible` test for end-to-end + // verification. substring_type { args: func_args![value: value!("The Needle In The Haystack"), substrings: value!([1, 2])], want: Err("expected string, got integer"), - tdef: TypeDef::boolean().fallible(), + tdef: TypeDef::boolean(), } yes { @@ -182,4 +188,36 @@ mod tests { tdef: TypeDef::boolean().infallible(), } ]; + + /// End-to-end check: `Parameter::with_element_kind(kind::BYTES)` should drive + /// the compiler to mark a `contains_all` call fallible when it cannot prove + /// the array elements are strings. + #[test] + fn compiler_flags_non_bytes_element_as_fallible() { + let fns = vec![Box::new(ContainsAll) as Box]; + + // All string literals: element kind is a subset of `bytes` -> infallible. + let src = r#"contains_all("The Needle", ["Needle", "Hay"])"#; + let res = crate::compiler::compile(src, &fns).expect("compiles"); + assert!( + !res.program.info().fallible, + "call with string literals should be infallible" + ); + + // Element kind cannot be proven a subset of `bytes` -> fallible. + // [1, 2] has element kind integer, which is provably disjoint from + // bytes. The compiler now rejects this outright — even `??` cannot + // silence a provably-impossible argument. + let src = r#"contains_all("The Needle", [1, 2]) ?? false"#; + assert!( + crate::compiler::compile(src, &fns).is_err(), + "[1, 2] is provably not array; must be a compile error even with `??`" + ); + + let src = r#"contains_all("The Needle", [1, 2])"#; + assert!( + crate::compiler::compile(src, &fns).is_err(), + "unhandled provably-wrong call should fail to compile" + ); + } } diff --git a/src/stdlib/encode_key_value.rs b/src/stdlib/encode_key_value.rs index 6c40df4135..9ac0251608 100644 --- a/src/stdlib/encode_key_value.rs +++ b/src/stdlib/encode_key_value.rs @@ -37,7 +37,8 @@ static DEFAULT_FLATTEN_BOOLEAN: Value = Value::Boolean(false); const PARAMETERS: &[Parameter] = &[ Parameter::required("value", kind::OBJECT, "The value to convert to a string."), Parameter::optional("fields_ordering", kind::ARRAY, "The ordering of fields to preserve. Any fields not in this list are listed unordered, after all ordered fields.") - .default(&DEFAULT_FIELDS_ORDERING), + .default(&DEFAULT_FIELDS_ORDERING) + .with_element_kind(kind::BYTES), Parameter::optional("key_value_delimiter", kind::BYTES, "The string that separates the key from the value.") .default(&DEFAULT_KEY_VALUE_DELIMITER), Parameter::optional("field_delimiter", kind::BYTES, "The string that separates each key-value pair.") @@ -71,7 +72,7 @@ impl Function for EncodeKeyValue { } fn notices(&self) -> &'static [&'static str] { - &["If `fields_ordering` is specified then the function is fallible else it is infallible."] + &["This function is fallible if `fields_ordering` contains non-string elements."] } fn parameters(&self) -> &'static [Parameter] { @@ -119,7 +120,7 @@ impl Function for EncodeKeyValue { example! { title: "Encode with default delimiters (fields ordering)", source: indoc! {r#" - encode_key_value!( + encode_key_value( { "ts": "2021-06-05T17:20:00Z", "msg": "This is a message", @@ -147,7 +148,7 @@ impl Function for EncodeKeyValue { example! { title: "Encode with default delimiters (nested fields ordering)", source: indoc! {r#" - encode_key_value!( + encode_key_value( { "agent": {"name": "foo"}, "log": {"file": {"path": "my.log"}}, @@ -231,7 +232,7 @@ impl FunctionExpression for EncodeKeyValueFn { } fn type_def(&self, _: &state::TypeState) -> TypeDef { - TypeDef::bytes().maybe_fallible(self.fields.is_some()) + TypeDef::bytes() } } @@ -427,7 +428,7 @@ mod tests { fields_ordering: value!(["lvl", "msg"]) ], want: Ok(r#"lvl=info msg="This is a log message" log_id=12345"#), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } nested_fields_ordering { @@ -446,7 +447,7 @@ mod tests { fields_ordering: value!(["event", "log.file.path", "agent.name"]) ], want: Ok("event=log log.file.path=encode_key_value.rs agent.name=vector"), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } fields_ordering_invalid_field_type { @@ -463,7 +464,7 @@ mod tests { got: Kind::integer(), expected: Kind::bytes() })), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } ]; } diff --git a/src/stdlib/encode_logfmt.rs b/src/stdlib/encode_logfmt.rs index 9f93b7afbc..ac0648fefb 100644 --- a/src/stdlib/encode_logfmt.rs +++ b/src/stdlib/encode_logfmt.rs @@ -78,7 +78,7 @@ impl Function for EncodeLogfmt { }, example! { title: "Encode to logfmt (fields ordering)", - source: r#"encode_logfmt!({"ts": "2021-06-05T17:20:00Z", "msg": "This is a message", "lvl": "info", "log_id": 12345}, ["ts", "lvl", "msg"])"#, + source: r#"encode_logfmt({"ts": "2021-06-05T17:20:00Z", "msg": "This is a message", "lvl": "info", "log_id": 12345}, ["ts", "lvl", "msg"])"#, result: Ok(r#"ts=2021-06-05T17:20:00Z lvl=info msg="This is a message" log_id=12345"#), }, example! { @@ -88,7 +88,7 @@ impl Function for EncodeLogfmt { }, example! { title: "Encode to logfmt (nested fields ordering)", - source: r#"encode_logfmt!({"agent": {"name": "foo"}, "log": {"file": {"path": "my.log"}}, "event": "log"}, ["event", "log.file.path", "agent.name"])"#, + source: r#"encode_logfmt({"agent": {"name": "foo"}, "log": {"file": {"path": "my.log"}}, "event": "log"}, ["event", "log.file.path", "agent.name"])"#, result: Ok(r"event=log log.file.path=my.log agent.name=foo"), }, ] diff --git a/src/stdlib/from_entries.rs b/src/stdlib/from_entries.rs index 2095a42dfe..c6efc42eda 100644 --- a/src/stdlib/from_entries.rs +++ b/src/stdlib/from_entries.rs @@ -65,6 +65,7 @@ impl Function for FromEntries { &[Parameter { keyword: "value", kind: kind::ARRAY, + element_kind: kind::ANY, required: true, description: "The array of key/value objects to convert.", default: None, diff --git a/src/stdlib/ip_cidr_contains.rs b/src/stdlib/ip_cidr_contains.rs index 3c8cbbdd34..b52d7dfb1b 100644 --- a/src/stdlib/ip_cidr_contains.rs +++ b/src/stdlib/ip_cidr_contains.rs @@ -90,7 +90,8 @@ impl Function for IpCidrContains { "cidr", kind::BYTES | kind::ARRAY, "The CIDR mask (v4 or v6).", - ), + ) + .with_element_kind(kind::BYTES), Parameter::required("value", kind::BYTES, "The IP address (v4 or v6)."), ]; PARAMETERS diff --git a/src/stdlib/join.rs b/src/stdlib/join.rs index bbec0ef2ae..a9c0d9119f 100644 --- a/src/stdlib/join.rs +++ b/src/stdlib/join.rs @@ -42,7 +42,8 @@ impl Function for Join { "value", kind::ARRAY, "The array of strings to join together.", - ), + ) + .with_element_kind(kind::BYTES), Parameter::optional( "separator", kind::BYTES, @@ -68,12 +69,12 @@ impl Function for Join { &[ example! { title: "Join array (no separator)", - source: r#"join!(["bring", "us", "together"])"#, + source: r#"join(["bring", "us", "together"])"#, result: Ok("bringustogether"), }, example! { title: "Join array (comma separator)", - source: r#"join!(["sources", "transforms", "sinks"], separator: ", ")"#, + source: r#"join(["sources", "transforms", "sinks"], separator: ", ")"#, result: Ok("sources, transforms, sinks"), }, ] @@ -99,7 +100,7 @@ impl FunctionExpression for JoinFn { } fn type_def(&self, _: &state::TypeState) -> TypeDef { - TypeDef::bytes().fallible() + TypeDef::bytes() } } @@ -113,25 +114,25 @@ mod test { with_comma_separator { args: func_args![value: value!(["one", "two", "three"]), separator: ", "], want: Ok(value!("one, two, three")), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } with_space_separator { args: func_args![value: value!(["one", "two", "three"]), separator: " "], want: Ok(value!("one two three")), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } without_separator { args: func_args![value: value!(["one", "two", "three"])], want: Ok(value!("onetwothree")), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } non_string_array_item_throws_error { args: func_args![value: value!(["one", "two", 3])], want: Err("all array items must be strings"), - tdef: TypeDef::bytes().fallible(), + tdef: TypeDef::bytes(), } ]; } diff --git a/src/stdlib/parse_groks.rs b/src/stdlib/parse_groks.rs index d2ac7fa9bd..95927f1a2b 100644 --- a/src/stdlib/parse_groks.rs +++ b/src/stdlib/parse_groks.rs @@ -78,7 +78,8 @@ const PARAMETERS: &[Parameter] = &[ "patterns", kind::ARRAY, "The [Grok patterns](https://github.com/daschl/grok/tree/master/patterns), which are tried in order until the first match.", - ), + ) + .with_element_kind(kind::BYTES), Parameter::optional( "aliases", kind::OBJECT, @@ -90,6 +91,7 @@ const PARAMETERS: &[Parameter] = &[ kind::ARRAY, "Path to the file containing aliases in a JSON format.", ) + .with_element_kind(kind::BYTES) .default(&DEFAULT_ALIAS_SOURCES), ]; diff --git a/src/stdlib/tally.rs b/src/stdlib/tally.rs index cd03cbd08c..2e79d03dd9 100644 --- a/src/stdlib/tally.rs +++ b/src/stdlib/tally.rs @@ -47,7 +47,7 @@ impl Function for Tally { fn examples(&self) -> &'static [Example] { &[example! { title: "tally", - source: r#"tally!(["foo", "bar", "foo", "baz"])"#, + source: r#"tally(["foo", "bar", "foo", "baz"])"#, result: Ok(r#"{"foo": 2, "bar": 1, "baz": 1}"#), }] } @@ -68,7 +68,8 @@ impl Function for Tally { "value", kind::ARRAY, "The array of strings to count occurrences for.", - )]; + ) + .with_element_kind(kind::BYTES)]; PARAMETERS } } @@ -85,7 +86,7 @@ impl FunctionExpression for TallyFn { } fn type_def(&self, _: &state::TypeState) -> TypeDef { - TypeDef::object(Collection::from_unknown(Kind::integer())).fallible() + TypeDef::object(Collection::from_unknown(Kind::integer())) } } @@ -102,15 +103,17 @@ mod tests { value: value!(["bar", "foo", "baz", "foo"]), ], want: Ok(value!({"bar": 1, "foo": 2, "baz": 1})), - tdef: TypeDef::object(Collection::from_unknown(Kind::integer())).fallible(), + tdef: TypeDef::object(Collection::from_unknown(Kind::integer())), } + // Runtime error still fires; compiler-level fallibility is driven by + // `element_kind(kind::BYTES)` on the parameter, not by `type_def()`. non_string_values { args: func_args![ value: value!(["foo", [1,2,3], "123abc", 1, true, [1,2,3], "foo", true, 1]), ], want: Err("all values must be strings, found: Array([Integer(1), Integer(2), Integer(3)])"), - tdef: TypeDef::object(Collection::from_unknown(Kind::integer())).fallible(), + tdef: TypeDef::object(Collection::from_unknown(Kind::integer())), } ]; } diff --git a/src/stdlib/to_entries.rs b/src/stdlib/to_entries.rs index 134cb310fd..6b6d4ba54e 100644 --- a/src/stdlib/to_entries.rs +++ b/src/stdlib/to_entries.rs @@ -67,6 +67,7 @@ impl Function for ToEntries { &[Parameter { keyword: "value", kind: kind::OBJECT | kind::ARRAY, + element_kind: kind::ANY, required: true, description: "The object or array to manipulate.", default: None, diff --git a/src/value/kind/collection.rs b/src/value/kind/collection.rs index 69c14fda77..703dc0dc59 100644 --- a/src/value/kind/collection.rs +++ b/src/value/kind/collection.rs @@ -273,6 +273,45 @@ impl Collection { /// # Errors /// If the type is not a superset, a path to one field that doesn't match is returned. /// This is mostly useful for debugging. + /// Check if `self` and `other` could hold a common value. + /// + /// Two collections intersect unless a **known** element on one side is + /// provably disjoint from the corresponding kind on the other side. If + /// both collections have no known elements they always intersect (both + /// could be the empty collection, e.g. `[]` or `{}`). + #[must_use] + pub fn intersects(&self, other: &Self) -> bool { + // A known element in `self` must be compatible with whatever `other` + // expects at that position (its own known kind, or its unknown kind). + for (key, self_kind) in &self.known { + let other_kind = other + .known + .get(key) + .cloned() + .unwrap_or_else(|| other.unknown_kind()); + if !self_kind.intersects(&other_kind) { + return false; + } + } + + // Symmetric check from `other`'s perspective. + for (key, other_kind) in &other.known { + let self_kind = self + .known + .get(key) + .cloned() + .unwrap_or_else(|| self.unknown_kind()); + if !self_kind.intersects(other_kind) { + return false; + } + } + + true + } + + /// # Errors + /// + /// Returns the path of the first element where `self` is not a superset of `other`. pub fn is_superset(&self, other: &Self) -> Result<(), OwnedValuePath> { // `self`'s `unknown` needs to be a superset of `other`'s. self.unknown @@ -952,4 +991,49 @@ mod tests { assert_eq!(this.reduced_kind(), want, "{title}"); } } + + #[test] + fn test_collection_intersects() { + use std::collections::BTreeMap; + + // Both empty: always intersect. + let empty: Collection = Collection::empty(); + assert!(empty.intersects(&empty)); + + // Unknown-only collections with disjoint element kinds still intersect + // because both could be empty. + let bytes_col = Collection::::from_unknown(Kind::bytes()); + let int_col = Collection::::from_unknown(Kind::integer()); + assert!(bytes_col.intersects(&int_col)); + + // Known element on lhs is disjoint from rhs unknown → no intersection. + let lhs: Collection = Collection::from_parts( + BTreeMap::from([(0.into(), Kind::integer())]), + Kind::undefined(), + ); + assert!(!lhs.intersects(&bytes_col)); + // Symmetric. + assert!(!bytes_col.intersects(&lhs)); + + // Known element on lhs matches rhs unknown → intersect. + let lhs_bytes: Collection = Collection::from_parts( + BTreeMap::from([(0.into(), Kind::bytes())]), + Kind::undefined(), + ); + assert!(lhs_bytes.intersects(&bytes_col)); + + // Both have matching known elements → intersect. + let rhs_with_known: Collection = Collection::from_parts( + BTreeMap::from([(0.into(), Kind::bytes())]), + Kind::undefined(), + ); + assert!(lhs_bytes.intersects(&rhs_with_known)); + + // lhs known element disjoint from rhs known at same index → no intersection. + let rhs_int_known: Collection = Collection::from_parts( + BTreeMap::from([(0.into(), Kind::integer())]), + Kind::undefined(), + ); + assert!(!lhs_bytes.intersects(&rhs_int_known)); + } } diff --git a/src/value/kind/comparison.rs b/src/value/kind/comparison.rs index 03e7098fc1..ce836c3c9f 100644 --- a/src/value/kind/comparison.rs +++ b/src/value/kind/comparison.rs @@ -287,7 +287,7 @@ impl Kind { /// /// Returns `true` if there are type states common to both `self` and `other`. #[must_use] - pub const fn intersects(&self, other: &Self) -> bool { + pub fn intersects(&self, other: &Self) -> bool { // a "never" type can be treated as any type if self.is_never() || other.is_never() { return true; @@ -325,8 +325,11 @@ impl Kind { return true; } - if self.contains_array() && other.contains_array() { - return true; + // For arrays, also check that their element kinds are not provably + // disjoint. A known element on either side that cannot satisfy the + // other side's element kind rules out any common value. + if let (Some(lhs), Some(rhs)) = (self.array.as_ref(), other.array.as_ref()) { + return lhs.intersects(rhs); } if self.contains_object() && other.contains_object() { @@ -624,4 +627,90 @@ mod tests { assert_eq!(kind.is_exact(), want, "{title}"); } } + + #[test] + fn test_intersects_arrays() { + struct TestCase { + lhs: Kind, + rhs: Kind, + want: bool, + } + + for (title, TestCase { lhs, rhs, want }) in HashMap::from([ + ( + "array vs array", + TestCase { + lhs: Kind::array(Collection::from_unknown(Kind::bytes())), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: true, + }, + ), + ( + "array vs array: disjoint unknowns but both could be empty", + TestCase { + lhs: Kind::array(Collection::from_unknown(Kind::bytes())), + rhs: Kind::array(Collection::from_unknown(Kind::integer())), + want: true, + }, + ), + ( + "array vs array", + TestCase { + lhs: Kind::array(Collection::any()), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: true, + }, + ), + ( + "literal [integer, integer] vs array: known elements are disjoint", + TestCase { + lhs: Kind::array(BTreeMap::from([ + (0.into(), Kind::integer()), + (1.into(), Kind::integer()), + ])), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: false, + }, + ), + ( + "literal [bytes] vs array: known element matches", + TestCase { + lhs: Kind::array(BTreeMap::from([(0.into(), Kind::bytes())])), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: true, + }, + ), + ( + "empty literal [] vs array: no known elements to violate", + TestCase { + lhs: Kind::array(BTreeMap::default()), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: true, + }, + ), + ( + "literal [bytes, integer] vs array: one known element is disjoint", + TestCase { + lhs: Kind::array(BTreeMap::from([ + (0.into(), Kind::bytes()), + (1.into(), Kind::integer()), + ])), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: false, + }, + ), + ( + "non-array vs array: never intersect", + TestCase { + lhs: Kind::integer(), + rhs: Kind::array(Collection::from_unknown(Kind::bytes())), + want: false, + }, + ), + ]) { + assert_eq!(lhs.intersects(&rhs), want, "{title}"); + // intersects must be symmetric + assert_eq!(rhs.intersects(&lhs), want, "{title} (reversed)"); + } + } }