Error if trailing free-form text after responses

Author: istathar

src/editor/server.rs

@@ -770,6 +770,10 @@ impl TechniqueLanguageServer {
                     "Must either be a list (values) or a tablet (labelled pairs), not a mix of the two".to_string(),
                     DiagnosticSeverity::ERROR,
                 ),
+                ParsingError::MixedStepContent(_) => (
+                    "A step is described first and answered second, so text can't follow responses".to_string(),
+                    DiagnosticSeverity::ERROR,
+                ),
                 ParsingError::InvalidInvocation(_) => (
                     "Invalid procedure Invocation".to_string(),
                     DiagnosticSeverity::ERROR,

src/language/types.rs

@@ -649,28 +649,6 @@ pub(crate) fn validate_genus(input: &str, span: Span) -> Option<Genus<'_>> {
     }
 }
 
-pub fn validate_response(input: &str) -> Option<Response<'_>> {
-    if input.len() == 0 {
-        return None;
-    }
-
-    // A response is the quoted value and nothing else. There has to be one,
-    // and it must not be padded as `'Yes'` and `' Yes '` would differ but
-    // render identically.
-    let re = regex!(r"^'([^'\s](?:[^']*[^'\s])?)'$");
-    let cap = re.captures(input)?;
-
-    let value = cap
-        .get(1)
-        .unwrap()
-        .as_str();
-
-    Some(Response {
-        value,
-        span: Span::default(),
-    })
-}
-
 #[cfg(test)]
 mod check {
     use super::*;

src/parsing/checks/errors.rs

@@ -171,6 +171,21 @@ making_coffee :
     );
 }
 
+#[test]
+fn invalid_text_after_responses() {
+    expect_error(
+        r#"
+making_coffee :
+
+    1. Do you want coffee?
+        'Yes' | 'No'
+        Tell the barista.
+            "#
+        .trim_ascii(),
+        ParsingError::MixedStepContent(Span::new(73, 0)),
+    );
+}
+
 #[test]
 fn invalid_multiline_missing_closing() {
     expect_error(

src/parsing/checks/parser.rs

@@ -288,6 +288,13 @@ fn response_as_expression() {
             Span::default()
         ))
     );
+
+    // and the value is held to the same rule as one in an enum
+    input.initialize("' Monarchy '");
+    assert_eq!(
+        input.read_expression(),
+        Err(ParsingError::InvalidResponse(Span::new(0, 0)))
+    );
 }
 
 #[test]
@@ -2502,8 +2509,12 @@ fn splitting_by() {
     // different split character
     input.initialize("'Yes'|'No'|'Maybe'");
     let result = input.take_split_by('|', |inner| {
-        validate_response(inner.source)
-            .ok_or(ParsingError::IllegalParserState(Span::new(inner.offset, 0)))
+        inner
+            .read_enum_response()
+            .map(|value| Response {
+                value,
+                span: Span::default(),
+            })
     });
     assert_eq!(
         result,
@@ -2598,6 +2609,51 @@ fn reading_responses() {
             }
         ])
     );
+
+    // A padded or empty value is not a response
+    input.initialize("' Yes '");
+    let result = input.read_responses();
+    assert_eq!(result, Err(ParsingError::InvalidResponse(Span::new(0, 0))));
+
+    input.initialize("''");
+    let result = input.read_responses();
+    assert_eq!(result, Err(ParsingError::InvalidResponse(Span::new(0, 0))));
+
+    // The enum ends with its lines, leaving what follows to the enclosing
+    // scope, and a wrapped enum continues onto the next line
+    input.initialize("'Yes' | 'No'\n    { x ~ y }");
+    let result = input.read_responses();
+    assert_eq!(
+        result,
+        Ok(vec![
+            Response {
+                value: "Yes",
+                span: Span::default()
+            },
+            Response {
+                value: "No",
+                span: Span::default()
+            }
+        ])
+    );
+    assert_eq!(input.source, "    { x ~ y }");
+
+    input.initialize("'Yes' |\n    'No'\n    { x ~ y }");
+    let result = input.read_responses();
+    assert_eq!(
+        result,
+        Ok(vec![
+            Response {
+                value: "Yes",
+                span: Span::default()
+            },
+            Response {
+                value: "No",
+                span: Span::default()
+            }
+        ])
+    );
+    assert_eq!(input.source, "    { x ~ y }");
 }
 
 #[test]

src/parsing/parser.rs

