From 6467d294ac5750d7ee060f5c4e0291275ece238d Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@cs.stanford.edu>
Date: Tue, 13 Apr 2021 09:58:46 -0700
Subject: [PATCH] segment: Improve segmentation when DEFINE command has early
 terminator.

The DEFINE command should not have a command terminator before the final
!ENDDEFINE.  When it does, that ends the whole thing early.  The segmenter
didn't handle this well, instead treating everything up to !ENDDEFINE as
part of the command.  That's probably not the best way because it makes
for confusing error messages.  This improves the behavior.
---
 src/language/lexer/segment.c    | 10 +++++++++-
 tests/language/lexer/scan.at    | 32 ++++++++++++++++++++++++++++++--
 tests/language/lexer/segment.at | 30 ++++++++++++++++++++++++++++--
 3 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/language/lexer/segment.c b/src/language/lexer/segment.c
index 43e02ac6fb..d2aa391748 100644
--- a/src/language/lexer/segment.c
+++ b/src/language/lexer/segment.c
@@ -1516,7 +1516,15 @@ segmenter_parse_define_3__ (struct segmenter *s,
   if (ofs < 0)
     return -1;
 
-  if (*type == SEG_NEWLINE)
+  if (*type == SEG_END_COMMAND)
+    {
+      /* The DEFINE command is malformed because there was a command terminator
+         before the first line of the body.  Transition back to general
+         parsing. */
+      s->state = S_GENERAL;
+      return ofs;
+    }
+  else if (*type == SEG_NEWLINE)
     s->state = S_DEFINE_4;
 
   return ofs;
diff --git a/tests/language/lexer/scan.at b/tests/language/lexer/scan.at
index 953be86deb..6df0caae9e 100644
--- a/tests/language/lexer/scan.at
+++ b/tests/language/lexer/scan.at
@@ -1129,10 +1129,39 @@ STOP
 PSPP_CHECK_SCAN([-i])
 AT_CLEANUP
 
+AT_SETUP([DEFINE command - early end of command 4])
+AT_KEYWORDS([segment])
+AT_DATA([input], [dnl
+dnl Notice the command terminator at the end of the DEFINE command,
+dnl which should not be there and ends it early.
+define !macro1.
+data list /x 1.
+])
+AT_DATA([expout-base], [dnl
+ID "define"
+SKIP
+MACRO_ID "!macro1"
+ENDCMD
+SKIP
+ID "data"
+SKIP
+ID "list"
+SKIP
+SLASH
+ID "x"
+SKIP
+POS_NUM 1
+ENDCMD
+-SKIP
+STOP
+])
+PSPP_CHECK_SCAN([-i])
+AT_CLEANUP
+
 AT_SETUP([DEFINE command - missing !ENDDEFINE])
 AT_KEYWORDS([scan])
 AT_DATA([input], [dnl
-define !macro1().
+define !macro1()
 content line 1
 content line 2
 ])
@@ -1142,7 +1171,6 @@ SKIP
 MACRO_ID "!macro1"
 LPAREN
 RPAREN
-ENDCMD
 SKIP
 STRING "content line 1"
 SKIP
diff --git a/tests/language/lexer/segment.at b/tests/language/lexer/segment.at
index 54520717c7..8f82dd2e6e 100644
--- a/tests/language/lexer/segment.at
+++ b/tests/language/lexer/segment.at
@@ -1295,6 +1295,33 @@ identifier      x
 end_command     .
 newline         \n (first)
 
+identifier      data    space
+identifier      list    space
+punct           /
+identifier      x    space
+number          1
+end_command     .
+-newline         \n (first)
+-
+end
+])
+PSPP_CHECK_SEGMENT([-i])
+AT_CLEANUP
+
+AT_SETUP([DEFINE command - early end of command 4])
+AT_KEYWORDS([segment])
+AT_DATA([input], [dnl
+dnl Notice the command terminator at the end of the DEFINE command,
+dnl which should not be there and ends it early.
+define !macro1.
+data list /x 1.
+])
+AT_DATA([expout-base], [dnl
+identifier      define    space
+macro_id        !macro1
+end_command     .
+newline         \n (first)
+
 identifier      data    space
 identifier      list    space
 punct           /
@@ -1311,7 +1338,7 @@ AT_CLEANUP
 AT_SETUP([DEFINE command - missing !ENDDEFINE])
 AT_KEYWORDS([segment])
 AT_DATA([input], [dnl
-define !macro1().
+define !macro1()
 content line 1
 content line 2
 ])
@@ -1320,7 +1347,6 @@ identifier      define    space
 macro_id        !macro1
 punct           (
 punct           )
-end_command     .
 newline         \n (DEFINE)
 
 macro_body      content_line_1
-- 
2.30.2