Skip to content

fix(check,describe,lsp,lint): #962 — CE0109 on void call outputs, flow-wide duplicate warning, LSP void calls, MPR010 on native - #967

Merged
ako merged 18 commits into
mainfrom
fix/962-check-describe-lsp
Oct 3, 2026
Merged

ako merged 18 commits into
mainfrom
fix/962-check-describe-lsp

Conversation

@ako

@ako ako commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Closes #962. All four items.

Measurements (mxbuild 11.13.0, PedApp copy, exec --no-check then docker check, compared with the baseline error list)

Construct mxbuild
void Java call $V1 = …, then log … + $V1 CE0109 Undefined variable 'V1'
void JS call $V3 = … (stored JS_RevokeUploadedFileFromMemory), then $V3 as a call argument CE0109
control: non-void Java call $V2 = …, then log … + $V2 0 errors
void call $V4 = …, declare $V4 …, then a read 0 errors
microflow: non-void Java call $R in each if/else branch CE0111
microflow: retrieve $T in each if/else branch CE0111
microflow: non-void call $L inside a loop, and again after it CE0111
form DataView directly on an Atlas_Core.NativePhone_Default page 0 errors
control: the same on an Atlas_Default page 0 errors (MPR010 is a design warning)
the MPR010 advice applied on the native page (layoutgrid → row → column → dataview) CE6858 "Please update Atlas UI to version 2.4 or higher to use Layout Grid on Native pages"
form DataView in a snippet of Type Native (raw-patched from Web) 0 errors
layoutgrid-wrapped form in a native snippet CE6858

1. check reports a read of a void call's output name (MDL093)

A new rule, checkVoidCallOutputUse, runs for microflows and nanoflows. It reports a name if three things hold: the only thing naming it is a call known to be void (from the script, or from the project with -p), nothing else in the flow defines it (a parameter, a declare, or a non-void producer, anywhere in the flow), and the flow reads it. It reports each name once, as an error that names CE0109.

The void resolver now returns voidness{void, known} instead of a bool. An unresolvable action is never reported as MDL093, whatever the duplicate-name policy says. Severity is error, so exec refuses it through the shared ValidateProgram.

Note: I picked rule ID MDL093 as the next free one. A parallel PR may pick the same ID.

2. describe's duplicate-output warning is flow-wide

duplicateOutputVariableWarnings now counts non-void output names over every object collection, loop bodies included, and warns for any name that is created twice. The reachability walk is gone, and #710's cost concern goes with it because the count is linear (the 120-diamond test still passes). Void calls are still excluded.

TestFormatMicroflowActivitiesDoesNotWarnForExclusiveBranchOutputs pinned the old scoping. It is now …WarnsForExclusiveBranchOutputs, and there is a new loop-body test. On the scratch app, describe now warns for the three CE0111 microflows above. Every Studio Pro-authored microflow and nanoflow in PedApp still describes without the warning.

3. The LSP resolves void action calls through the project

runSemanticValidation used executor.ValidateMicroflow/ValidateNanoflow, which pass a nil resolver. It now uses the new executor.FlowRules (NewFlowRules(prog, s.findMprPath(), s.codeActions)):

  • Actions resolve through the script and the workspace project. The project is opened lazily, only when a call needs it.
  • An action that neither can resolve is treated as possibly void. It is not an MDL063 duplicate, and it is never MDL093.
  • One stored-action read costs about 320 ms on PedApp, so project answers are cached in the server for 30 s between keystrokes (CodeActionCache).

check keeps the strict reading from #958: an unresolvable call still counts as a declaration. The issue text says "as check does", but check does not treat unknown as void. I left check as it is, since #958 chose that deliberately to avoid hiding a real CE0111.

4. MPR010 on native pages

The advice does not apply, as the measurements above show.

  • lint: skips pages in LintContext.NativePages() and snippets whose stored Type is Native. The rule now declares RequiredCatalogMode() = CatalogFull, because LayoutRef is only recorded by a full build. A default lint already builds full because of MPR012. NativePages is added to the catalog-mode guard list, so a future rule that reads it without declaring full fails the test.
  • check: ValidatePageLayoutGrid(prog, nativeLayout). With -p, it asks the project (layoutIsNative) whether a page's layout is native, and only for a page that has something to report. Without a project it warns as before.

