diff --git a/src/domain/engine.rs b/src/domain/engine.rs index bda1092..22eb372 100644 --- a/src/domain/engine.rs +++ b/src/domain/engine.rs @@ -146,26 +146,18 @@ impl<'i> Scope<'i> { } } - /// Returns the tablet pairs if this is a CodeBlock containing a single - /// list whose elements are all labelled values. + /// Returns the tablet entries if this is a CodeBlock containing a single + /// tablet. pub fn tablet(&self) -> Option>> { match self { Scope::CodeBlock { expressions, .. } => { - if expressions.len() == 1 { - if let Expression::List(elements, _) = &expressions[0] { - let pairs: Vec<&Pair<'i>> = elements - .iter() - .filter_map(|element| { - if let Expression::Pair(pair, _) = element { - Some(pair.as_ref()) - } else { - None - } - }) - .collect(); - if !pairs.is_empty() && pairs.len() == elements.len() { - return Some(pairs); - } + if let [Expression::Tablet(pairs, _)] = expressions.as_slice() { + if !pairs.is_empty() { + return Some( + pairs + .iter() + .collect(), + ); } } None @@ -382,6 +374,16 @@ fn render_expression(expr: &Expression) -> String { .collect(); format!("({})", items.join(", ")) } + Expression::Tablet(pairs, _) => { + if pairs.is_empty() { + return "[=]".to_string(); + } + let entries: Vec<_> = pairs + .iter() + .map(|pair| format!("\"{}\" = {}", pair.label, render_expression(&pair.value))) + .collect(); + format!("[{}]", entries.join(", ")) + } Expression::Hole(_) => "?".to_string(), Expression::Unit(_) => "()".to_string(), Expression::Separator => String::new(), @@ -407,26 +409,20 @@ impl<'i> Paragraph<'i> { targets } - /// Returns tablet pairs if a `CodeInline` in this paragraph is a single - /// list whose elements are all labelled values. + /// Returns tablet entries if a `CodeInline` in this paragraph is a single + /// tablet. pub fn tablet(&self) -> Option>> { for d in &self.0 { if let Descriptive::CodeInline(exprs) = d { - let [Expression::List(elements, _)] = exprs.as_slice() else { + let [Expression::Tablet(pairs, _)] = exprs.as_slice() else { continue; }; - let pairs: Vec<&Pair<'i>> = elements - .iter() - .filter_map(|element| { - if let Expression::Pair(pair, _) = element { - Some(pair.as_ref()) - } else { - None - } - }) - .collect(); - if !pairs.is_empty() && pairs.len() == elements.len() { - return Some(pairs); + if !pairs.is_empty() { + return Some( + pairs + .iter() + .collect(), + ); } } } diff --git a/src/editor/server.rs b/src/editor/server.rs index a5a088c..f446850 100644 --- a/src/editor/server.rs +++ b/src/editor/server.rs @@ -766,6 +766,10 @@ impl TechniqueLanguageServer { "Section content must be steps or procedures".to_string(), DiagnosticSeverity::ERROR, ), + ParsingError::MixedBracketContent(_) => ( + "Must either be a list (values) or a tablet (labelled pairs), not a mix of the two".to_string(), + DiagnosticSeverity::ERROR, + ), ParsingError::InvalidInvocation(_) => ( "Invalid procedure Invocation".to_string(), DiagnosticSeverity::ERROR, diff --git a/src/formatting/formatter.rs b/src/formatting/formatter.rs index ff5c718..0d7e638 100644 --- a/src/formatting/formatter.rs +++ b/src/formatting/formatter.rs @@ -292,26 +292,11 @@ fn hugs_left(word: &str) -> bool { } } -/// A list reads as a tablet when it is non-empty and every element is a -/// labelled value. Such lists are laid out and treated as blocks rather than -/// inline. -fn is_tablet_list(elements: &[Expression]) -> bool { - !elements.is_empty() - && elements - .iter() - .all(|element| { - if let Expression::Pair(_, _) = element { - true - } else { - false - } - }) -} - -/// True when an expression is a tablet-shaped list (see `is_tablet_list`). -fn is_tablet_list_expr(expr: &Expression) -> bool { - if let Expression::List(elements, _) = expr { - is_tablet_list(elements) +/// A tablet with entries is laid out as a block rather than inline. The empty +/// tablet `[=]` has nothing to lay out, so it stays inline. +fn is_tablet_block(expr: &Expression) -> bool { + if let Expression::Tablet(pairs, _) = expr { + !pairs.is_empty() } else { false } @@ -335,7 +320,7 @@ fn is_inline_code_block(expressions: &[Expression]) -> bool { if has_separator { true } else if expressions.len() == 1 { - !is_tablet_list_expr(&expressions[0]) + !is_tablet_block(&expressions[0]) } else { false } @@ -485,7 +470,7 @@ impl<'i> Formatter<'i> { } fn render_inline_code(&self, expr: &'i Expression) -> Vec<(Syntax, Cow<'i, str>)> { - if is_tablet_list_expr(expr) { + if is_tablet_block(expr) { // Not inline; caller handles the block layout specially. return Vec::new(); } @@ -900,7 +885,7 @@ impl<'i> Formatter<'i> { } } match expr { - _ if is_tablet_list_expr(expr) => { + _ if is_tablet_block(expr) => { line.flush(); self.append_char('\n'); self.indent(); @@ -1382,6 +1367,7 @@ impl<'i> Formatter<'i> { } Expression::Pair(pair, _) => self.append_pair(pair), Expression::List(elements, _) => self.append_list(elements), + Expression::Tablet(pairs, _) => self.append_tablet(pairs), Expression::Tuple(elements, _) => self.append_tuple(elements), Expression::Hole(_) => { self.add_fragment_reference(Syntax::Hole, "?"); @@ -1539,28 +1525,32 @@ impl<'i> Formatter<'i> { self.append_expression(&pair.value); } - /// A list whose elements are all labelled (a tablet) is laid out one - /// element per line; any other list, and the empty list, is inline. - fn append_list(&mut self, elements: &'i Vec) { - if elements.is_empty() { - self.add_fragment_reference(Syntax::Structure, "[]"); + /// A tablet is laid out one entry per line. The empty tablet has nothing + /// to lay out and is written `[=]`. + fn append_tablet(&mut self, pairs: &'i Vec) { + if pairs.is_empty() { + self.add_fragment_reference(Syntax::Structure, "[=]"); return; } - if is_tablet_list(elements) { - self.add_fragment_reference(Syntax::Structure, "["); + self.add_fragment_reference(Syntax::Structure, "["); + self.append_char('\n'); + + self.increase(4); + for pair in pairs { + self.indent(); + self.append_pair(pair); self.append_char('\n'); + } + self.decrease(4); - self.increase(4); - for element in elements { - self.indent(); - self.append_expression(element); - self.append_char('\n'); - } - self.decrease(4); + self.indent(); + self.add_fragment_reference(Syntax::Structure, "]"); + } - self.indent(); - self.add_fragment_reference(Syntax::Structure, "]"); + fn append_list(&mut self, elements: &'i Vec) { + if elements.is_empty() { + self.add_fragment_reference(Syntax::Structure, "[]"); return; } diff --git a/src/language/types.rs b/src/language/types.rs index 3937db4..8739269 100644 --- a/src/language/types.rs +++ b/src/language/types.rs @@ -453,6 +453,7 @@ pub enum Expression<'i> { Binding(Box>, Vec>, Span), Pair(Box>, Span), List(Vec>, Span), + Tablet(Vec>, Span), Tuple(Vec>, Span), Hole(Span), Unit(Span), @@ -482,6 +483,7 @@ impl PartialEq for Expression<'_> { } (Expression::Pair(a, _), Expression::Pair(b, _)) => a == b, (Expression::List(a, _), Expression::List(b, _)) => a == b, + (Expression::Tablet(a, _), Expression::Tablet(b, _)) => a == b, (Expression::Tuple(a, _), Expression::Tuple(b, _)) => a == b, (Expression::Hole(_), Expression::Hole(_)) => true, (Expression::Unit(_), Expression::Unit(_)) => true, diff --git a/src/parsing/checks/parser.rs b/src/parsing/checks/parser.rs index dd13654..3fd9df3 100644 --- a/src/parsing/checks/parser.rs +++ b/src/parsing/checks/parser.rs @@ -1,10 +1,5 @@ use super::*; -/// Test helper: a labelled value (`"label" = value`) with a default span. -fn pair<'i>(label: &'i str, value: Expression<'i>) -> Expression<'i> { - Expression::Pair(Box::new(Pair { label, value }), Span::default()) -} - #[test] fn magic_line() { let mut input = Parser::new(); @@ -1794,18 +1789,16 @@ echo test fn tablets() { let mut input = Parser::new(); - // Tablets are lists whose elements are all labelled values. - // Test simple single-entry tablet input.initialize(r#"{ ["name" = "Johannes Grammerly"] }"#); let result = input.read_code_block(); assert_eq!( result, - Ok(vec![Expression::List( - vec![pair( - "name", - Expression::String(vec![Piece::Text("Johannes Grammerly")], Span::default()) - )], + Ok(vec![Expression::Tablet( + vec![Pair { + label: "name", + value: Expression::String(vec![Piece::Text("Johannes Grammerly")], Span::default()) + }], Span::default() )]) ); @@ -1820,16 +1813,19 @@ fn tablets() { let result = input.read_code_block(); assert_eq!( result, - Ok(vec![Expression::List( + Ok(vec![Expression::Tablet( vec![ - pair( - "name", - Expression::String(vec![Piece::Text("Alice of Chains")], Span::default()) - ), - pair( - "age", - Expression::String(vec![Piece::Text("29")], Span::default()) - ) + Pair { + label: "name", + value: Expression::String( + vec![Piece::Text("Alice of Chains")], + Span::default() + ) + }, + Pair { + label: "age", + value: Expression::String(vec![Piece::Text("29")], Span::default()) + } ], Span::default() )]) @@ -1846,26 +1842,26 @@ fn tablets() { let result = input.read_code_block(); assert_eq!( result, - Ok(vec![Expression::List( + Ok(vec![Expression::Tablet( vec![ - pair( - "answer", - Expression::Number(Numeric::Integral(42), Span::default()) - ), - pair( - "message", - Expression::Variable(Identifier::new("msg"), Span::default()) - ), - pair( - "timestamp", - Expression::Execution( + Pair { + label: "answer", + value: Expression::Number(Numeric::Integral(42), Span::default()) + }, + Pair { + label: "message", + value: Expression::Variable(Identifier::new("msg"), Span::default()) + }, + Pair { + label: "timestamp", + value: Expression::Execution( Function { target: Identifier::new("now"), parameters: vec![] }, Span::default() ) - ) + } ], Span::default() )]) @@ -1876,6 +1872,19 @@ fn tablets() { let result = input.read_code_block(); assert_eq!(result, Ok(vec![Expression::List(vec![], Span::default())])); + // whereas `[=]` is the empty tablet + input.initialize("{ [=] }"); + let result = input.read_code_block(); + assert_eq!( + result, + Ok(vec![Expression::Tablet(vec![], Span::default())]) + ); + + // written exactly, the same way unit is `()` and not `( )` + input.initialize("{ [ = ] }"); + let result = input.read_code_block(); + assert!(result.is_err()); + // Test tablet with interpolated string values input.initialize( r#"{ [ @@ -1886,19 +1895,19 @@ fn tablets() { let result = input.read_code_block(); assert_eq!( result, - Ok(vec![Expression::List( + Ok(vec![Expression::Tablet( vec![ - pair( - "context", - Expression::String( + Pair { + label: "context", + value: Expression::String( vec![Piece::Text("Details about the thing")], Span::default() ) - ), - pair( - "status", - Expression::Variable(Identifier::new("active"), Span::default()) - ) + }, + Pair { + label: "status", + value: Expression::Variable(Identifier::new("active"), Span::default()) + } ], Span::default() )]) @@ -2081,16 +2090,16 @@ fn tablet_inline_commas() { let result = input.read_code_block(); assert_eq!( result, - Ok(vec![Expression::List( + Ok(vec![Expression::Tablet( vec![ - pair( - "answer", - Expression::Number(Numeric::Integral(42), Span::default()) - ), - pair( - "truth", - Expression::String(vec![Piece::Text("yes")], Span::default()) - ) + Pair { + label: "answer", + value: Expression::Number(Numeric::Integral(42), Span::default()) + }, + Pair { + label: "truth", + value: Expression::String(vec![Piece::Text("yes")], Span::default()) + } ], Span::default() )]) @@ -2098,27 +2107,21 @@ fn tablet_inline_commas() { } #[test] -fn bracket_mixed_pairs_and_values_parses() { +fn bracket_mixed_pairs_and_values_rejected() { let mut input = Parser::new(); - // The parser makes no tablet/list judgement: a bracket mixing a labelled - // value with a bare value parses as a list with mixed elements. Rejecting - // it is a translation-stage concern. - input.initialize(r#"{ [ "answer" = 42, 99 ] }"#); - let result = input.read_code_block(); - assert_eq!( - result, - Ok(vec![Expression::List( - vec![ - pair( - "answer", - Expression::Number(Numeric::Integral(42), Span::default()) - ), - Expression::Number(Numeric::Integral(99), Span::default()) - ], - Span::default() - )]) - ); + // A bracket mixing a labelled value with a bare value is neither a tablet + // nor a list. Everything needed to decide sits between the brackets, so + // the parser rejects it rather than deferring to a later phase. + input.initialize("% technique v1\nrun :\n{ [ \"answer\" = 42, 99 ] }"); + let errors = input + .parse_collecting_errors() + .expect_err("mixed bracket content should fail to parse"); + + assert_eq!(errors.len(), 1); + let ParsingError::MixedBracketContent(_) = &errors[0] else { + panic!("expected MixedBracketContent, got {:?}", errors[0]); + }; } #[test] diff --git a/src/parsing/parser.rs b/src/parsing/parser.rs index eb896c4..23a90bc 100644 --- a/src/parsing/parser.rs +++ b/src/parsing/parser.rs @@ -44,6 +44,7 @@ pub enum ParsingError { InvalidDeclaration(Span), InvalidSection(Span), MixedSectionContent(Span), + MixedBracketContent(Span), InvalidInvocation(Span), InvalidFunction(Span), InvalidTuple(Span), @@ -81,6 +82,7 @@ impl ParsingError { | ParsingError::InvalidDeclaration(span) | ParsingError::InvalidSection(span) | ParsingError::MixedSectionContent(span) + | ParsingError::MixedBracketContent(span) | ParsingError::InvalidInvocation(span) | ParsingError::InvalidFunction(span) | ParsingError::InvalidTuple(span) @@ -1801,12 +1803,19 @@ impl<'i> Parser<'i> { Ok(Expression::Binding(Box::new(expression), identifiers, span)) } - /// Read a list. Elements are comma or newline-separated expressions. An - /// element is of the form `"label" = value` for a labelled tablet value - /// (an `Expression::Pair`) or without a label as an indexed list element - /// (an `Expression:List`). + /// Read a bracket literal. Elements are comma or newline-separated + /// expressions. If every element is of the form `"label" = value` that + /// makes it an `Expression::Tablet`. If there are no labels at all that + /// makes an `Expression::List`. Mixing the two is an error and is + /// reported as MixedBracketContent. fn read_bracket_expression(&mut self) -> Result, ParsingError> { let start = self.offset; + + if is_empty_tablet(self.source) { + self.advance(3); + return Ok(Expression::Tablet(vec![], self.span_since(start))); + } + let elements = self.take_block_chars("a list", '[', ']', true, |outer| { outer.take_elements(true, |inner| { if is_pair(inner.source) { @@ -1825,7 +1834,40 @@ impl<'i> Parser<'i> { }) })?; let span = self.span_since(start); - Ok(Expression::List(elements, span)) + + let labelled = elements + .iter() + .filter(|element| { + if let Expression::Pair(_, _) = element { + true + } else { + false + } + }) + .count(); + + if labelled == 0 { + return Ok(Expression::List(elements, span)); + } + + if labelled < elements.len() { + self.problems + .push(ParsingError::MixedBracketContent(span)); + return Ok(Expression::List(elements, span)); + } + + let pairs = elements + .into_iter() + .map(|element| { + // every element is labelled, tested just above + let Expression::Pair(pair, _) = element else { + unreachable!() + }; + *pair + }) + .collect(); + + Ok(Expression::Tablet(pairs, span)) } /// Read a tuple: two or more comma-separated expressions in @@ -3481,6 +3523,12 @@ fn is_pair(content: &str) -> bool { } } +/// Detect the empty tablet literal `[=]`. A bare `[]` is the empty list; the +/// `=` is what distinguishes the two. +fn is_empty_tablet(content: &str) -> bool { + content.starts_with("[=]") +} + fn is_attribute_assignment(input: &str) -> bool { // Matches any combination of @ and ^ attributes separated by + // Also matches the special @* "reset to all" role diff --git a/src/problem/messages.rs b/src/problem/messages.rs index 9998f85..a49138c 100644 --- a/src/problem/messages.rs +++ b/src/problem/messages.rs @@ -503,6 +503,18 @@ can write a helper procedure that follows .trim_ascii() .to_string(), ), + ParsingError::MixedBracketContent(_) => ( + "Bracket mixes List and Tablet syntax".to_string(), + r#" +A `[...]` literal must be either a Tablet (every entry in the list a +`"label" = value` pair) or a List (entries are actual values in +sequence), not a mix of the two. + +An empty List is written `[]`. An empty Tablet is written `[=]`. + "# + .trim_ascii() + .to_string(), + ), ParsingError::InvalidInvocation(_) => { let examples = vec![ Invocation { @@ -1197,16 +1209,6 @@ cannot be bound to a variable. .trim_ascii() .to_string(), ), - TranslationError::HeterogenousList { .. } => ( - "Mixed List and Tablet syntax".to_string(), - r#" -A `[...]` literal must be either a Tablet (every entry in the list a -`"label" = value` pair) or a List (entries are actual values in -sequence), not a mix of the two. - "# - .trim_ascii() - .to_string(), - ), } } diff --git a/src/translation/checks/errors.rs b/src/translation/checks/errors.rs index 033b8d7..959cc14 100644 --- a/src/translation/checks/errors.rs +++ b/src/translation/checks/errors.rs @@ -225,27 +225,3 @@ This text is past the body. .expect("description in source"); assert_eq!(at.offset, expected); } - -#[test] -fn mixed_bracket_entries() { - // A bracket mixing a labelled value with a bare value is neither a - // tablet nor a list; the parser accepts it but translation rejects it. - let source = r#" -% technique v1 - -run : - -{ - [ "answer" = 42, 99 ] -} - "# - .trim_ascii(); - let path = Path::new("Test.tq"); - let document = parsing::parse(path, source).expect("parse"); - let errors = translate(&document).expect_err("translate should fail"); - - assert_eq!(errors.len(), 1); - let TranslationError::HeterogenousList { .. } = &errors[0] else { - panic!("expected HeterogenousList, got {:?}", errors[0]); - }; -} diff --git a/src/translation/checks/translate.rs b/src/translation/checks/translate.rs index fbe519b..3e24e42 100644 --- a/src/translation/checks/translate.rs +++ b/src/translation/checks/translate.rs @@ -912,6 +912,43 @@ run : assert_eq!(entries[1].label, "weight"); } +#[test] +fn expression_empty_tablet_translates() { + let source = r#" +% technique v1 + +run : + +{ + [=] ~ empty + [] ~ nothing +} + "# + .trim_ascii(); + let path = Path::new("Test.tq"); + let document = parsing::parse(path, source).expect("parse"); + let program = translate(&document).expect("translate"); + + let Operation::Sequence(ops, _) = &program.subroutines[0].body else { + panic!("expected Sequence"); + }; + let Operation::Bind { value: tablet, .. } = &ops[0] else { + panic!("expected Bind, got {:?}", ops[0]); + }; + let Operation::Tablet(entries, _) = tablet.as_ref() else { + panic!("expected Tablet, got {:?}", tablet); + }; + assert!(entries.is_empty()); + + let Operation::Bind { value: list, .. } = &ops[1] else { + panic!("expected Bind, got {:?}", ops[1]); + }; + let Operation::List(items, _) = list.as_ref() else { + panic!("expected List, got {:?}", list); + }; + assert!(items.is_empty()); +} + #[test] fn expression_list_translates() { let source = r#" diff --git a/src/translation/translator.rs b/src/translation/translator.rs index 65dc199..e5adc21 100644 --- a/src/translation/translator.rs +++ b/src/translation/translator.rs @@ -75,12 +75,6 @@ pub enum TranslationError<'i> { BoundRepeat { at: Span, }, - /// A list mixing labelled pairs (`"label" = value`) with bare values is - /// neither a tablet nor a plain list. The two forms can't be combined in - /// one set of brackets. - HeterogenousList { - at: Span, - }, } impl<'i> TranslationError<'i> { @@ -91,7 +85,6 @@ impl<'i> TranslationError<'i> { TranslationError::InterleavedDescription { at, .. } => *at, TranslationError::SignatureParameterMismatch { procedure, .. } => procedure.span, TranslationError::BoundRepeat { at } => *at, - TranslationError::HeterogenousList { at } => *at, } } } @@ -584,6 +577,7 @@ impl<'i> Translator<'i> { | [expr @ language::Expression::Multiline(..)] | [expr @ language::Expression::Pair(..)] | [expr @ language::Expression::List(..)] + | [expr @ language::Expression::Tablet(..)] | [expr @ language::Expression::Tuple(..)] => { Some(Fragment::Interpolation(self.translate_expression(expr))) } @@ -782,55 +776,22 @@ impl<'i> Translator<'i> { *span, ) } - language::Expression::List(elements, span) => { - let labelled = elements + language::Expression::Tablet(pairs, span) => { + let entries = pairs .iter() - .any(|element| { - if let language::Expression::Pair(..) = element { - true - } else { - false - } - }); - let unlabelled = elements + .map(|pair| Entry { + label: pair.label, + value: self.translate_expression(&pair.value), + }) + .collect(); + Operation::Tablet(entries, *span) + } + language::Expression::List(elements, span) => { + let items = elements .iter() - .any(|element| { - if let language::Expression::Pair(..) = element { - false - } else { - true - } - }); - - if labelled && unlabelled { - self.problems - .push(TranslationError::HeterogenousList { at: *span }); - } - - // All elements labelled: a tablet. Otherwise (including the - // empty list and the mixed-content recovery case) a list. - if labelled && !unlabelled { - let entries = elements - .iter() - .filter_map(|element| { - if let language::Expression::Pair(pair, _) = element { - Some(Entry { - label: pair.label, - value: self.translate_expression(&pair.value), - }) - } else { - None - } - }) - .collect(); - Operation::Tablet(entries, *span) - } else { - let items = elements - .iter() - .map(|element| self.translate_expression(element)) - .collect(); - Operation::List(items, *span) - } + .map(|element| self.translate_expression(element)) + .collect(); + Operation::List(items, *span) } language::Expression::Tuple(elements, span) => { let items = elements diff --git a/tests/broken/parsing/MixedBracketContent.tq b/tests/broken/parsing/MixedBracketContent.tq new file mode 100644 index 0000000..085e17e --- /dev/null +++ b/tests/broken/parsing/MixedBracketContent.tq @@ -0,0 +1,6 @@ +% technique v1 + +record : +{ + [ "answer" = 42, 99 ] +} diff --git a/tests/formatting/formatter.rs b/tests/formatting/formatter.rs index 1ea915f..f152095 100644 --- a/tests/formatting/formatter.rs +++ b/tests/formatting/formatter.rs @@ -336,31 +336,25 @@ We must take action! Span::default(), )], subscopes: vec![Scope::CodeBlock { - expressions: vec![Expression::List( + expressions: vec![Expression::Tablet( vec![ - Expression::Pair( - Box::new(Pair { - label: "timestamp", - value: Expression::Execution( - Function { - target: Identifier::new("now"), - parameters: vec![], - }, - Span::default(), - ), - }), - Span::default(), - ), - Expression::Pair( - Box::new(Pair { - label: "message", - value: Expression::Variable( - Identifier::new("msg"), - Span::default(), - ), - }), - Span::default(), - ), + Pair { + label: "timestamp", + value: Expression::Execution( + Function { + target: Identifier::new("now"), + parameters: vec![], + }, + Span::default(), + ), + }, + Pair { + label: "message", + value: Expression::Variable( + Identifier::new("msg"), + Span::default(), + ), + }, ], Span::default(), )],