Skip to content

Commit

Permalink
chore!: Drop support for coalesce paths
Browse files Browse the repository at this point in the history
  • Loading branch information
bruceg committed May 10, 2024
1 parent da595ab commit f890fe2
Show file tree
Hide file tree
Showing 32 changed files with 48 additions and 1,176 deletions.
1 change: 1 addition & 0 deletions changelog.d/NNN.breaking.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The deprecated coalesce paths (i.e. `(field1|field2)`) feature is now removed.
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,6 @@
# = see documentation about error handling at https://errors.vrl.dev/#handling
# = see language documentation at https://vrl.dev
# = try your code in the VRL REPL, learn more at https://vrl.dev/examples
#
# error[E642]: parent path segment rejects this mutation
# ┌─ :14:5
# │
# 14 │ foo.(bar | baz) = "bar baz"
# │ --- ^^^^^^^^^^^ querying a field of a non-object type is unsupported
# │ │
# │ this path resolves to a value of type integer
# │
# = try: change parent value to object, before assignment
# =
# = foo = {}
# = foo.(bar | baz) = "bar baz"
# =
# = see documentation about error handling at https://errors.vrl.dev/#handling
# = see language documentation at https://vrl.dev
# = try your code in the VRL REPL, learn more at https://vrl.dev/examples


foo = "foo"
Expand All @@ -97,7 +80,6 @@ foo.bar.baz = "baz"
.foo.bar = "bar"

foo = 42
foo.(bar | baz) = "bar baz"

foo = {}
foo.bar = {}
Expand Down
2 changes: 1 addition & 1 deletion lib/tests/tests/diagnostics/syntax_error_path_segment.vrl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
# │ ^
# │ │
# │ unexpected end of query path
# │ expected one of: "(", "abort", "identifier", "path field", "return", "string literal"
# │ expected one of: "abort", "identifier", "path field", "return", "string literal"
# │
# = see language documentation at https://vrl.dev
# = try your code in the VRL REPL, learn more at https://vrl.dev/examples
Expand Down
12 changes: 0 additions & 12 deletions lib/tests/tests/expressions/assignment/read_only_coalesce.vrl

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion lib/tests/tests/expressions/query/ampersat.vrl
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# object: { "foo@bar": { "@buz" : [ { "wibble@" : 42 } ] } }
# result: 42
.foo@bar.(@noog | @buz)[0].wibble@
.foo@bar.@buz[0].wibble@
4 changes: 0 additions & 4 deletions lib/tests/tests/expressions/query/coalesce.vrl

This file was deleted.

2 changes: 1 addition & 1 deletion lib/tests/tests/functions/del/external_del_type_def.vrl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@

. = {}
.test1 = {"a": 5, "b": 6}
del(.test1.(a|b))
del(.test1.a)

type_def(.)
6 changes: 3 additions & 3 deletions lib/tests/tests/internal/query_ignore_parens_in_quotes.vrl
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ _x = r'\).'
_x = upcase(").")
_x = upcase(s').')
_x = replace("", r'\).', "")
.foo.(bar | "baz)")
.foo.(bar | "{foo}")
.foo.(bar | "[{bar}]")
.foo."baz)"
.foo."{foo}"
.foo."[{bar}]"
2 changes: 0 additions & 2 deletions lib/tests/tests/issues/13461/1.vrl

This file was deleted.

2 changes: 0 additions & 2 deletions lib/tests/tests/issues/13461/2.vrl

This file was deleted.