Test plan

  • New tests:
    • mdl/executor/validate_void_call_output_test.go: MDL093 table with controls (non-void, declare, parameter, unresolvable, unread), plus possibly-void vs known-void
    • cmd/mxcli/check_void_calls_test.go TestCheck_ReadOfAStoredVoidCallOutput: PedApp stored void JS action, Boolean and unresolvable controls
    • cmd_microflows_duplicate_output_test.go: branches and loop body
    • cmd/mxcli/lsp_void_calls_test.go: PedApp, with and without a project, Boolean control, MDL093 only with the project
    • TestCodeActionCache_SharesProjectAnswersAcrossRuns
    • mdl/linter/rules/dataview_layout_grid_test.go TestDataViewLayoutGridRule_SkipsNativePagesAndSnippets: real catalog plus a raw-unit reader, with web page and web snippet controls
    • validate_page_layout_test.go TestValidatePageLayoutGrid_SkipsNativeLayouts
    • cmd/mxcli/check_native_layout_grid_test.go: PedApp check -p
  • Revert checks (fix removed → the test fails with the symptom → fix restored):
    • MDL093 wiring for microflows and nanoflows (unit and cmd)
    • old duplicateOutputVariableWarnings (both describe tests fail)
    • LSP back on ValidateMicroflow/ValidateNanoflow (3 assertions fail)
    • unknownIsVoid line (no-project assertion fails)
    • shared-cache lookup (re-open assertion fails)
    • MPR010 native-page skip, native-snippet skip, and RequiredCatalogMode (each fails its test, the last via the catalog-mode guard)
    • check-side projectNativeLayouts → nil (cmd test fails)
  • End to end on the scratch app: check -p reports MDL093 for the CE0109 flows only. lint --rules MPR010 reports only the web page, and also the snippet once it is flipped back to Web.
  • make build, go test ./mdl/executor/ ./mdl/linter/... ./cmd/mxcli/, make check-conformance, make lint, make check-findings, make check-skill-mdl, make sync-skills

Findings appended to mdl-executor.jsonl (items 1 and 2), cmd-mxcli.jsonl (item 3) and mdl-other.jsonl (item 4). CHANGELOG has entries under Unreleased / Fixed.

Not done

  • The bug-pattern digest (check-mxbuild-drift.md) is not re-synced.

🤖 Generated with Claude Code

