diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index ee17cd08d..220b96a62 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -47,6 +47,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Studio Pro renders **every activity/decision as a 1-px sliver** (caption wraps one letter per line) after a modelsdk round-trip; `mx check` reports **NO** error (it ignores box size, so the corruption is silent) | `flowObjectFromGen` carried each object's `Position` but not its `Size`, so the round-trip rewrote every node with size `0;0` | `mdl/backend/modelsdk/microflow.go` (`sizeFromGen`, `splitFlowObjects`) | Add `sizeFromGen` (mirrors `pointFromGen`) and apply it at the `splitFlowObjects` call site — covers nested loop bodies via recursion; add `GetSize`/`SetSize` to `BaseMicroflowObject`. Go round-trip test (not MDL, same reason as A1). Issue #723 A2 | | `mx check` **CE0117** "Error in expression" when creating a rule-based decision (`if Module.SomeRule(...)`) via MDL on the **modelsdk** engine; the decision's subtype is demoted Rule→Expression on every round-trip | modelsdk `*Backend` never implemented `IsRule` → fell back to the embedded `unimplemented` stub (which errors); the builder's `isRule, err := backend.IsRule(...); if err != nil || !isRule` guard treated the error as "not a rule" and emitted an invalid `ExpressionSplitCondition` | `mdl/backend/modelsdk/microflow.go` (`IsRule`) | Implement `IsRule` on `*Backend` (list `Microflows$Rule` units, match qualified name via `moduleNameFor`), mirroring the legacy reader; it shadows the generated stub. With the existing builder mock-test this guards the full chain modelsdk.IsRule→RuleSplitCondition. Issue #723 A4 | | `create enumeration … ("Value" = 'Caption')` fails with cryptic `mismatched input '=' expecting ')'`; user blames the *quotes* | Not a quoting bug — enum value names quote fine (`enumValueName` accepts `QUOTED_IDENTIFIER`). The `=` is invalid: MDL enum values are `Value 'Caption'` (or `Value caption 'Caption'`), no equals sign | `mdl/visitor/visitor.go` (`enhanceErrorMessage`/`looksLikeEnumEquals`) | Add an error hint keyed on `mismatched input '=' expecting ')'` (specific to enum value lists — attribute-default `=` gives a different message) pointing at the `=`. Clarify in `check-syntax.md` that captions use `Name 'Caption'`, never `=` | +| A genuine parse error on a short lowercase MDL keyword/identifier (`mismatched input 'on'`, `'in'`, `'as'`, `'to'`, `'by'`, …) is misdiagnosed with the "unescaped apostrophe" hint, sending the user to look for a quote that isn't there | `looksLikeUnescapedApostrophe` matched **any** 1–4 char lowercase token as a contraction leftover, so real short keywords tripped it | `mdl/visitor/visitor.go` (`looksLikeUnescapedApostrophe`, `contractionSuffixes`) | Match only the fixed contraction-suffix set (`s`/`t`/`d`/`m`/`re`/`ve`/`ll` — the leftovers from it's/don't/he'd/I'm/you're/we've/you'll), not arbitrary short lowercase words. Real apostrophe errors still hint; `on`/`in`/`as`/`to`/`by` no longer do. Findings #4 | | Docs/skills document MDL that doesn't parse (drift): `ALTER ENTITY X ADD (a, b)` / `DROP (a)` / `MODIFY (a)` / `RENAME a TO b`, `ALTER ENUMERATION … REMOVE VALUE`, `CREATE CONSTANT … TYPE X;` (no default), `DELETE_BEHAVIOR ` | Docs were written against a SQL-DDL mental model, never `mxcli check`-validated. Correct forms: `ADD ATTRIBUTE a: type` (one action/statement), `DROP ATTRIBUTE a`, `MODIFY ATTRIBUTE a: type`, `RENAME ATTRIBUTE a TO b`, `DROP INDEX `; `DROP VALUE`; enum `ADD VALUE X CAPTION 'y'` (CAPTION keyword required for ALTER, unlike CREATE); a constant `DEFAULT` is mandatory; delete-behavior ∈ {DELETE_AND_REFERENCES, DELETE_BUT_KEEP_REFERENCES, DELETE_IF_NO_REFERENCES, CASCADE, PREVENT} | `scripts/check-skill-mdl.sh` + `make check-skill-mdl` (CI) | The guard extracts DDL statements from `.claude/skills/mendix/` **and** `docs-site/src/` and runs `mxcli check`; keeps this class of drift from recurring. Run it after editing any MDL example | | `DESCRIBE microflow` on the **modelsdk** engine renders `download file $X …;` as `-- Empty action` | `actionFromGen` lacked a `DownloadFileAction` case | `mdl/backend/modelsdk/microflow_read_actions.go` (`actionFromGen`) | Add the case reading `FileDocumentVariableName()`/`ShowFileInBrowser()`, defaulting an empty `ErrorHandlingType` to Rollback. Note: the storage key is `ShowFileInBrowser` (legacy's `parseDownloadFileAction` reads the wrong `ShowInBrowser` key — a latent legacy bug; the gen reads it correctly) | | `DESCRIBE microflow` on the **modelsdk** engine renders a legacy SOAP `call web service …` as `-- Empty action` | `actionFromGen` lacked a `WebServiceCallAction` case | `mdl/backend/modelsdk/microflow_read_actions.go` (`actionFromGen`, `webServiceActionRequiresRawBSON`) | Add the case: read the structured fields (ImportedService / OperationName / NewResultHandling / RequestHandling) and, when the action carries any field the structured form can't represent, set `RawBSON = a.Raw()` so the renderer emits `call web service raw ''`. Mirror legacy's supported-key set exactly; `canonicalRawBSON` makes both engines' base64 byte-identical | diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index 731434003..836688f6a 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -218,11 +218,23 @@ func looksLikeQuotedGrantAttribute(msg string) bool { return false } +// contractionSuffixes are the token fragments ANTLR is left holding after an +// unescaped apostrophe splits an English contraction: 'don't' → string 'don' + +// leftover t; 'it's' → s; 'you'll' → ll; 'you're' → re; 'we've' → ve; 'he'd' → +// d; 'I'm' → m. Matching this fixed set (rather than "any short lowercase word") +// is what keeps the apostrophe hint from firing on real MDL keywords/identifiers +// that happen to be short and lowercase — e.g. a misplaced `on`, `in`, `as`, +// `to`, `by` produced the wrong "unescaped apostrophe" advice before (findings #4). +var contractionSuffixes = map[string]bool{ + "s": true, "t": true, "d": true, "m": true, + "re": true, "ve": true, "ll": true, +} + // looksLikeUnescapedApostrophe detects ANTLR errors that are likely caused by // unescaped apostrophes in string literals. When 'don't' is parsed, ANTLR sees // 'don' as a complete string, then 't' as an unexpected token, producing errors // like: missing END at 's', mismatched input 't', or token recognition error at: ”; -// We detect short (1-4 char) lowercase word fragments and unbalanced quote errors. +// We detect the specific contraction-suffix fragments and unbalanced quote errors. func looksLikeUnescapedApostrophe(msg string) bool { // Pattern 1: "token recognition error at: ''" — unbalanced trailing quote if strings.Contains(msg, "token recognition error at: ''") { @@ -265,26 +277,16 @@ func looksLikeUnescapedApostrophe(msg string) bool { token = msg[searchFrom : searchFrom+tokenEnd] } - // Short lowercase word fragments are likely apostrophe artifacts - // e.g., "s" from "it's", "ll" from "you'll", "t" from "don't", - // "re" from "you're", "ve" from "we've", "d" from "he'd" - if len(token) >= 1 && len(token) <= 4 && isLowerAlpha(token) { + // Only the specific contraction-suffix fragments are apostrophe artifacts. + // A real MDL keyword/identifier (on, in, as, to, by, …) is short and + // lowercase too, but is NOT a contraction leftover, so it must not match. + if contractionSuffixes[token] { return true } } return false } -// isLowerAlpha returns true if s consists entirely of lowercase ASCII letters. -func isLowerAlpha(s string) bool { - for i := 0; i < len(s); i++ { - if s[i] < 'a' || s[i] > 'z' { - return false - } - } - return true -} - // Builder walks the ANTLR parse tree and builds AST nodes. type Builder struct { *parser.BaseMDLParserListener diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 6fed7d349..2ea1f129a 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1224,6 +1224,33 @@ func TestEnhanceErrorMessage_Apostrophe(t *testing.T) { msg: "no viable alternative at input 'CREATE PERSISTENT'", wantHint: false, }, + // findings #4: real short lowercase MDL keywords/identifiers are NOT + // contraction leftovers and must not draw the apostrophe hint. + { + name: "keyword on is not apostrophe", + msg: "mismatched input 'on' expecting {';', ','}", + wantHint: false, + }, + { + name: "keyword in is not apostrophe", + msg: "extraneous input 'in' expecting {';', ','}", + wantHint: false, + }, + { + name: "keyword as is not apostrophe", + msg: "mismatched input 'as' expecting {';', ','}", + wantHint: false, + }, + { + name: "keyword to is not apostrophe", + msg: "missing ';' at 'to'", + wantHint: false, + }, + { + name: "keyword by is not apostrophe", + msg: "mismatched input 'by' expecting {';', ','}", + wantHint: false, + }, } for _, tt := range tests {