Expression type checking that actually checks, plus check/write-path fixes from field reports - #929
Merged
Conversation
…columns Three defects around DataGrid 2 columns in ALTER PAGE, found investigating mendixlabs#919. Verified on Mendix 11.13.0. **1. The alias tables had drifted (the reported bug).** `ALTER PAGE SET DynamicCellClass ON grid.Column` failed with "column property not found" for a property CREATE PAGE writes happily, and `check --references` passed the statement `exec` rejected. Two hand-written tables described one widget schema: the create path aliased DynamicCellClass → columnClass, the ALTER path kept its own copy that did not — mutator.go contained zero occurrences of the string. The second table is now deleted rather than corrected. The MDL name resolves against the schema keys the document itself declares, case insensitively, falling back to one shared alias table (moved to mdl/types) for genuine renames. A property the create path can write is one ALTER can write, by construction. Case-only differences need no entry, which keeps the shared table to real aliases. A miss now lists the settable keys read off that grid, so it is right for the widget version installed rather than the one mxcli was built against. **2. Every value was written to the wrong field, silently.** setColumnPropertyMut always wrote PrimitiveValue. columnClass and visible are Expression-valued, so `SET DynamicCellClass` and `SET Visible` reported success, put the value in a field Mendix does not read, did not survive a DESCRIBE round trip, and left mx check at 0 errors. That is worse than the loud failure it was hiding behind. The shape cannot be inferred from the stored document — a WidgetValue carries Expression, PrimitiveValue, TextTemplate and AttributeRef all at once, with the unused ones empty — so it now comes from the schema's declared ValueType.Type. Structured kinds (Attribute, DataSource, Action, Widgets) are refused with a pointer to CREATE OR REPLACE PAGE rather than written as a plausible-looking string. Measured: before, the write was invisible to DESCRIBE and mxbuild said 0 errors. After, it round-trips and mxbuild *validates* it — a valid expression builds at 0 errors, a bare identifier is CE0117, matching what CREATE produces for the same input. **3. A column name written in MDL is discarded, undiscoverably.** Mendix stores no name on a pluggable DataGrid 2 column: the schema has no name key and DataGridColumnSpec has no field for one. So `column colLabel (attribute: Label, …)` drops "colLabel", and everything downstream addresses the column by a derived name — bound attribute, else sanitized caption, else colN. The author meets this as `ON dg1.colLabel` → "column not found" on a column they just named. mxcli check now reports MDL-WIDGET16 at authoring time, naming the addressable name. It warns rather than rejects: the name reads as documentation and rejecting it would break every existing script, including mxcli's own doctype tests. It stays silent when the derivation depends on position (colN), because naming the wrong one is worse than naming none. Also documented, because it bites both paths equally and neither says so: Expression-valued column properties take a Mendix expression, so a literal CSS class needs doubled quotes — `SET DynamicCellClass = '''highlight'''`. A bare identifier is CE0117 on create and on alter alike. Controls: stubbing the resolver back to a hardcoded table reproduces "column property \"DynamicCellClass\" not found" verbatim; stubbing the value-kind branch back to always-PrimitiveValue puts the value in the wrong field again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CqQLyxnppZGfqPYbUHvSWG
fix(alter-page): one alias table, schema-shaped column writes, named columns
…WIDGET16
Four fixes from mxcli-dbreplication's findings. Independent concerns, one
commit because they are one round of feedback; each is separable.
**1. `check -p` resolved nothing (F5).**
`mxcli check script.mdl -p app.mpr` printed an unqualified "Check passed!"
having resolved no references at all — reference validation was gated on
`--references` as well. So a misspelled icon, entity or page name sailed
through a command that had been handed the model.
Reported as "check does not validate icon names", which it does:
validate_icon_refs.go names the offending icon and the `describe icon
collection` command to list the real ones. It returns early when not
connected, so it simply never ran.
`-p` now implies reference resolution — someone who hands the command a
project has said what they want — and `--references` stays accepted so
existing invocations survive. A run *without* a project qualifies its
verdict by naming what was not resolved, so a pass is never read as more
than it is.
**2. Two rules that only real validation caught (F10).**
Four scripts passed `mxcli check` with 0 errors, executed cleanly, and
`mx check` then reported CE0156 and CE5601. Both are decidable from the
MDL alone:
MDL-SEC20 a user role with no System module role — nobody holding it
can sign in or read System entities
MDL-PAGE20 a page with parameters and a Url lacking a {Name} segment,
which Mendix needs to bind each parameter
The URL matcher is the subtle half. Mendix binds a parameter by an
attribute path, so `url: 'p006/{Customer/Name}'` is the ordinary shape and
an exact `{Customer}` match would flag correct pages. It matches the
segment's leading identifier instead — verified at 0 false positives
against mxcli's own page examples, which is the control that matters for a
new rule.
**3. Marketplace search missed the name people use (F7).**
`marketplace search 'Database Replication'` returned No results for a
module that is right there; `search replication` found it. filterItems
matched the packaged name (`DatabaseReplication`) verbatim, and packaged
names have no spaces. Content carries no display-name field, so both sides
are now normalised — case folded, separators dropped — and the written
name meets the packaged one in the middle.
**4. MDL-WIDGET16 said the same thing 44 times (F6).**
Feedback on a rule shipped days earlier, from the project using it. It was
written per column when the fact — this grid stores no column names —
belongs to the grid. Now one violation per grid listing each
`written → addressable` mapping. An advisory rule's unit should be the
thing the advice is about, not the thing that triggered it; N identical
infos train the reader to skip the category.
Also adds PROPOSAL_bootstrap_source.md for the fifth finding: `init`
hard-codes the mendixlabs nightly URL, so a fork-pinned repo silently
returns on a different binary after a reap. Proposes two bootstrap
prompts — a user one that resolves the release source, and an opt-in
developer one that clones and builds — rather than one script with a
hidden mode. Open questions recorded rather than guessed.
Controls: stubbing the two new rules' wiring puts them back to silent;
`check -p` without the implication resolves nothing again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CqQLyxnppZGfqPYbUHvSWG
`calculated by Module.Microflow` parsed, validated and reported success
while the binding never reached the model. attributeToGen in the
modelsdk writer had arms for OqlViewValue and the two OData mapped
values, plus a default that emits StoredValue — and no CalculatedValue
arm — so the value the executor had already resolved fell through and
was discarded. The attribute was stored as an ordinary stored value with
no link to the microflow, mx check reported 0 errors, and the attribute
was simply empty at runtime.
The legacy writer had the arm all along, which is why the feature read
as implemented; modelsdk is the default engine, so every default
invocation hit the broken path.
The reader had no CalculatedValue case either. That is the worse half:
an unrelated ALTER on the same entity read the attribute back as a plain
value and wrote it out as one, destroying a binding made in Studio Pro.
Both directions are fixed.
PassEntity is derived from the microflow's signature rather than
hardcoded (legacy always wrote true). Measured on 11.13.0, both shapes
build at 0 errors: a microflow taking the owning entity stores
PassEntity=true, a parameterless one stores false — so refusing the
parameterless form would have been wrong.
The signature itself is now checked before the write, at the same place
as the other write-blocking rules, since a script can skip check. Each
rule was put to mxbuild rather than assumed:
wrong entity parameter -> CE7247 "Microflow parameter 'Other' should
be of type MyFirstModule.Order."
wrong return type -> CE7247 "Microflow return type should be
Integer/Long."
That second message is why the return-type rule is not a strict
equality: Integer and Long are one family, and a Long-returning
microflow on an Integer attribute builds clean. A strict check refused
valid MDL until the CE text was read properly.
A microflow created earlier in the same script cannot be inspected yet,
so its signature is left to the build rather than refused on a guess.
The backend tests fail with the reported symptom when reverted
(value is *domainmodels.StoredValue).
`make lint` runs `go fmt ./...`, so anyone who ran it picked up unrelated reformatting of three files and had to strip it out of their diff by hand. That happened on four consecutive PRs in this branch's history, each time adding noise to a review that had nothing to do with it. All three are pure alignment gofmt wanted after a longer field name landed next to shorter ones — `git diff -w` on this commit is empty, so nothing here changes behaviour. The other half of the churn is already gone: cmd/mxcli/lsp_completions_gen.go was stale against MDLLexer.g4 a few days ago (it carried NOTEBOOK tokens the grammar no longer defines) and regenerating it now produces a byte-identical file, so there is nothing to commit for it. Checked rather than assumed — `go run ./cmd/gen-completions` against the current grammar, diffed. After this, `make lint` on a clean tree leaves it clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CqQLyxnppZGfqPYbUHvSWG
mdl/exprcheck ships a complete expression type checker whose semantic rules —
enum-value comparisons, attribute and operand type mismatches, function argument
types — all run through its CatalogReader seam. Nothing implemented that seam,
so every real invocation ran with a nil Catalog and every semantic rule was
silently skipped. The checker was in the tree and checked nothing; only the
narrow hand-rolled entry points (InferSourceKind, UnknownFunctionCalls) did any
work.
Three of the five lookups had no data behind them either, so this is two changes
in one place.
Catalog (schema 10, so cached catalogs regenerate — without the bump a stale
cache answers "unknown" for every lookup, which the checker reads as "cannot
tell" and skips, i.e. a green run that checked nothing):
- attributes_data.EnumerationQualifiedName. DataType comes from GetTypeName and
is the bare kind, so an enumeration attribute reported only "Enumeration" and
lost which enumeration. Kept as a new column rather than folded into DataType
as "Enumeration:QN", because existing queries and lint rules match DataType by
equality.
- enumeration_values_data. The table stored ValueCount but not the values, so
nothing could answer "is 'Open' a case of this enum" — the check behind the
most common expression bug.
- microflow_parameters_data, for microflows and nanoflows alike. Likewise
ParameterCount without the parameters. ParameterType reuses the ReturnType
encoding ("Object:Mod.Entity", "Enumeration:Mod.Enum", …).
mdl/exprcatalog implements all five methods over that data as an index loaded in
four queries, not SQL per lookup: a project has thousands of expressions each
asking several questions.
Two behaviours worth stating because they are choices, not accidents:
- Anything unanswerable returns (zero, false) → KindUnknown → the rule is
suppressed. A stale or partial catalog makes the checker catch less, never
false-positive on valid code.
- A Void return reports not-found rather than being mapped to KindEmpty.
Calling a void microflow in a value position is a real error, but inventing a
type for it here would diagnose it in the wrong place.
Known blind spot, verified rather than assumed: the System module contributes no
enumerations at all (`show enumerations in System` is empty on a stock 11.13
app) because they are platform metadata, not stored units. An attribute typed by
System.WorkflowEventType resolves its enum name but not its cases, so the
enum-value rule is skipped for it. Pre-existing, documented in the package.
Verified against a real 11.13.0 project, not only seeded rows: attribute kinds,
the attribute → enum → cases chain in model order, microflow and nanoflow return
kinds, and parameters by name with or without the $ sigil. Each new test was
shown to fail with the defect it pins (index handed out by reference, attributes
keyed without their entity, Void given a kind).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
Struct-field and trailing-comment alignment that gofmt rewrites. All three came in with the loop-sizing, MPR011 and StartEvent changes; `gofmt -l` on mdl/ and cmd/ is clean afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
…ow example Both microflows in it declared $msg and then assigned the output of a call microflow to the same name, which is CE0111 "Duplicate variable name" -- the call activity creates its own output variable, so the declare collides with it. The example is about reference validation skipping excluded documents; the declare was incidental to that and simply wrong. Measured on mxbuild 11.6.6 with the microflows asserted present before reading the error count (an exec that aborts writes nothing, and the untouched baseline then reads as a pass): as shipped, 2 microflows written and CE0111 alongside the app's baseline CE0079; with the declare dropped, 2 microflows written and the baseline alone. The excluded twin is exempt from checking either way -- measured, mxbuild reports nothing for it -- but it carried the same mistake and no longer does. Found by running the new mendixlabs#893 rules over mdl-examples before wiring them up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
…mendixlabs#893) Three constructs from upstream mendixlabs#893 that reported "Syntax OK", were written by exec, and surfaced only when a human opened Studio Pro's Errors pane. Reproduced on mxbuild 11.6.6 against an app whose baseline is 1 error; the issue's own script took it to 4: MDL061 declare with no value CE0038 a Create Variable needs a value MDL062 return inside a loop CE0068 no End event inside a loop MDL063 duplicate variable name CE0111 the activity creates its own Each suggested fix was measured back to the 1-error baseline before being put in the message: `= ''`, `break`, and dropping the declare. Error severity alone closes the gap -- exec pre-flights the whole script and refuses with nothing written, verified end to end on the reproduction (3 reported, 0 microflows written). They are deliberately kept out of execEnforcedMicroflowRules so --no-check still applies; the reason is below. MDL063 implements the rule mxbuild actually enforces rather than the reported shape alone. A microflow's variable namespace is FLAT: parameters, loop iterators and every activity output share it, and neither a branch nor a loop body opens a scope. All seven combinations were measured one microflow at a time. Assignment to an existing variable stays clean, because `set` is a Change Variable and creates nothing -- a rule keyed on "appears left of `=`" would have flagged the normal idiom. The rules read the AST but the outcome depends on what the BUILDER emits, so running them over mdl-examples/ before wiring them up was the load-bearing step: 4 of 374 files hit, and 3 were false positives. - `while true` builds an ExclusiveMerge back-edge, not a LoopedActivity (mendixlabs#350), so there is no loop for the End event to be inside. A plain `while <cond>` IS a loop object and is not exempt -- measured separately. - `returns T as $Var` makes the builder synthesize the End event from the variable and none lands in the loop. That shape is broken in a different way (CE0109), which this rule must not mislabel. - `set $x = contains($str, $str)` parses as a ListOperationStmt that addListOperationAction rewrites into a Change Variable precisely to avoid this CE0111 (ledger #53/#63). The operation test is now a shared predicate, stringOverloadedListOp, so rule and builder cannot drift. - an @excluded document is never checked by mxbuild -- measured, the microflow that is CE0111 when included reports nothing when excluded. Flagging it would have undone mendixlabs#312 by another route, since error-severity rules block exec. The fourth hit was a genuine CE0111 in a shipped example, fixed separately. statementProducedVars reads OutputVariable by reflection: fifteen statement types carry it, the name is unambiguous, and a type added later is covered without anyone extending a list -- the blind spot that made mendixlabs#892's DROP FOLDER guard miss five document kinds. Reflection is NOT used for `Variable`, which names a produced variable on declare/create/retrieve/create list and a consumed one on change/commit/delete/rollback; those five are listed. Tests: 11 positive cases, all confirmed to fail with the rules stubbed, plus a control per rule and one per exemption. Fixtures 893-check-gaps-*.mdl are the failing script and its measured-clean counterpart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uQvFDd5R4eNqqita59jM8
CI caught MDL-SEC20 shipping too strict. `make check-mdl` runs `mxcli
check` over every example in mdl-examples/, and an error-severity rule
broke two of them — a gate `make test` does not cover and I had not run.
Measured against mxbuild 11.13 rather than argued about. The same user
role, with no System module role:
security level Off no error at all
security level Prototype CE0156 "User role should have at least one
System module role."
A blank project ships Off, so reporting this as an error unconditionally
fails scripts that are correct for the project they target. MDL-SEC20 now
warns by default and is an error only when the script itself contains
ALTER PROJECT SECURITY LEVEL set to something other than Off — at which
point the author has said which world they are in. The message says which
case applies rather than leaving the reader to wonder why it is a warning.
CE5601 is not conditional: it fires at security level Off too, in the same
measurement. So MDL-PAGE20 stays an error, and the example it flagged —
295-showpage-null-variable.mdl, a page with a $Product parameter and
`Url: 'Product_Detail'` — was genuinely broken and is fixed rather than
excused. It builds the page mxbuild rejects, which is not what a repro for
an unrelated bug should do.
The generalisable half, recorded in the symptom table: `make test` is not
the CI gate for a new validation rule. `make check-mdl` runs it against
real scripts, which is exactly where a false-positive rate shows up. Also
run scripts/check-skill-mdl.sh over .claude/skills/mendix and docs-site/src
when the change touches docs — both verified clean here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CqQLyxnppZGfqPYbUHvSWG
With the CatalogReader seam implemented, the checker has answers but still no
caller: adapters.CheckAdapter was invoked by nothing but its own test. This
wires it to `mxcli check --references` via Executor.TypeCheckProgram, so the
catalog-backed rules actually report.
Codes stay exprcheck's own E0xx. Remapping to a TC0xx of our own would mean two
vocabularies for one diagnostic, and the code printed in the message would no
longer be the code you can look up in the hints registry.
Exercising it against a real project found two more defects in the ported
adapter, both of which had kept it silent regardless of the catalog:
- exprSource read only ast.SourceExpr. The visitor attaches one to some slots
and not others — on the fixture project neither a CREATE's nor a CHANGE's enum
value carried one — so the walk had nothing to parse. Now injectable via
WithSourceFunc, and the executor passes microflowExprSource, which falls back
to rendering the AST. Controls: with the adapter's default, 0 violations; with
the executor's, 2.
- Captured source arrives with the statement's trailing layout ("'Open'\n "),
now trimmed before parsing rather than left for the lexer to recover from.
Also adds CheckNanoflow: a nanoflow's body is the same []ast.MicroflowStatement
and every rule here is about expressions, so leaving nanoflows to reach for
CheckMicroflow would make the asymmetry look deliberate.
Placement: after the reference check, because a script naming things that do not
exist has a more basic problem than a mistyped operand, and building a catalog
for a run that already failed is wasted work. Fast-mode catalog is enough — none
of the type lookups need the full build.
Only an error severity fails the run, as everywhere else in this command. A
checker whose first outing turns advice into a broken build is a checker people
turn off.
False-positive probe before making errors non-zero-exit: all 21 microflows of a
Mendix 11.13.0 app (App, Administration, FeedbackModule, MyFirstModule — most of
them marketplace-authored and Studio Pro-validated) described back to MDL and
re-checked → 0 findings.
Known depth limit, stated rather than discovered later: inferKind returns
KindUnknown for AttributePathExpr, so `if $obj/Status = 'Open'` is still NOT
caught — only slot-qualified positions reach the catalog. Closing it needs the
var→entity scope, which exprcheck.Scope cannot carry (it speaks TypeKind only)
even though the adapter already computes the map. Recorded in the proposal as
the remaining Tier-2 item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
check -p resolves references; two new rules; quieter MDL-WIDGET16
fix(domainmodel): write and read the calculated-attribute binding
…ed docs Three findings from mxcli-owid. **1. `Caption:` on a combobox is accepted and silently dropped (#38).** combobox cb (Association: Card_X, datasource: …, Caption: Name) passes `mxcli check`, executes, and the caption is gone — `describe page` does not show it, and the build fails: [error] [CE0642] "Property 'Caption' is required." at Combo box 'cb' `Caption` is in isBuiltinPropName's universal allow-list, so MDL-WIDGET01 never questions it on a widget the engine has no route for, while `OptionCaption` is correctly rejected. That allow-list exists for a good reason — it stops false positives on `Label:`/`Class:`, which the engine routes outside a def's propertyMappings — but its cost is that every unrouted builtin becomes a silent drop. The reporter concluded the association combo box was unusable and redesigned their UI around it. It is not: `CaptionAttribute:` works and round-trips. They were one property name away with nothing to tell them. New rule MDL-WIDGET17 names the working spelling. It is an explicit list, not inference: whether a builtin is routed lives in the engine's dispatch, not the .def.json — the combobox def declares optionsSourceAssociationCaption{Type,Expression} and nothing called `Caption` — so inferring it would mean reimplementing that dispatch and getting it wrong in the other direction. **2. A skill pack we ship teaches a guard that cannot fire (#30).** mendix-odata-pushdown's patterns.md showed a splice caller doing `IF $Q/Rejected THEN -- fail the request`. The pack's own QueryObject.java throws IllegalArgumentException when `r.rejected && rejectUnsupported` *before* Core.instantiate, so a caller that receives a Query at all always has Rejected = false. The branch is dead code; a project wrote and then deleted a Java action for it. The code is right and the documentation was wrong — in three places, not one. SKILL.md's field table said Rejected/RejectReason apply to `both` caller shapes, and failure-modes.md said "Pass `true`: `Rejected` comes back set". Both corrected: it is a bind-caller field, and the honest outcome for a splice caller is the 500 the throw produces. **3. Never put a grid on a Mendix widget's own class (#15, #41).** Mendix wraps a repeating widget's children in an intermediate element — a `ul` for a list view, `.mx-dataview-content` for a data view — so `display: grid` on the widget's own class has exactly one grid item and every card stacks. The skill already used `> ul > li` in one example but never stated the rule, which is why the same project hit it twice in two different widgets. Stated now, with why `min-width: 0` on the child matters and how to find the right element rather than guess. Gates run this time, after last round's miss: make test, make check-mdl, check-skill-mdl.sh over .claude/skills/mendix and docs-site/src, lint-go. Control: stubbing misusedBuiltinProperty puts the new tests back to failing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CqQLyxnppZGfqPYbUHvSWG
Catch three build errors at check time: MDL061/062/063 (mendixlabs#893 items 1, 2, 6)
One conflict, in cmd/mxcli/cmd_check.go's help text, and it is a semantic one rather than a textual clash: main (54c8f95) made --references implied by -p, because `mxcli check script.mdl -p app.mpr` used to print an unqualified "Check passed!" having resolved nothing. That is strictly better for this branch — expression type checking is inside the same block, so it now runs whenever a project is given rather than only behind an extra flag. Resolved by taking main's example line and rewriting this branch's paragraph, which said the type rules "run only with --references" and would have been wrong the moment it merged. The generated ANTLR parser needed regenerating again for main's queue clause (mdl/visitor references parser.IQueueClauseContext); `make build` does it. Re-verified after the merge, with the flag now omitted: the two-bug script still reports both E001s with -p alone, the corrected script still passes, and the false-positive probe over all 21 microflows of the 11.13.0 project is still clean. make test and make lint pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
DESCRIBE printed the element's ExposedName — Mendix's display name — instead of the raw JSON key, so its own output no longer matched the input: Total = total -> Total = Total CamelCase = camelCase -> CamelCase = CamelCase ... = item -> ... = ItemItem (array item object) LineId = id -> LineId = _id (Id is reserved) and the header was a bare `create`, so re-running the output against the project it came from failed with "import mapping already exists". Export mappings had both defects identically; the issue reported only import. DESCRIBE now prints the raw key from JsonPath and emits `create or modify`. The array case needs the trailing "|(Object)" stripped first: the mapping element sits at the ITEM object while the script addressed the array, which is where "ItemItem" came from. Elements with no JsonPath — XML-schema and message-definition mappings — keep the exposed name, which is all they have. Safe by construction: the raw path is jsonSchemaIndex.resolve's first lookup, so printing it cannot regress the mendixlabs#882 resolution. Two things were deliberately NOT changed. The capitalisation is Mendix's own convention, confirmed against a Studio Pro-authored document in a blank app (ExposedName "Uuid" against Path "(Object)|uuid") — rewriting it would diverge from Studio Pro. And the "Item" suffix could not be confirmed the same way, because a blank app ships no Studio Pro array structure; with a separate ExposedItemName property in the BSON, that deserves a marketplace module to compare against before anyone touches storage. Worth recording: the mapping already round-tripped semantically. Re-executing the old output rebuilt byte-identical JsonPaths, so this was a text/diff defect rather than a broken mapping — the title said otherwise. Verified on 11.13.0: describe output now matches the authoring script member for member, re-executes in place, leaves the stored paths identical, is a fixed point across a second describe, and builds at 0 errors. Two existing tests asserted the old bare header and were updated with the intentional change.
Make expression type checking actually check something
fix: flag a builtin property a widget cannot route; correct two shipped docs
`if $obj/Status = 'Open'` — the first example in the type-checking proposal and the shape people actually write — passed a check that caught the same mistake written as a create or change member. Two causes, both invisible: - inferKind returned KindUnknown for every AttributePathExpr, so `$obj/Attr` typed to nothing and every rule downstream of it stayed quiet. exprcheck.Scope is Lookup(name) (TypeKind, bool): it can say "$P is an Object" but not *which* entity, so there was nowhere to put the answer. The adapter computed a variable→entity map and used it only to label a slot path — never handing it to the checker — and it covered body-introduced variables but not parameters, which is the ordinary case. - Even resolved, nothing would have fired: E001 keys off the SLOT (CreateItem.Value:Entity.Attr), which exists for an assignment and not for a comparison. The object side gets its own seam (EntityScope: VariableEntity + AssociationTarget) beside Scope, rather than widening Scope or CatalogReader — CatalogReader's shape is what stays re-syncable from the upstream fork. A nil Entities is exactly today's behaviour. A multi-hop expression path is not an XPath path, and the difference is the whole reason this needs a resolver: XPath spells the intermediate entity (`[Assoc/Entity/Attr]`) so a walk can read it off, while an expression does not (`$O/Mod.Assoc/Attr`), so every hop resolves through the association index — which exprcatalog now loads from associations_data (no schema change needed). For the comparison, the same code and message as the slot form: one defect should not have two names depending on where it was spotted. Resolution also feeds the rules that were already there — E004 now flags `'x' + $O/Status` (Enumeration) while correctly staying quiet on `'x' + $O/Total` (Mendix auto-converts numerics, verified against mx check when that rule was written). Verified on a Mendix 11.13.0 project: the bug fires with the right enum QN, the corrected form is clean, and the false-positive probe over 21 microflows — whose described MDL contains 40 attribute-path lines including an association hop — is still 0 findings. Each control fails a distinct test: path→KindUnknown restored, Entities dropped from the Context, addParamEntities removed. Still open, and stated in the proposal: a terminal association step (`$Order/Mod.Order_Lines`) types to unknown, because Object vs List depends on the association's kind and direction and guessing would cost false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018hifgRSawfaRWXS44YKtSJ
feat(exprcheck): type attribute paths, and catch the enum comparison
fix(mappings): DESCRIBE reproduces the script that made the mapping
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
25 commits since the last sync (13 changes plus their merges). The bulk is one
arc — making the expression type checker do something — plus independent fixes
from field reports on mxcli-dbreplication, mxcli-owid and upstream issues.
Expression type checking becomes real
mdl/exprcheckhas shipped a complete checker for a while — parser, typelattice, function table, hints registry, slot resolver — and had never
reported a single semantic finding on a real project. Four independent silent
absences, each of which alone reduced it to a no-op:
exprcheck.CatalogReader, so every invocation ran witha nil
Catalogand every semantic rule was skipped.adapters.CheckAdapterwas called by nothing but its own test.
attributes_data.DataTypeis thebare kind (
"Enumeration", losing which one),enumerations_datastoredValueCountbut not the values,microflows_datastoredParameterCountbutnot the parameters. Catalog schema 10 adds
attributes_data.EnumerationQualifiedName,enumeration_values_dataandmicroflow_parameters_data.exprSourceread onlyast.SourceExpr,which the visitor attaches to some slots and not others — on a fixture project
neither a CREATE's nor a CHANGE's enum value carried one.
inferKindreturnedKindUnknownfor everyAttributePathExpr, so$obj/Attrtyped to nothing. The variable→entity map the adapter computed wasused only to label a slot path, never handed to the checker, and covered
body-introduced variables but not parameters — the ordinary case.
mxcli check -p app.mprnow type-checks microflow and nanoflow expressions,reporting under exprcheck's own
E0xxcodes.if $obj/Status = 'Open'— thefirst example in the proposal's problem statement — is caught, as is the
create/change-member form, and attribute paths resolve through associations.
Two things worth knowing for review:
intermediate entity (
[Assoc/Entity/Attr]) so a walk reads it off; anexpression does not (
$O/Mod.Assoc/Attr), so every hop resolves through anassociation index.
A stale or partial catalog makes the checker catch less, never
false-positive. Before letting errors exit non-zero, the false-positive probe
was 21 microflows of a Mendix 11.13.0 app described back to MDL — 40
attribute-path lines including a real association hop — with 0 findings.
Still open and recorded in the proposal: terminal association steps
(
$Order/Mod.Order_Lines) type to unknown, since Object vs List depends on theassociation's kind and direction.
checkcatches more of what the build rejectscheck -presolved nothing.mxcli check script.mdl -p app.mprprinted anunqualified "Check passed!" having looked up no icon, entity, page or
microflow name —
--referenceswas required as well.-pnow implies it, anda project-less run says plainly what it did not resolve.
by
exec, and surfaced only in Studio Pro's Errors pane (upstream mxcli check accepts seven constructs that Studio Pro rejects (CE0038/CE0068/CE0079/CE0711/CE0249/CE0111 + unvalidated icon glyphs) #893).unless the script enables security. Caught by
make check-mdl, whichmake testdoes not cover.Caption:on a combobox) wasaccepted and silently dropped; now flagged. Plus two corrections to shipped
docs.
Round-trip and write-path fixes
calculated by Module.Microflownever reached the model. It parsed,validated and reported success while
attributeToGenhad noCalculatedValuearm, so the resolved value fell through the default and was stored as an
ordinary stored value.
printed each element's
ExposedName(Mendix's display name) instead of the rawJSON key —
Total = totalcame back asTotal = Total— and emitted a barecreate, so re-running the output against its own project failed with"mapping already exists". Export mappings had both defects identically; the
issue reported only import.
ALTER PAGE SET DynamicCellClass ON <column>passescheck --referencesbut fails atexecwith "column property not found" #919): driftedalias tables, column writes that did not match the schema, and named columns.
Housekeeping
declarecolliding with a call activity's ownoutput variable) corrected.
gofmton files left unaligned by earlier commits, somake lintstopsdirtying the tree