ako and others added 18 commits October 3, 2026 17:42
…structure and the catalog (#963)

list workflows and show structure recursed over outcome flows only and
skipped boundary-event flows and event sub-processes, so TestApp Workflow1
listed 5 activities where the catalog counted 8. The catalog's walk moves to
wfnames.WalkActivities/CountActivities and all three use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uct (#963)

The catalog has carried TotalActivityCount (loop bodies included) since #940;
microflows() now hands it to rules for microflows, nanoflows and rules alike.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, CE0109)

A call to a void Java/JavaScript action keeps its output name and declares
nothing (#953), so reading the name is CE0109 "Undefined variable" in
mxbuild 11.13.0. check now reports it for microflows and nanoflows when the
script or the project says the action is void. The void resolver returns
whether it knows the action, so an unresolvable call is never reported.

Part of #962 (item 1).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g create/change (#963)

commit $Order on a loop variable wrote no refs row because refs had no commit
kind. A commit action, or a create/change with commit Yes/YesWithoutEvents,
now emits FLOW -> ENTITY 'commit', resolved through the same intra-flow
variable map as change/delete. It stays out of the analysis graph and the
caller kinds. The ref_kind skill test now reads every RefKind constant, so a
new kind cannot ship undocumented. Catalog schema version 19.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The same output name in each if/else branch, or inside a loop and again
after it, is CE0111 in mxbuild 11.13.0. describe only warned when one
assignment reached the other, so it treated branches and loop bodies as
scopes. Count names over the whole flow instead; void calls stay excluded.
The test that pinned branch scoping now asserts the warning.

Part of #962 (item 2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The language server ran the flow rules without a project, so two calls to
a stored void action with the same output name were flagged MDL063. It now
uses executor.FlowRules: actions resolve through the script and the
workspace project, an unresolvable action is treated as possibly void (for
MDL063 only, never MDL093), and project answers are cached for 30s between
keystrokes because one read costs ~300ms on PedApp.

Part of #962 (item 3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion

widgets_data gains ParentWidgetId (nearest indexed ancestor, so skipped
wrappers, layout grid rows/columns, tab pages and pluggable property /
object-list items are transparent), Depth (0 at the page or snippet root;
a list view template is a level), Class, Style, DynamicClasses, ActionType
(raw $Type of Action, else OnClickAction, else ClickAction) and
HasConfirmation (ConfirmationInfo on a microflow/nanoflow/workflow call).
Catalog schema 19. Tested on hand-built shapes and on Studio Pro-authored
TestApp pages. mendixlabs#1268

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gets()

widgets() structs gain parent_widget_id, depth, class_name, style,
dynamic_classes, action_type, has_confirmation and page_ref.
mendixlabs#1268

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The widget table moves to write-lint-rules/catalog-tables.md (SKILL.md was
over the 700-line bound) with an example rule for inline styles, a class
allow-list and direct delete buttons. The vocabulary test scopes action_type
per section (activity vs widget), holds documented widget action types to
codec-registered storage names, and pins that only flow calls carry a
ConfirmationInfo. mendixlabs#1268

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ction' into c24-965

# Conflicts:
#	CHANGELOG.md
#	mdl/catalog/tables.go
#963's commit refs and mendixlabs#1268's widget columns both bumped 18 -> 19 on parallel
branches. A cache built at 19 by either alone would never rebuild for the
other, the 15/16 collision again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The TUI checker (after every change) and the eval runner's mx_check ran a
plain `mx check <project>`, which writes theme-cache/web/ and
deployment/sass/ into the project. Measured with mx 11.14 on a v1 and a v2
copy of the testapp: the model is left alone, those two folders are added.

New docker.MxCheckOnCopy runs `mx check` on copyProjectToTemp's copy (the
#956 helper, renamed now that it is not check-only) with output paths
rewritten to the project's, and applies PrepareMxCommand, which these two
callers lacked. mxCheckCmd takes extra args for the TUI's -j/-w/-d.

Part of #961.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tory

`docker build` (and docker run / reload, which call it) ran update-widgets
on the project under a snapshot that restored only MPRv2 storage, then
mx check and MxBuild on the project itself. Measured on the 11.14 testapp:
an MPRv1 .mpr was rewritten, every MPRv2 .mxunit was rewritten and put
back with new mtimes, and theme-cache/, deployment/, 160 javasource/
proxies, the .launch file, .classpath and .project were written into it.

buildOnCopy now runs all three tools on one copyProjectToTemp copy and
writes only the PAD output directory (absolute, default .docker/build).
MxBuild still sees the widget-normalised model, from the copy. The PAD
differs from an in-place build in the same 9 files in which two in-place
builds of identical copies differ (cache-bust stamps, operation ids,
native metro paths), and the rebuild time is unchanged (58s vs 59s).

runUpdateWidgets (the v2 snapshot) has no caller left and is removed with
its tests; build_readonly_test.go covers v1 and v2 with stub tools, and
TestBuild_LeavesProjectUntouched with real mx and MxBuild.

Part of #961.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The layout-grid advice is about Bootstrap label/input columns, which a
native page does not have. Measured on mxbuild 11.13.0 (PedApp copy): a
bare form DataView on a NativePhone_Default page, or in a native snippet,
builds clean, and wrapping it in a layoutgrid as advised is CE6858. lint
skips pages on a native layout (LintContext.NativePages, so the rule now
declares CatalogFull) and snippets of Type Native; check -p asks the
project whether a reported page's layout is native.

Part of #962 (item 4).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without checkCmd.InheritedFlags() the test only saw the project when another
test had merged the flags first.

Part of #962.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ako
ako merged commit 0cac362 into main Oct 3, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check/describe/LSP follow-ups: CE0109 on a void call's output, flow-wide duplicate warning, LSP void calls, MPR010 on native pages

1 participant