Better detection of misplaced procedure declarations in sections

src/editor/server.rs

@@ -762,6 +762,10 @@ impl TechniqueLanguageServer {
                     "Invalid section heading".to_string(),
                     DiagnosticSeverity::ERROR,
                 ),
+                ParsingError::MixedSectionContent(_) => (
+                    "Section content must be steps or procedures".to_string(),
+                    DiagnosticSeverity::ERROR,
+                ),
                 ParsingError::InvalidInvocation(_) => (
                     "Invalid procedure Invocation".to_string(),
                     DiagnosticSeverity::ERROR,

src/main.rs

@@ -13,7 +13,7 @@ use technique::highlighting::{self, Terminal};
 use technique::linking;
 use technique::parsing;
 use technique::resolution;
-use technique::runner::{self, Builtin, Library, Mode, Outcome, RunId};
+use technique::runner::{self, Builtin, Conclusion, Library, Mode, Outcome, RunId};
 use technique::templating::{self, Checklist, NasaEsaIss, Procedure, Recipe, Source};
 use technique::translation;
 
@@ -349,7 +349,7 @@ fn main() {
                         .default_value("interactive")
                         .action(ArgAction::Set)
                         .conflicts_with_all(["interactive", "automatic", "quiet"])
-                        .help("How to walk the procedure: interactively, prompting the operator at each step; automatically, taking each step's computed value and running to completion or first failure; or quietly, also running automatically but suppressing all progress trace output, so that only the output of external commands is printed to the terminal."),
+                        .help("How to walk the procedure: interactively, prompting the user at each step; automatically, taking each step's computed value and running to completion or first failure; or quietly, also running automatically but suppressing all progress trace output, so that only the output of external commands is printed to the terminal."),
                 )
                 .arg(
                     Arg::new("interactive")
@@ -960,14 +960,16 @@ fn main() {
             match runner::start(
                 mode, colour, filename, &program, &arguments, library, &names,
             ) {
-                Ok((run_id, Outcome::Stopped)) => {
+                Ok((run_id, Conclusion::Stopping)) => {
                     eprintln!(
                         "stopped; resume with `technique resume {}`",
                         run_id.render()
                     );
                     std::process::exit(0);
                 }
-                Ok((_, Outcome::Failed(_) | Outcome::Throw(_))) => std::process::exit(1),
+                Ok((_, Conclusion::Completed(Outcome::Fail(_)) | Conclusion::Throwing(_))) => {
+                    std::process::exit(1)
+                }
                 Ok((_, _)) => std::process::exit(0),
                 Err(error) => {
                     eprintln!("{}", problem::concise_runner_error(&error, &Terminal));
@@ -1084,7 +1086,7 @@ fn main() {
             }
 
             match runner::resume(run_id, &program, library) {
-                Ok(Outcome::Stopped) => {
+                Ok(Conclusion::Stopping) => {
                     eprintln!(
                         "stopped; continue with `technique resume {}`",
                         run_id.render()

src/parsing/parser.rs

@@ -43,6 +43,7 @@ pub enum ParsingError {
     InvalidParameters(Span),
     InvalidDeclaration(Span),
     InvalidSection(Span),
+    MixedSectionContent(Span),
     InvalidInvocation(Span),
     InvalidFunction(Span),
     InvalidCodeBlock(Span),
@@ -77,6 +78,7 @@ impl ParsingError {
             | ParsingError::InvalidParameters(span)
             | ParsingError::InvalidDeclaration(span)
             | ParsingError::InvalidSection(span)
+            | ParsingError::MixedSectionContent(span)
             | ParsingError::InvalidInvocation(span)
             | ParsingError::InvalidFunction(span)
             | ParsingError::InvalidCodeBlock(span)
@@ -1344,6 +1346,18 @@ impl<'i> Parser<'i> {
                     } else if is_step_parallel(outer.source) {
                         let step = outer.read_step_parallel()?;
                         steps.push(step);
+                    } else if begins_procedure_declaration(outer.source) {
+                        // A section that began with steps cannot then declare a
+                        // procedure; the two cannot be mixed.
+                        let line = outer
+                            .source
+                            .lines()
+                            .next()
+                            .unwrap_or("");
+                        return Err(ParsingError::MixedSectionContent(Span::new(
+                            outer.offset,
+                            line.len(),
+                        )));
                     } else {
                         // Skip unrecognized content line by line
                         outer.skip_to_next_line();
@@ -2121,43 +2135,47 @@ impl<'i> Parser<'i> {
 
     /// Parse top-level ordered step
     fn read_step_dependent(&mut self) -> Result<Scope<'i>, ParsingError> {
-        self.take_block_lines(is_step_dependent, is_step_dependent, |outer| {
-            outer.trim_whitespace();
-            let start = outer.offset;
+        self.take_block_lines(
+            is_step_dependent,
+            |line| is_step_dependent(line) || begins_procedure_declaration(line),
+            |outer| {
+                outer.trim_whitespace();
+                let start = outer.offset;
 
-            // Parse ordinal
-            let re = regex!(r"^\s*(\d+)\.\s+");
-            let cap = re
-                .captures(outer.source)
-                .ok_or(ParsingError::InvalidStep(Span::new(outer.offset, 0)))?;
+                // Parse ordinal
+                let re = regex!(r"^\s*(\d+)\.\s+");
+                let cap = re
+                    .captures(outer.source)
+                    .ok_or(ParsingError::InvalidStep(Span::new(outer.offset, 0)))?;
 
-            let number = cap
-                .get(1)
-                .ok_or(ParsingError::Expected(
-                    Span::new(outer.offset, 0),
-                    "the ordinal Step number",
-                ))?
-                .as_str();
-
-            let l = cap
-                .get(0)
-                .unwrap()
-                .len();
+                let number = cap
+                    .get(1)
+                    .ok_or(ParsingError::Expected(
+                        Span::new(outer.offset, 0),
+                        "the ordinal Step number",
+                    ))?
+                    .as_str();
 
-            outer.advance(l);
+                let l = cap
+                    .get(0)
+                    .unwrap()
+                    .len();
 
-            let text = outer.read_descriptive()?;
+                outer.advance(l);
 
-            // Parse scopes (role assignments and substeps)
-            let scopes = outer.read_scopes()?;
+                let text = outer.read_descriptive()?;
 
-            return Ok(Scope::DependentBlock {
-                ordinal: number,
-                description: text,
-                subscopes: scopes,
-                span: outer.span_since(start),
-            });
-        })
+                // Parse scopes (role assignments and substeps)
+                let scopes = outer.read_scopes()?;
+
+                return Ok(Scope::DependentBlock {
+                    ordinal: number,
+                    description: text,
+                    subscopes: scopes,
+                    span: outer.span_since(start),
+                });
+            },
+        )
     }
 
     /// Parse a top-level concurrent step
@@ -2827,7 +2845,6 @@ fn is_identifier(content: &str) -> bool {
 ///
 /// terminated by an end of line.
 
-#[allow(unused)]
 fn is_signature(content: &str) -> bool {
     let re = regex!(r"\s*.+?\s*->\s*.+?\s*$");
 
@@ -3056,6 +3073,29 @@ fn is_procedure_declaration(content: &str) -> bool {
     }
 }
 
+/// Tests whether a line begins a complete procedure declaration. Used as a
+/// boundary when reading steps: unlike the permissive is_procedure_declaration(),
+/// the text after the colon must be empty or a valid signature, so a descriptive
+/// line such as "note: be careful here" is not mistaken for a declaration.
+fn begins_procedure_declaration(content: &str) -> bool {
+    let line = content
+        .lines()
+        .next()
+        .unwrap_or("");
+
+    if !is_procedure_declaration(line) {
+        return false;
+    }
+
+    match line.split_once(':') {
+        Some((_, after)) => {
+            let after = after.trim_ascii();
+            after.is_empty() || is_signature(after)
+        }
+        None => false,
+    }
+}
+
 /// Detects any line that could potentially be a procedure declaration,
 /// including malformed ones. Used for detecting the end-boundary of a
 /// procedure.

src/problem/messages.rs

@@ -472,6 +472,37 @@ author of the Technique.
                 .to_string(),
             )
         }
+        ParsingError::MixedSectionContent(_) => (
+            "Section mixes steps and procedures".to_string(),
+            r#"
+A section contains either steps or procedures, but not both. A section that
+begins with steps cannot then declare a procedure:
+
+    I. Mix Drink
+
+        1.  Take the juice from one bottle
+        2.  Pour in one measure of water
+
+    prepare_mega_gin :
+
+Writing steps directly under a heading is a convenient shorthand and perfectly
+valid Technique, but to add a helper procedure alongside them you will need to
+upgrade to the main part of the section being a named procedure, and then you
+can write a helper procedure that follows
+
+    I. Mix Drink
+
+    mix_drink :
+
+        1.  Take the juice from one bottle
+        2.  Pour in one measure of water
+        3.  Allow three cubes to melt <prepare_mega_gin>
+
+    prepare_mega_gin :
+                "#
+            .trim_ascii()
+            .to_string(),
+        ),
         ParsingError::InvalidInvocation(_) => {
             let examples = vec![
                 Invocation {