@@ -45,6 +45,7 @@ pub enum ParsingError {
     InvalidSection(Span),
     MixedSectionContent(Span),
     MixedBracketContent(Span),
+    MixedStepContent(Span),
     InvalidInvocation(Span),
     InvalidFunction(Span),
     InvalidTuple(Span),
@@ -83,6 +84,7 @@ impl ParsingError {
             | ParsingError::InvalidSection(span)
             | ParsingError::MixedSectionContent(span)
             | ParsingError::MixedBracketContent(span)
+            | ParsingError::MixedStepContent(span)
             | ParsingError::InvalidInvocation(span)
             | ParsingError::InvalidFunction(span)
             | ParsingError::InvalidTuple(span)
@@ -1568,8 +1570,7 @@ impl<'i> Parser<'i> {
             let span = self.span_since(start);
             Ok(Expression::String(parts, span))
         } else if is_enum_response(content) {
-            let value =
-                self.take_block_chars("a response literal", '\'', '\'', |inner| Ok(inner.source))?;
+            let value = self.read_enum_response()?;
             let span = self.span_since(start);
             Ok(Expression::Response(value, span))
         } else if is_invocation(content) {
@@ -2613,19 +2614,48 @@ impl<'i> Parser<'i> {
         )
     }
 
+    /// Parse a single response literal like 'Yes'
+    fn read_enum_response(&mut self) -> Result<&'i str, ParsingError> {
+        let start = self.offset;
+        let value =
+            self.take_block_chars("a response literal", '\'', '\'', |inner| Ok(inner.source))?;
+
+        // There has to be a value, and it must not be padded as `'Yes'` and
+        // `' Yes '` would differ but render identically.
+        if value.is_empty() || value != value.trim_ascii() {
+            return Err(ParsingError::InvalidResponse(Span::new(start, 0)));
+        }
+
+        Ok(value)
+    }
+
     /// Parse enum responses like 'Yes' | 'No' | 'Not Applicable'
     fn read_responses(&mut self) -> Result<Vec<Response<'i>>, ParsingError> {
-        self.take_split_by('|', |inner| {
-            let mut resp = validate_response(inner.source)
-                .ok_or(ParsingError::InvalidResponse(Span::new(inner.offset, 0)))?;
-            resp.span = Span::new(
-                inner.offset,
-                inner
-                    .source
-                    .len(),
-            );
-            Ok(resp)
-        })
+        // The block is the run of lines beginning with a response literal, so
+        // that whatever follows the enum is left for the enclosing scope.
+        self.take_block_lines(
+            is_enum_response,
+            |line| !is_enum_response(line),
+            |outer| {
+                outer.take_split_by('|', |inner| {
+                    let span = Span::new(
+                        inner.offset,
+                        inner
+                            .source
+                            .len(),
+                    );
+                    let value = inner.read_enum_response()?;
+
+                    // a response is the literal and nothing else
+                    inner.trim_whitespace();
+                    if !inner.is_finished() {
+                        return Err(ParsingError::InvalidResponse(Span::new(span.offset, 0)));
+                    }
+
+                    Ok(Response { value, span })
+                })
+            },
+        )
     }
 
     fn parse_multiline_content(&mut self) -> Result<(Option<&'i str>, Vec<&'i str>), ParsingError> {
@@ -2844,6 +2874,10 @@ impl<'i> Parser<'i> {
                     span: self.span_since(responses_start),
                 });
             } else {
+                // an enum answers the step it is in, so text cannot follow it
+                if let Some(Scope::ResponseBlock { .. }) = scopes.last() {
+                    return Err(ParsingError::MixedStepContent(Span::new(self.offset, 0)));
+                }
                 break;
             }
         }

src/problem/messages.rs

@@ -515,6 +515,23 @@ An empty List is written `[]`. An empty Tablet is written `[=]`.
             .trim_ascii()
             .to_string(),
         ),
+        ParsingError::MixedStepContent(_) => (
+            "Step mixes description and responses".to_string(),
+            r#"
+A step is described first and answered second, so the responses enumerating
+the choices valid for its result come last; free-form text cannot follow
+them:
+
+    1.  Do you want coffee?
+            'Yes' | 'No'
+        Tell the barista.
+
+The trailing sentence needs to be above the responses, or in a substep of its
+own.
+            "#
+            .trim_ascii()
+            .to_string(),
+        ),
         ParsingError::InvalidInvocation(_) => {
             let examples = vec![
                 Invocation {