Make MDL syntax errors point at the fix, not just the location

mdl/visitor/visitor.go

@@ -5,6 +5,7 @@ package visitor
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/antlr4-go/antlr/v4"
@@ -16,6 +17,7 @@ import (
 type errorListener struct {
 	*antlr.DefaultErrorListener
 	errors []error
+	source []string // input split into lines, for source-aware hints
 }
 
 func newErrorListener() *errorListener {
@@ -27,14 +29,67 @@ func newErrorListener() *errorListener {
 
 // SyntaxError is called by ANTLR when a syntax error is encountered.
 func (l *errorListener) SyntaxError(_ antlr.Recognizer, _ any, line, column int, msg string, _ antlr.RecognitionException) {
-	// Check if the error is about a reserved keyword being used as an identifier
-	enhancedMsg := enhanceErrorMessage(msg)
+	offending := ""
+	if line >= 1 && line <= len(l.source) {
+		offending = l.source[line-1]
+	}
+	enhancedMsg := enhanceErrorMessage(msg, offending)
 	l.errors = append(l.errors, fmt.Errorf("line %d:%d %s", line, column, enhancedMsg))
 }
 
-// enhanceErrorMessage checks if an error message indicates a reserved keyword
-// was used as an identifier and provides a more helpful message.
-func enhanceErrorMessage(msg string) string {
+// simplifyExpecting collapses ANTLR's raw `expecting {A, B, …}` token-set dumps,
+// which are precise but noisy: a failed statement start lists ~30 internal token
+// names (`{<EOF>, DOC_COMMENT, CREATE, ALTER, …}`), drowning the actual signal.
+// The all-statement-keywords set is rewritten to a human phrase; any other
+// oversized set is truncated. A small set (the useful case, e.g. `expecting ':'`)
+// is left untouched.
+func simplifyExpecting(msg string) string {
+	const marker = "expecting {"
+	i := strings.Index(msg, marker)
+	if i < 0 {
+		return msg
+	}
+	rel := strings.Index(msg[i:], "}")
+	if rel < 0 {
+		return msg
+	}
+	inner := msg[i+len(marker) : i+rel]
+	toks := strings.Split(inner, ", ")
+	set := make(map[string]bool, len(toks))
+	for _, t := range toks {
+		set[t] = true
+	}
+	rest := msg[i+rel+1:]
+	// The top-level statement-start set — the "this isn't a valid statement"
+	// case, usually a misspelled verb or an unterminated previous statement.
+	if set["CREATE"] && set["ALTER"] && set["DROP"] {
+		return msg[:i] + "expecting the start of a statement (create, alter, drop, show, describe, …)" + rest
+	}
+	if len(toks) > 6 {
+		return msg[:i] + "expecting one of {" + strings.Join(toks[:6], ", ") + ", …}" + rest
+	}
+	return msg
+}
+
+// enhanceErrorMessage turns a raw ANTLR syntax error into something that points
+// at the fix: it collapses noisy token-set dumps, recognises a handful of common
+// mistakes (adding a correct-vs-wrong example), and otherwise appends a pointer
+// to `mxcli syntax`. offendingLine is the source line the error occurred on (""
+// when unavailable), used for source-aware hints.
+func enhanceErrorMessage(msg, offendingLine string) string {
+	// Collapse ANTLR's oversized `expecting {…}` token dumps first, so every
+	// branch below (and the fall-through) shows the tamer form.
+	msg = simplifyExpecting(msg)
+
+	// A bare `not $x` — Mendix requires `not(expr)`. The parse error surfaces
+	// downstream (e.g. "missing THEN at '$x'"), so key off the source line, which
+	// is unambiguous for `not $…`. (sudoku findings #3)
+	if bareNotRe.MatchString(offendingLine) {
+		return fmt.Sprintf("%s\n\n  Mendix requires parentheses around a negated expression — a bare\n"+
+			"  'not <expr>' does not parse:\n"+
+			"    if not($Cell/IsInvalid) then …   (correct)\n"+
+			"    if not $Cell/IsInvalid then …    (wrong — causes parse error)", msg)
+	}
 	// Check for quoted attribute names after READ/WRITE in a GRANT clause.
 	// Users often write `READ "Attr1", "Attr2"` instead of the correct
 	// `READ (Attr1, Attr2)` — the grammar expects unquoted identifiers in parens.
@@ -116,9 +171,17 @@ func enhanceErrorMessage(msg string) string {
 		}
 	}
 
-	return msg
+	// Nothing matched a specific pattern — the location is precise but the fix
+	// isn't spelled out. Point at the syntax reference so the correct form is one
+	// command away. (Kept to one line since it can repeat across cascading errors.)
+	return msg + "  [see: mxcli syntax <topic>, e.g. entity | microflow | page]"
 }
 
+// bareNotRe matches a bare `not $…` (not followed by `(`) on a source line — the
+// exact shape of the unparenthesized-negation mistake. Scoped to `not $var` to
+// stay false-positive-free (it won't fire on `not(...)`, `is not null`, etc.).
+var bareNotRe = regexp.MustCompile(`(?i)\bnot\s+\$`)
+
 // looksLikeMisplacedExtends detects ANTLR errors caused by an EXTENDS /
 // GENERALIZATION clause placed after the entity's attribute parentheses instead
 // of before them. The offending token surfaces as extraneous/mismatched input.
@@ -264,6 +327,7 @@ func collectLeafTokens(tree antlr.Tree, tokens *[]string) {
 func Build(input string) (*ast.Program, []error) {
 	// Create custom error listener to capture syntax errors
 	errListener := newErrorListener()
+	errListener.source = strings.Split(input, "\n")
 
 	// Create lexer with custom error listener
 	is := antlr.NewInputStream(input)

mdl/visitor/visitor_test.go

@@ -1228,8 +1228,8 @@ func TestEnhanceErrorMessage_Apostrophe(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := enhanceErrorMessage(tt.msg)
-			hasHint := result != tt.msg
+			result := enhanceErrorMessage(tt.msg, "")
+			hasHint := strings.Contains(result, "apostrophe")
 			if hasHint != tt.wantHint {
 				if tt.wantHint {
 					t.Errorf("expected apostrophe hint but got none.\n  input:  %s\n  output: %s", tt.msg, result)
@@ -1306,7 +1306,7 @@ func TestEnhanceErrorMessage_QuotedGrantAttribute(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := enhanceErrorMessage(tt.msg)
+			result := enhanceErrorMessage(tt.msg, "")
 			hasHint := strings.Contains(result, "Attribute-level GRANT")
 			if hasHint != tt.wantHint {
 				t.Errorf("expected hint=%v\n  input:  %s\n  output: %s", tt.wantHint, tt.msg, result)
@@ -1327,7 +1327,7 @@ func TestEnhanceErrorMessage_EnumEquals(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := enhanceErrorMessage(tt.msg)
+			result := enhanceErrorMessage(tt.msg, "")
 			hasHint := strings.Contains(result, "Enumeration values do not use '='")
 			if hasHint != tt.wantHint {
 				t.Errorf("expected hint=%v\n  input:  %s\n  output: %s", tt.wantHint, tt.msg, result)
@@ -2281,3 +2281,53 @@ func TestCreateOrReplaceEnumeration_FlagsCreateOrModify(t *testing.T) {
 		t.Error("CREATE OR REPLACE should set CreateOrModify=true so the executor takes the update path")
 	}
 }
+
+// TestEnhanceErrorMessage_CollapsesTokenDump verifies the noisy statement-start
+// `expecting {30 tokens}` set is rewritten to a readable phrase (findings: error
+// messages should convey the fix, not dump internal token names).
+func TestEnhanceErrorMessage_CollapsesTokenDump(t *testing.T) {
+	raw := "extraneous input '(' expecting {<EOF>, DOC_COMMENT, CREATE, ALTER, DROP, SHOW, DESCRIBE, IDENTIFIER}"
+	out := enhanceErrorMessage(raw, "")
+	if strings.Contains(out, "DOC_COMMENT") || strings.Contains(out, "IDENTIFIER") {
+		t.Errorf("token dump not collapsed:\n%s", out)
+	}
+	if !strings.Contains(out, "start of a statement") {
+		t.Errorf("expected the statement-start phrase, got:\n%s", out)
+	}
+}
+
+// TestEnhanceErrorMessage_SmallExpectingSetUntouched keeps the useful single/small
+// expected-token messages intact.
+func TestEnhanceErrorMessage_SmallExpectingSetUntouched(t *testing.T) {
+	raw := "mismatched input '=' expecting ':'"
+	out := enhanceErrorMessage(raw, "")
+	if !strings.Contains(out, "expecting ':'") {
+		t.Errorf("small expecting set should be preserved, got:\n%s", out)
+	}
+}
+
+// TestEnhanceErrorMessage_BareNotHint fires the not(expr) hint from the source
+// line (sudoku findings #3), and not otherwise.
+func TestEnhanceErrorMessage_BareNotHint(t *testing.T) {
+	const notHint = "negated expression"
+	out := enhanceErrorMessage("mismatched input '$x' expecting THEN", "  if not $Cell/IsInvalid then")
+	if !strings.Contains(out, notHint) || !strings.Contains(out, "not($Cell") {
+		t.Errorf("expected the bare-not hint, got:\n%s", out)
+	}
+	// A properly parenthesized not must not trigger it.
+	if strings.Contains(enhanceErrorMessage("mismatched input 'foo' expecting ';'", "  if not($Cell/Flag) then"), notHint) {
+		t.Error("not(expr) should not trigger the bare-not hint")
+	}
+}
+
+// TestBuild_BareNotEndToEnd confirms the hint reaches the surface through Build.
+func TestBuild_BareNotEndToEnd(t *testing.T) {
+	_, errs := Build("create microflow M.T() begin if not $x then log info 'a'; end if; return true; end")
+	joined := ""
+	for _, e := range errs {
+		joined += e.Error() + "\n"
+	}
+	if !strings.Contains(joined, "not(") {
+		t.Errorf("expected a not(expr) hint in Build errors, got:\n%s", joined)
+	}
+}