2 changes: 1 addition & 1 deletion src/compiler/expression/assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ fn verify_overwritable(
// approach should be taken here
// https://github.com/vectordotdev/vrl/issues/206
let (variant, segment_span, valid) = match last {
segment @ (OwnedSegment::Field(_) | OwnedSegment::Coalesce(_)) => {
segment @ OwnedSegment::Field(_) => {
let segment_str = segment.to_string();
let segment_start = parent_span.end().saturating_sub(segment_str.len());
let segment_span = Span::new(segment_start, parent_span.end());
Expand Down
22 changes: 0 additions & 22 deletions src/compiler/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,11 @@ pub trait Target: std::fmt::Debug + SecretTarget {
/// .foo."bar.baz"
/// ```
///
/// * coalesced path segments:
///
/// ```txt
/// .foo.(bar | "bar.baz").qux
/// ```
///
/// * path indices:
///
/// ```txt
/// .foo[2][-1]
/// ```
///
/// When inserting into a coalesced path, the implementor is encouraged to
/// insert into the right-most segment if none exists, but can return an
/// error if needed.
fn target_insert(&mut self, path: &OwnedTargetPath, value: Value) -> Result<(), String>;

/// Get a value for a given path, or `None` if no value is found.
Expand Down Expand Up @@ -313,11 +303,6 @@ mod tests {
owned_value_path!("foo", 0, "bar"),
Ok(Some(value!(true))),
),
(
value!({foo: {"bar baz": {baz: 2}}}),
owned_value_path!("foo", vec!["qux", "bar baz"], "baz"),
Ok(Some(value!(2))),
),
];

for (value, path, expect) in cases {
Expand Down Expand Up @@ -463,13 +448,6 @@ mod tests {
Some(value!("bar")),
Some(value!({})),
),
(
value!({foo: "bar"}),
owned_value_path!(vec!["foo bar", "foo"]),
false,
Some(value!("bar")),
Some(value!({})),
),
(
value!({foo: "bar", baz: "qux"}),
owned_value_path!(),
Expand Down
12 changes: 0 additions & 12 deletions src/compiler/unused_expression_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,18 +556,6 @@ mod test {
unused_test(source, vec!["unused variable `y`".to_string()]);
}

#[test]
fn unused_coalesce_result() {
let source = indoc! {r#"
parse_syslog("not syslog") ?? parse_common_log("not common") ?? "malformed"
.res = parse_syslog("not syslog") ?? parse_common_log("not common") ?? "malformed"
"#};
unused_test(
source,
vec![r#"unused result for function call `parse_syslog("not syslog")`"#.to_string()],
);
}

#[test]
fn used_queries() {
let source = indoc! {r#"
Expand Down
47 changes: 1 addition & 46 deletions src/parser/lex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,6 @@ impl DiagnosticMessage for Error {
}
}

#[derive(Debug)]
enum CoalesceState {
Dot,
Coalesce,
}

// -----------------------------------------------------------------------------
// lexer
// -----------------------------------------------------------------------------
Expand All @@ -215,9 +209,6 @@ pub(crate) struct Lexer<'input> {
open_parens: usize,
query_start: Option<usize>,

// used to track if the lexer is inside a coalesce (to differentiate a '@' path prefix from a coalesce field starting with '@')
coalesce_state: Option<CoalesceState>,

/// Keep track of when the lexer is supposed to emit an `RQuery` token.
///
/// For example:
Expand Down Expand Up @@ -692,18 +683,7 @@ impl<'input> Iterator for Lexer<'input> {
type Item = SpannedResult<'input, usize>;

fn next(&mut self) -> Option<Self::Item> {
let result = self.next_token();
if let Some(Ok((_, token, _))) = &result {
self.coalesce_state = match (&self.coalesce_state, token) {
(None, Token::Dot) => Some(CoalesceState::Dot),
(Some(CoalesceState::Coalesce), Token::RParen) => None,
(Some(CoalesceState::Coalesce), _) | (Some(CoalesceState::Dot), Token::LParen) => {
Some(CoalesceState::Coalesce)
}
_ => None,
};
}
result
self.next_token()
}
}

Expand Down Expand Up @@ -757,11 +737,6 @@ impl<'input> Lexer<'input> {
return Ok(false);
}

// If we are in the middle of a coalesce query, this can't be the start of another query
if matches!(self.coalesce_state, Some(CoalesceState::Coalesce)) {
return Ok(false);
}

// Take a clone of the existing chars iterator, to allow us to look
// ahead without advancing the lexer's iterator. This is cheap, since
// the original iterator only holds references.
Expand Down Expand Up @@ -1195,7 +1170,6 @@ impl<'input> Lexer<'input> {
open_parens: 0,
rquery_indices: vec![],
query_start: None,
coalesce_state: None,
}
}

Expand Down Expand Up @@ -1780,25 +1754,6 @@ mod test {
);
}

#[test]
fn coalesced_queries() {
test(
data(".foo.(bar | baz)"),
vec![
("~ ", LQuery),
("~ ", Dot),
(" ~~~ ", Identifier("foo")),
(" ~ ", Dot),
(" ~ ", LParen),
(" ~~~ ", Identifier("bar")),
(" ~ ", Operator("|")),
(" ~~~ ", Identifier("baz")),
(" ~", RParen),
(" ~", RQuery),
],
);
}

#[test]
fn complex_query_1() {
use StringLiteral as L;
Expand Down
5 changes: 0 additions & 5 deletions src/parser/parser.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,6 @@ Path: OwnedValuePath = PathSegment+ => OwnedValuePath::from(<>);
PathSegment: OwnedSegment = {
"."? <Field> => OwnedSegment::field(&<>),
"[" <Integer> "]" => OwnedSegment::index(<> as isize),
"."? "(" <v:(<Field> "|")+> <e:Field> ")" => {
let mut v = v;
v.push(e);
OwnedSegment::coalesce(v)
},
};

#[inline]
Expand Down
25 changes: 10 additions & 15 deletions src/path/borrowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::borrow::Cow;
use std::iter::Cloned;
use std::slice::Iter;

use super::{PathPrefix, TargetPath, ValuePath};
use super::{OwnedSegment, PathPrefix, TargetPath, ValuePath};

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct BorrowedValuePath<'a, 'b> {
Expand Down Expand Up @@ -35,8 +35,6 @@ impl<'a, 'b> BorrowedTargetPath<'a, 'b> {
pub enum BorrowedSegment<'a> {
Field(Cow<'a, str>),
Index(isize),
CoalesceField(Cow<'a, str>),
CoalesceEnd(Cow<'a, str>),
Invalid,
}

Expand All @@ -58,6 +56,15 @@ impl BorrowedSegment<'_> {
}
}

impl<'a> From<&'a OwnedSegment> for BorrowedSegment<'a> {
fn from(segment: &'a OwnedSegment) -> Self {
match segment {
OwnedSegment::Field(field) => Self::Field(field.as_str().into()),
OwnedSegment::Index(i) => Self::Index(*i),
}
}
}

impl<'a> From<&'a str> for BorrowedSegment<'a> {
fn from(field: &'a str) -> Self {
BorrowedSegment::field(field)
Expand Down Expand Up @@ -128,18 +135,6 @@ impl quickcheck::Arbitrary for BorrowedSegment<'static> {
.shrink()
.map(|f| BorrowedSegment::Field(f.into())),
),
BorrowedSegment::CoalesceField(field) => Box::new(
field
.to_string()
.shrink()
.map(|f| BorrowedSegment::Field(f.into())),
),
BorrowedSegment::CoalesceEnd(field) => Box::new(
field
.to_string()
.shrink()
.map(|f| BorrowedSegment::Field(f.into())),
),
}
}
}
Loading

0 comments on commit f890fe2

Please sign in to comment.