diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 8d26d20718..b6fb95babc 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -167,3 +167,4 @@ {"area": "cmd/mxcli", "date": "2026-10-05", "symptom": "`mxcli playwright check` (and the login of `run --page-check`/`--screenshot-user`) failed with \"node not found; the page check needs the Node that Playwright uses\" on a machine with no system node whose newest cached mxbuild is 11.15, although mxbuild 11.15 ships tools/node//node", "cause": "resolveNodeForScript found mxbuild's bundled node through resolveNodeTooling, which also requires tools/node/rollup-runner.mjs; mxbuild 11.15 dropped the rollup runner (rspack only), so the lookup failed before it looked for the node binary", "file": "`cmd/mxcli/docker/screenshot_login.go` (resolveNodeForScript)", "fix": "look up the node binary directly with findNodeBinary(/tools/node) instead of going through the rollup-runner check", "insight": "a helper that validates a whole toolchain was reused to find one binary in it; when the platform dropped an unrelated piece of that toolchain the reuse broke a caller that never needed it. Found by a smoke test on a machine whose newest mxbuild was 11.15, not by unit tests, which put node on PATH", "test": "cmd/mxcli/docker/screenshot_login_node_test.go TestResolveNodeForScript_RspackOnlyMxBuild (PATH emptied, 11.15 tools/node layout; fails with the old lookup)"} {"area": "cmd/mxcli", "date": "2026-10-05", "symptom": "`mxcli run --local --watch`: a model change made while the app is still booting is never built — the app keeps serving the model from before it, `--watch` logs no `Change detected`, and a waiter on the change times out. Re-running the exec does nothing (byte-idempotent: it writes nothing, so nothing re-triggers the watcher)", "cause": "watchAndApply took its baseline `last := sourceMTime(...)` after the boot finished, so an edit landing between the boot build's model read and the watch loop's start was already folded into the baseline", "file": "`cmd/mxcli/docker/runlocal.go` (`RunLocal` bootSource, `watchAndApply`)", "insight": "A watcher's baseline must be the source time the last build was MADE FROM, taken before that build — not 'now' when the watcher starts. Same class as the earlier fix that stopped moving the baseline to time.Now() after an apply. Measured with the run-lifecycle integration test: exec a page while the detached run's state says the boot build is in progress; with the old baseline `run wait` times out after 2m (no build #2), with the fix it reports `applied: build #2 via reload`. Exposed by making `run wait` race-free on source time: a generation-counting wait could not have told the change was lost.", "refs": ["cmd_run_lifecycle_integration_test.go"]} {"area": "cmd/mxcli", "date": "2026-10-06", "symptom": "`mxcli run --local --watch` on Mendix 10.24 / 11.6 dies after 5 minutes with `starting web client bundler: web client watcher timed out after 5m0s`, while the bundler's own log says `Bundling finished in 8999 milliseconds` — the bundle was done in 9s", "cause": "`parseBundlerStatus` (`cmd/mxcli/docker/webclient_watch.go`) only understood the modern-web-bundler protocol (`{\"protocol\":\"mx-modern-web-bundler\",\"type\":\"status\",\"payload\":{\"kind\":…}}`). The rollup-runner.mjs shipped with 10.24 and 11.6 predates it and writes `{\"code\":\"START\"|\"SUCCESS\"|\"ERROR\",\"payload\":…}` (ERROR payload an object with `message`, or a bare string when the config fails to load). Every status line was dropped as plain logging, so the first-build wait never saw a success", "file": "cmd/mxcli/docker/webclient_watch.go", "insight": "--watch had never worked on these versions: the parser has accepted only the new protocol since it was written (#349). Nothing exercised the watcher below 11.12 until the run-lifecycle integration test reached the nightly matrix — and it reached it only because ResolveMxForVersion substitutes ANY cached mxbuild for the test's default 11.13.0, so each matrix leg ran it against its own version. The runner's protocol is part of the mxbuild version contract: read tools/node/rollup-runner.mjs of the oldest supported version before assuming a stdout shape. Control: the lifecycle test on 11.6.8 with the parser reverted fails with the nightly's exact timeout (MXCLI_WEB_CLIENT_TIMEOUT=60s makes it fail in a minute); with the fix it passes", "refs": ["nightly 2026-10-06 (ako/mxcli run 37437089065, mendixlabs/mxcli run 37437605368)"]} +{"area": "cmd/mxcli", "date": "2026-10-06", "symptom": "`mxcli syntax rename` printed `Unknown topic: rename` and `mxcli syntax --json` had rename only for attributes, values and definitions, while `RENAME MICROFLOW TO ;` passed `mxcli check` and `mxcli help rename` documented it; an agent concluded MDL could not rename a microflow and copied, re-pointed and deleted it by hand", "cause": "The renameStatement rule (MDLParser.g4) and its executor (mdl/executor/cmd_rename.go) never got a SyntaxFeature in cmd/mxcli/syntax/; the only discovery path was the cobra `rename` subcommand's Long text, which `syntax` does not read. BySegmentMatch could not rescue it because no registered path has a `rename` segment", "file": "cmd/mxcli/syntax/features_misc.go", "insight": "Same class as the TABCONTAINER gap (widget_keywords_drift_test.go): an agent treats absence from `mxcli syntax` as absence from the language, so a statement the grammar accepts but the registry omits is effectively missing. Fixed with a `rename` topic plus a guard (rename_topic_test.go) that reads the renameTarget rule from the .g4 and requires every alternative in the topic, and see_also links from microflow/page/entity/move. The CLI subcommand and the MDL statement diverge (MDL also takes JAVA ACTION and WORKFLOW); the topic says so rather than implying parity. Control: with the topic reverted the built binary prints `Unknown topic: rename` verbatim and TestSyntaxRenameTopic fails; dropping the RENAME WORKFLOW line fails the grammar guard. A sweep for other top-level statements with no topic would be the cheap next step", "refs": ["mendixlabs/mxcli#1318"]} diff --git a/.claude/skills/fix-issue/findings/mdl-backend.jsonl b/.claude/skills/fix-issue/findings/mdl-backend.jsonl index 0734bee63c..69631cc904 100644 --- a/.claude/skills/fix-issue/findings/mdl-backend.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-backend.jsonl @@ -170,3 +170,5 @@ {"area": "mdl/backend", "date": "2026-10-04", "symptom": "ako/mxcli#980: create or modify navigation on a native profile ignored its menu block, sync block, login/not-found page and on-sync-error while reporting 'updated'; describe printed a native nanoflow home as `home microflow` and bottom bar items without action or icon", "cause": "navPatchNativeProfile only patched home pages; nativeNavProfileFromGen read BottomBarItem.Page only; the grammar had no nanoflow home", "file": "`mdl/backend/modelsdk/navigation_write.go` navPatchNativeProfile, `navigation_read.go` nativeNavProfileFromGen, `mdl/executor/cmd_navigation.go` checkProfileClauses", "insight": "A profile-kind-specific writer must refuse every clause it does not apply (checked in the executor before anything is written, including profile creation), and describe must not print what exec would refuse — list it as comments. No native profile exists in any local fixture, so the reader is pinned with a hand-built gen document", "refs": ["#980"]} {"area": "mdl/backend", "date": "2026-10-05", "symptom": "On Mendix 11.15 `TestMappingFixtureRoundTrip` fails for the blank app's own FeedbackModule mappings: describe -> exec adds `\"MessageDefinition\": \"\"`; and a mapping `with message definition` built on 11.15 fails `mx check` with CE0270 \"No root element could be found in the schema\"", "cause": "Mendix 11.15 REMOVED the mapping's `MessageDefinition` key (message definitions became `MessageDefinitions$MessageDefinition2` documents; the source moved to `MessageDefinition2` as `Module.MessageName`, measured with `mx convert` 11.15.0). The writer added the key unconditionally and the executor put the source in it, which 11.15 no longer reads", "file": "`model.ImportMapping`/`ExportMapping` (`MessageDefinition *string`, `MessageDefinitionSource()`), `mdl/backend/modelsdk/mapping_read.go` (`optionalStringFromRaw`) + `mapping_write.go`, `mdl/executor/mapping_messagedefinition.go` (`mappingMessageDefinitionKeys`, `messageDefinitionsAreDocuments`), `sdk/versions/mendix-1{0,1}.yaml` (`integration.message_definition_collection` max 11.14.99, `message_definition_document` min 11.15.0)", "insight": "**A key can be REMOVED by a version, not only introduced** \u2014 the #279 carry rule applies in both directions, so `MessageDefinition` is now a pointer like `MessageDefinition2`. The source key follows the stored document's shape on an update (a pre-11.15 document transplanted into an 11.15 project keeps its source in `MessageDefinition`, which is what kept the fixture's AgentCore/Email_Connector mappings green) and the project version only on a create. Caught by the new 11.15 nightly job, not by any fixture: none of the pinned documents was 11.15-shaped until the blank 11.15 app's own FeedbackModule mappings appeared in the base project", "refs": ["ako/mxcli#987", "ako/mxcli#279"]} {"area": "mdl/backend", "date": "2026-10-06", "symptom": "`describe microflow` prints `-- Unsupported action: Microflows$SendEmailAction` for the Studio Pro 11.13+ Send Email activity, and MDL has no statement to create it", "cause": "No reader case, and no grammar/AST/builder/writer. The vendored gen type `genMf.SendEmailAction`/`EmailMessage` predates 11.12: it binds Subject/MessageBody* as expressions, but 11.13 deleted those and stores SubjectTemplate/MessageBodyPlainTextTemplate/MessageBodyHtmlTemplate (Microflows$StringTemplate) plus a CustomHeaders list the gen type has no field for", "file": "mdl/backend/modelsdk/microflow_send_email.go", "insight": "The gen type existing (the issue cited it as 'the storage side seems known') was a false lead: it is the pre-11.12 shape, so both directions are raw-keyed against a Studio Pro-saved document (ako/TestApp Email.EmailMF) and a key-for-key shape test (TestSendEmailActionToGen_MatchesStudioProShape) pins types and markers — it caught ConnectionTimeout being int64 and the empty CustomHeaders list needing marker 3. Version facts come from mendixmodelsdk 4.116 StructureVersionInfo (templates 11.12.0, plain Subject deleted 11.13.0), hence the 11.13.0 gate. Run negative controls through mxbuild before writing check rules: of the plausible rules, 'check server identity needs SSL' and the header-name rule are ACCEPTED by mxbuild 11.15.0-rc.4 (so warnings), while type faults are CE9528 for address/host/port/user (not the CE0117 exprcheck's E009 used to name — slots can now carry their own Mxbuild code). Attachment is a bare variable name (CE0109 on a wrong one). Shapes MDL cannot restate (auth document, pre-11.13 expression subject, non-SMTP) stay UnsupportedAction. Studio Pro's Test Email tab (TestEmailMessage) is written empty on rewrite, by design. An unedited describe->exec round trip is elided as Unchanged, so it cannot demonstrate loss; author-from-script is the meaningful control (old binary: `mismatched input 'email' expecting REST`).", "refs": ["mendixlabs/mxcli#1315"], "ce": ["CE9528", "CE0117", "CE0720", "CE0109"], "rules": ["MDL-EMAIL01", "MDL-EMAIL02", "MDL-EMAIL03", "E009"]} +{"area": "mdl/backend", "date": "2026-10-07", "symptom": "After `RENAME JAVA ACTION M.JA_Old TO JA_New` (or `mxcli rename java-action`), javasource/m/actions/JA_New.java still declares `public class JA_Old`, its constructor `public JA_Old(` and toString `return \"JA_Old\";` \u2014 javac: `class JA_Old is public, should be declared in a file named JA_Old.java`. `mxcli docker build` reports BUILD SUCCEEDED regardless", "cause": "Backend.RenameJavaSourceFile (mdl/backend/modelsdk/java_write.go) only os.Rename'd the file; nothing rewrote the generated parts that carry the action's name", "file": "mdl/backend/modelsdk/java_write.go, sdk/javaactions/rename.go", "insight": "A green mxbuild does NOT prove the javasource on disk compiles: mxbuild regenerates action stubs in the copy it builds (class, constructor, toString follow the model; user code, extra code and the import list are kept byte-for-byte, a mention of the old name inside them included), so the defect only shows where the on-disk file is compiled directly \u2014 `run --local --watch` hot reload, IDEs. The cheap probe is javac on the one file with runtime/bundles/com.mendix.public-api.jar on the classpath. The oracle for the rewrite is mxbuild itself: run the mxbuild binary on the project IN PLACE (not `docker build`, which uses a temp copy) and diff the javasource file \u2014 it changed exactly three lines; that pair is now testdata/mxbuild/JA_Renamed*. Rewrite those three spots, not a full GenerateSource, which would reformat a Studio Pro-authored file. Control: unfixed binary \u2192 javac error above; fixed \u2192 javac exit 0, docker build BUILD SUCCEEDED", "refs": ["follow-up to mendixlabs/mxcli#1318"]} +{"area": "mdl/backend", "date": "2026-10-06", "symptom": "Opening a change in Studio Pro's Changes panel throws `System.InvalidOperationException: Objects with ID … of type Forms$MicroflowParameterMapping do not have the same properties. baseNames = Expression, Parameter, Variable; newNames = Parameter, Expression` after `alter page … set ('onClickAction': call microflow M.F(P = $P)) on w`. `mx check` is 0 errors; the page opens. Second, silent half: `$P` (a page parameter) is stored as the Expression \"$P\", which Studio Pro does not bind (CE1571, the #1140 form)", "cause": "(1) `bindParameterMappingValue` sets only Expression for an Expression-bound argument; the gen `Variable` Part stays unset and the encoder omits an unset Part on a new element, so the key vanished. Studio Pro writes all three keys, nulling the unused slot, and its merge library compares key sets per $ID — the canon $ID carry makes the new mapping the SAME object as the stored one, so the diff sees two shapes of one element. (2) `convertASTAction` (ALTER PAGE SET Action and named action slots) built its pageBuilder with no paramScope/localVariables, so `classifyFlowArgValue` could not recognise a page/snippet parameter", "file": "`mdl/backend/modelsdk/widget_write.go` (RegisterTypeDefaults NullFields Variable on Forms$MicroflowParameterMapping + Forms$NanoflowParameterMapping), `mdl/executor/cmd_alter_page.go` (convertASTAction seeds ParamScope, storedPageVariables, isSnippet from the mutator)", "insight": "**`mx diff base.mpr new.mpr out.mpr` reproduces Changes-panel crashes headless** — it runs the same MergeLib DifferenceComputer and prints the identical \"do not have the same properties\" line (exit 129). Base = a copy of the project before the write, made by the FIXED build so it has Studio Pro's key set. That turns a Windows-GUI-only symptom into a two-binary A/B on Linux. **The base must share the $ID** — the crash needs a paired object, which the identity carry supplies; a fresh element is only an add and never trips it, so a create-only repro passes on broken code. **Register the null slot, don't set it at the call site**: `Part.Set(nil)` still encodes as omitted, and there are three construction sites (action, nanoflow action, nanoflow data source) — NullFields covers all and any future one. **A builder constructed outside CREATE PAGE is the recurring hole for #1140-style classification**: INSERT/REPLACE already seeded the scope from the mutator, SET did not; grep for `&pageBuilder{` and check each seeds paramScope + localVariables + isSnippet. Measured on mxbuild 11.12.2: buggy mx diff exit 129 with the reported line, fixed exit 0; mx check 0 errors on both", "refs": ["mendixlabs/mxcli#1317", "mendixlabs/mxcli#1180", "mendixlabs/mxcli#1140"], "ce": ["CE1571"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 78100e9361..935d6b987f 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -868,3 +868,10 @@ {"area": "mdl/executor", "date": "2026-10-05", "symptom": "On Mendix 11.15, `TestMxCheck_DoctypeScripts/40-message-definition-examples.mdl` fails with CE0270 \"No root element could be found in the schema\" at MsgTest.IMM_Order. On a converted or Studio Pro 11.15 project, `list message definition collections` finds nothing, and `describe import mapping` prints no source", "cause": "Mendix 11.15 replaced `MessageDefinitions$MessageDefinitionCollection` with one `MessageDefinitions$MessageDefinition2` document per definition. `mx convert` turns the collection into a `Projects$Folder` with the same unit ID and name, and the ExposedEntity tree is byte-identical apart from $IDs. mxcli read and wrote only collections, so on 11.15 there was nothing to resolve a mapping against", "file": "`mdl/backend/modelsdk/messagedefinition_document.go` (read/write, reusing the collection's exposedNodeFromGen/messageNodeToGen), `mdl/executor/cmd_messagedefinition_documents.go` (create/describe/drop/list + version refusals), `mdl/executor/mapping_messagedefinition.go` (`findMessageDefinition(ctx, ref)` returns the stored reference), `cmd_messagedefinitions.go` (alter target resolution, association-drop guard), grammar `createMessageDefinitionStatement` / `DROP|DESCRIBE MESSAGE DEFINITION` / `LIST MESSAGE DEFINITIONS`", "insight": "**`mx convert` is the oracle for a storage change.** Converting a known 11.14 project gives a Studio Pro-authored 11.15 reference for the new shape and for how references move, so no guessing is needed. **Under the revert, 11.15 fails earlier than CE0270.** A two-part name written into the old `MessageDefinition` key makes mxbuild refuse to load the project (`StorageLoadException: ... is not a valid OldMessageDefinitionIdentifier`). The old key is typed as a three-part identifier even though 11.15 no longer reads it. The doctype script uses `-- @version: ..11.14` / `11.15+` sections, and the 11.15 mapping needs a different name from the older section's, because a plain `check` sees both sections and reports MDL-DUPDEF", "refs": ["ako/mxcli#987"]} {"area": "mdl/executor", "date": "2026-10-06", "symptom": "v0.25.0: `describe microflow` 4-8x slower than v0.24.0 on a flow arranged by hand in Studio Pro, and the cost grows faster than the flow (60 if/else blocks: 4.7 s -> 35.6 s); flows laid out by mxcli barely change", "cause": "The canonical describe's layout derivation (#748) re-renders the description every round, and a hand-laid flow takes gatedLayoutRounds+2 = 8 rounds. Each render re-ran the stored flow's split/merge analysis (findSplitMergePoints, labelCrossedMerges) and the body warnings (postDominators via microflowgraph.Analyze) - superlinear, and independent of which annotations the round keeps", "file": "`mdl/executor/cmd_microflows_derived_layout.go` (flowAnalysisMemo, installed by useDerivedFlowLayout, rebuildOnly view for the rounds), `mdl/executor/cmd_microflows_show.go` (findSplitMergePoints / labels / microflowBodyWarnings go through it)", "insight": "**Profile before believing the issue's theory.** The report blamed the round count, but rounds were already capped at 8; the rebuild itself was cheap. The CPU profile showed the cost was the *render* inside each round - the stored flow's graph analysis, redone per round. Memoising it per describe (keyed on the stored collection pointer) and leaving the `-- WARNING` comment lines out of rounds (the rebuild never reads them) gave 14.2 s -> 1.7 s at 60 blocks, with the printed description byte-identical to the unfixed build. A wall-clock assertion would be flaky; the test counts split/merge analyses per describe (18 over 8 rounds before, 2 after), with the round count as its control", "refs": ["mendixlabs/mxcli#1301"]} {"area":"mdl-executor","date":"2026-10-05","symptom":"A view entity selecting a **non-localized** DateTime column (Studio Pro's \"Localize\" unticked — the normal choice for a calendar date) passes `mxcli check --references` and exec, then `mx check` fails CE6770 \"View Entity is out of sync with the OQL Query.\" The view attribute was always written LocalizeDate = true, and MDL has no spelling for the flag, so describe -> exec re-broke it on every run","cause":"execCreateViewEntity built every view attribute with convertDataType, whose DateTime is Mendix's default LocalizeDate = true; nothing looked at the attribute the column reads","file":"`mdl/executor/oql_view_localize_date.go` (viewDateTimeLocalize), `mdl/executor/cmd_entities.go` (execCreateViewEntity); test `mdl/executor/view_entity_localize_date_test.go`; bug-test `mdl-examples/bug-tests/1297-view-entity-non-localized-datetime.mdl`","insight":"**Derive, don't add syntax**: on a view entity the column's localization is a property of the query, so the fix reads it from the source attribute and describe -> exec round-trips (`Unchanged`) with nothing new to spell — same reasoning as the view association (the column is the declaration). **Measure the shapes before choosing the scope**: one project, one view per shape, mxbuild 11.12.5 — pass-through, `s/Attr`, MIN, MAX and CASE over a non-localized source are ALL CE6770 when written localized and 0 errors when patched to false, so a pass-through-only rule (the obvious one, mirroring the string-length rule) would have fixed the report and left MAX(date) broken. Sources that disagree (coalesce of a localized and a non-localized column) are unmeasured and keep the default. **Repro needs a Studio Pro-authored flag**: mxcli writes every persistent DateTime localized, so the bug is invisible from MDL alone — the reporter's pymongo patch flipping Sale.SaleDate is the cheapest stand-in. Control: stubbing the assignment fails 5 of 6 cases with `LocalizeDate = true`; pre-fix binary on the same project gives 5 × CE6770, fixed gives 0","refs":["mendixlabs/mxcli#1297"]} +{"area": "mdl-executor", "date": "2026-10-07", "refs": ["mendixlabs/mxcli#1324"], "symptom": "A button directly inside data view `dvGate` passing `$dvGate` (its own container, by widget name) passes `check --references` and `exec`, then `mx check` reports `[error] [CE0117] \"Error(s) in expression.\" at Action button 'btnOwn'`. The same `$dvGate` from a button in a NESTED data view builds clean", "cause": "No check modelled which data-container names are in scope. A container's widget-name variable exists only for the data containers nested below it; in its own context the object is $currentObject", "file": "`mdl/executor/validate_page_button_context.go` (`checkButtonContextTree` carries `nearest`; `checkOwnContainerName`, `exprReadsVariable`, MDL-BUTTON02)", "insight": "**Measure the neighbours before writing the rule — the report's shape is one of six.** One probe page with ten buttons through `mx check` 11.14.0 settled the whole rule in one build: own data view, own data view through a plain container, a control bar of a grid inside it, `$dv/Attr`, a list view's own name from its item, a grid's own name from its column and a gallery's from its template are all CE0117; the enclosing name from a grid column or list item is clean, and so is a grid's own name from its control bar (the selection). So the rule is 'the NEAREST data container's name', and it moves exactly like the existing MDL-BUTTON01 context (a control bar takes its context from above its grid) — which is why it went into the same walk rather than a new one. A data-view-only fix, the literal reading of the issue, would have missed four of the six. **Unmeasured, deliberately not flagged:** the same name in a nested widget's DATA SOURCE arguments and in widget expressions (visibility, dynamic text). Control: stubbing the call fails all three tests with `got []`; end to end the fixed binary refuses the issue's script ('Nothing was written') and the suggested `$currentObject` rewrite builds at 0 errors. Repro `mdl-examples/bug-tests/1324-own-data-container-name.fail.mdl`; tests `validate_page_own_container_name_test.go`"} +{"area": "mdl-executor", "date": "2026-10-07", "symptom": "`retrieve … where starts-with(Name, 'MS-' + $Key)` passes `check --references` and `exec`, then mx check CE0161 \"Error(s) in XPath constraint.\" — the same `+` at the constraint's top level (`Name = 'X-' + $Key`) builds clean", "cause": "MDL091 only matched expression-only function NAMES (startsWith/endsWith/getKey) by regex over the rendered XPath string; an operator inside a function ARGUMENT is a different expression construct the string regex never looked for", "file": "`mdl/executor/validate_microflow.go` (`xpathFunctionArgumentOperators`, `checkXPathFunctionArgumentOperators`, MDL091)", "insight": "Walk the `RetrieveStmt.Where` AST, not the XPath string: a call's argument that IS a `+`/`-` BinaryExpr is CE0161 (after unwrapping ParenExpr AND `SourceExpr` — the retrieve where-clause is wrapped in a SourceExpr, and a walker that misses it silently matches nothing, which is how the first cut passed nothing while compiling clean). Only the unbracketed form reaches the writer: the bracketed `[…]` grammar already refuses `+` in a function argument as a parse error. Measured on mxbuild 11.14.0 with a stubbed-out check to force the fault in: `+` and `-` in a string argument CE0161 (also nested in `not()`), `year-from-dateTime(Due) = $N - 1` clean. `*`/`div`/`mod` deliberately NOT flagged — the only argument they fit is numeric, and the control `contains(Name, $N)` is already CE0161 with no operator, so no measurement isolates the operator; MDL091 is exec-enforced, so an unmeasured operator would be a write barrier on a guess. Tests `TestValidateMicroflow_XPathFunctionArgumentOperator`, `TestCheckExecAgree_RetrieveConstraintFunctionArgumentOperator`; repro `mdl-examples/bug-tests/1326-xpath-function-argument-operator.fail.mdl`", "refs": ["mendixlabs/mxcli#1326", "mendixlabs/mxcli#1213"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $Cursor = $Next;` where both are OBJECT variables (Reference retrieves followed from the FROM entity) passes `check --references` and `exec` (\"Created microflow\"), is written as a Microflows$ChangeVariableAction, and mx check reports [CE7247] \"Variable 'Cursor' does not have a primitive type.\" Natural to write when walking a parent chain in a `while` loop", "cause": "`set` on a plain variable had two outcomes: Change list Replace for a known list (ako/mxcli#949) and Change variable for everything else, so an object fell into the primitive-only action. Mendix has NO action that reassigns an object variable. Check could not have caught it even with a guard: the check-time validator (validateFlowBody) typed every association retrieve as `List of `, because it had no association multiplicity, so the forward-Reference object read as a list", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (`objectVariableType`, `refuseSetOnObject`), called from `cmd_microflows_builder_graph.go` (exec) and `cmd_microflows_builder_validate.go` (check); `validate.go` passes `checkAssociationShapes(ctx, sc)` into `validateFlowBody` so `forwardReferenceTarget` types a forward Reference retrieve as an object", "insight": "A refusal is only as good as the variable typing under it, and check and exec type variables differently: exec asks the backend for the association, check guessed `List of`. A guard added to the check validator alone passes the reported repro because the object it should refuse is, to check, a list. Write the check-path test against the REPORTED shape (association retrieve), not an object parameter, or it goes green while the issue stays open. Reuse `checkAssociationShapes` (project + script associations, built for MDL-ASSOCDS01) rather than a new lookup. Type only the unambiguous shape (Reference followed from FROM, not a self-association): Mendix types a self-association retrieve as a list, and that `set` is a valid Change list Replace. Measured on 11.14.0: faulted build exec -> CE7247; the self-association and recursive sub-microflow forms -> 0 errors. Plain `check` with no -p does not run validateFlowBody at all, so a `.fail.mdl` cannot pin this", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#949"], "ce": ["CE7247"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "Plain `mxcli check` (no -p) passed `set $X = $Y;` on an OBJECT variable (entity parameter, create, retrieve first, loop iterator, head) even after check --references and exec refused it; the build then failed with CE7247. The bug-test could not be a .fail.mdl because check-mdl runs plain check", "cause": "Plain check runs only the MDL0xx rule set (ValidateProgram -> ValidateMicroflow/ValidateNanoflow); validateFlowBody, where the first #1323 refusal lived, runs only under --references (validate.go) and in exec. A refusal added to one validator is invisible to the other path", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable`, MDL-SET01), wired from `validate_microflow.go` (validate) and `validate_nanoflow.go`; `cmd_microflows_builder_validate.go` now reports only `assocObjectVars`", "insight": "Know which validator each entry point runs before adding a refusal: plain check = ValidateMicroflow; --references = that PLUS validateFlowBody; exec = the builder (plus execEnforcedMicroflowRules). Putting the rule in both check validators made --references print it twice; split by what each can see — the rule takes every object visible in the text, validateFlowBody only the association-typed ones that need the project. Measure every producer a rule judges, with the faulted build: all five plus a nanoflow gave CE7247 on 11.14.0, but a PARAMETER gives different wording, \"Parameter 'A' cannot be changed.\" — and that wording also fires for a PRIMITIVE parameter (`set $N = 1` on `$N: Integer`), a separate unrefused gap the object-only rule does not cover. Leave `find` out of the object producers: it is also the String function", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#1011"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} +{"area": "mdl-executor", "date": "2026-10-06", "symptom": "`create constant M.Flag (Type: Boolean, DefaultValue: true)` (or `True`) stores DefaultValue \"true\". Studio Pro stores \"True\"/\"False\"; its constant dialog shows a stored \"true\" as False while the runtime reads it as true, so the developer sees one value and the app runs with another. `mx check` is silent and `describe constant` prints `true` for both, so only the stored unit shows it. Quoted `'True'` was the workaround. `alter settings constant @M.Flag value true` (or 'true') wrote the configuration override the same way", "cause": "createConstant rendered the AST literal with fmt.Sprintf(\"%v\"), so a Go bool became Go's lowercase \"true\"; the quoted string passed through verbatim, which is why only 'True' was right. The settings path stored the visitor's token text verbatim and never looked at the constant's type", "file": "`mdl/executor/cmd_constants.go` (storedConstantDefault, used by the create and create-or-modify branches, and by `alterSettingsConstant` in `mdl/executor/cmd_settings.go` via settingsConstantType); test `mdl/executor/cmd_constant_boolean_default_test.go`; bug-test `mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl`", "insight": "**`%v` on an AST literal is Go's spelling, not Mendix's**: any place that stringifies a parsed value into a stored property must normalise to the platform's form. The case-insensitive reader (formatDefaultValue's EqualFold) hid the writer's defect from describe, so a round-trip test could not catch it; assert on the value handed to the backend. Neither `mx check` nor the runtime distinguishes the two spellings — only Studio Pro's dialog does — so a build cannot verify this; the evidence is the reporter's Studio Pro 11.12.4 measurement plus the stored unit. Control: test written before the fix failed 5 of 6 spellings with `stored as \"true\"`/`\"false\"`, the quoted 'True' control passing; exec on a copy of testdata/pedapp then stores True/True/True/False. **Enumerate every write path for the value, not just the reported one**: the configuration override holds the same typed value and had the same defect; it needs the constant's type looked up (a String constant holding 'true' must stay lowercase). Control: the settings test failed 3 of 5 cases with `stored as \"true\"`/`\"FALSE\"` before the fix, the 'True' and String-constant controls passing; the stored Settings unit on a pedapp copy read 'true' for `value true` and `value 'true'` before, 'True' after, the String override 'true' both times", "refs": ["mendixlabs/mxcli#1321"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "A button added to a snippet with `alter snippet … insert` (or replace) calling `call microflow M.F(P = $P)` with a snippet parameter fails mxbuild with CE0115 \"The arguments that are passed to microflow 'M.F' do not match the expected parameters and need to be refreshed\" on that button only; the same button in CREATE SNIPPET or via ALTER SNIPPET SET is clean", "cause": "`buildWidgetsFromAST` and `buildColumnSpecsFromAST` (mdl/executor/cmd_alter_page.go) seeded paramScope and localVariables from the mutator but not `isSnippet`, so `classifyFlowArgValue` returned kind \"parameter\" and the Forms$PageVariable named PageParameter instead of SnippetParameter", "file": "`mdl/executor/cmd_alter_page.go` (isSnippet from mutator.ContainerType() in both builders)", "insight": "**A pageBuilder built outside CREATE needs THREE scope fields, not two** — paramScope, localVariables and isSnippet; #1317 found the SET builder missing all three and this one missing the last. When adding a builder, copy the seeding from convertASTAction rather than from memory. **Unlike the Expression-form defect (#1140/#1317), mxbuild DOES catch the wrong slot** — CE0115 rather than CE1571 — so one script exercising CREATE, INSERT and SET on the same snippet with `mx check` is a complete A/B: only the broken path's button is named. Verified on 11.12.2: 1 error before, 0 after", "refs": ["mendixlabs/mxcli#1317", "mendixlabs/mxcli#1140"], "ce": ["CE0115"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $N = 1;` on a microflow/nanoflow/rule PARAMETER ($N: Integer or String) passes `mxcli check` and `exec`; mx check reports [CE7247] \"Parameter 'N' cannot be changed.\" at the Change variable activity", "cause": "Nothing modelled that a Change variable activity cannot target a parameter; the variable-kind tracking treated a primitive parameter like any declared primitive. Found while measuring MDL-SET01's object producers (#1323): the object parameter's CE7247 came back with a different wording, which was the tell that the restriction is on parameters, not on types", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable` primitive-parameter branch, `setTargetViolations`); rules via `validate_program.go` (check) and `rule_validation.go` `validateRuleSetTargets` (exec, `cmd_rules_create.go`); MDL-SET01 added to `execEnforcedMicroflowRules` in `validate.go`", "insight": "When one measurement returns a different MESSAGE under the same CE code, test the cause the message names — here 'Parameter … cannot be changed' fired for an Integer too, a whole adjacent class. Measure the neighbours before writing the rule: list parameters (`set` = Change list Replace, `add`) and member changes on parameters BUILD, so the rule is 'any non-list parameter', not 'any parameter'. Rules never run ValidateMicroflow: plain check needs a ValidateProgram hook and exec a gate in cmd_rules_create; putting it in validateRule instead would double-print under --references, which runs both", "refs": ["mendixlabs/mxcli#1323"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} diff --git a/.claude/skills/mendix/cheatsheet-errors/SKILL.md b/.claude/skills/mendix/cheatsheet-errors/SKILL.md index 578f4695f8..002af66d67 100644 --- a/.claude/skills/mendix/cheatsheet-errors/SKILL.md +++ b/.claude/skills/mendix/cheatsheet-errors/SKILL.md @@ -265,6 +265,7 @@ Run with `-p` for the fullest coverage. | CE0104 | Action activity is unreachable | Code after RETURN | | CE0105 | Must end with end event | Missing RETURN | | CE0117 | Error in expression | Unqualified association path | +| CE0117 | …on a button inside a data container | The button passes the container it sits in by its widget name (`$dvGate` inside `dvGate`). That name is a variable only for containers nested below it; use `$currentObject` there. MDL-BUTTON02 | | CE1571 | No argument selected for parameter | A microflow/nanoflow call with a parameter nothing fills — as a `datasource:` **or** an `action:`. Give it an argument (`action: call nanoflow M.NF(P = $value)`), or nest the widget in a data container of the parameter's type. `check -p` reports both | | CE1571 | …in a control bar | A control bar is **not** row-scoped, so the grid's row does not fill it: pass the grid's selection (`$dgOrders`, with `Selection:` set) or move the widget into a column. `$currentObject` there is MDL-BUTTON01 | | CE1834 | The 'Page' property is required | Workflow user task without a `page` — `check` flags MDL-WF01 | diff --git a/.claude/skills/mendix/create-page/reference/widgets.md b/.claude/skills/mendix/create-page/reference/widgets.md index e6e1154cc9..24d650928d 100644 --- a/.claude/skills/mendix/create-page/reference/widgets.md +++ b/.claude/skills/mendix/create-page/reference/widgets.md @@ -1068,6 +1068,15 @@ A **container** takes an argument list exactly like an `actionbutton` does — t two share one action grammar. Reaching for a button because a container "cannot pass parameters" changes the rendering for no reason (mendixlabs/mxcli#1082). +**A data container's name is a variable only *below* it.** `$dvOrder` reads data +view `dvOrder`'s object from a data view, list or grid nested inside it. In the +container's own context — directly inside it, through plain containers, or in the +control bar of a grid inside it — the object is `$currentObject`, and +`$dvOrder` is **CE0117** "Error(s) in expression." (`mxcli check` reports +MDL-BUTTON02). The same goes for a list view, gallery or grid read by its own +name from its item or row. A grid's own name *is* valid from its control bar — +that is the selection, above (mendixlabs/mxcli#1324). + ### Charts (Charts.mpk — ColumnChart / BarChart / AreaChart / PieChart) Charts are pluggable widgets whose data lives in one or more `series` object-list diff --git a/.claude/skills/mendix/write-microflows/reference/data-operations.md b/.claude/skills/mendix/write-microflows/reference/data-operations.md index e7f319897e..dc20190903 100644 --- a/.claude/skills/mendix/write-microflows/reference/data-operations.md +++ b/.claude/skills/mendix/write-microflows/reference/data-operations.md @@ -134,6 +134,17 @@ primitive type"). mxcli knows a variable is a list when it is a list parameter, a `create list`, a list retrieve or a list operation's result. Both work in microflows and nanoflows. +`set` on an **object** variable is refused (MDL-SET01 in `check`; `check --references` and `exec` also catch an object from an association retrieve): +Mendix has no action that reassigns an object variable, and a Change variable on +one is the same CE7247. To walk a chain (`$Cursor = $Next` in a `while` loop), +write a sub-microflow that **returns** the next object and recurse; to change the +object itself, use `change $Obj (…)`. + +`set` on a **parameter** is refused as well (MDL-SET01), whatever its type unless +it is a list: a Change variable cannot target a parameter (CE7247 "Parameter 'N' +cannot be changed."), in microflows, nanoflows and rules. Copy it into a variable +first — `declare $Value Integer = $N;` — and change that. + ### One statement per activity Every list operation and aggregate is **one Studio Pro activity**, and it is diff --git a/.claude/skills/mendix/xpath-constraints/SKILL.md b/.claude/skills/mendix/xpath-constraints/SKILL.md index 50299a1034..634d04def1 100644 --- a/.claude/skills/mendix/xpath-constraints/SKILL.md +++ b/.claude/skills/mendix/xpath-constraints/SKILL.md @@ -215,7 +215,10 @@ where [Displayed = false()] Supported functions: `contains()`, `starts-with()`, `not()`, `true()`, `false()` The expression functions `startsWith()` / `endsWith()` are not XPath: in a -constraint they are CE0161, and `check` reports them as **MDL091**. A member the +constraint they are CE0161, and `check` reports them as **MDL091**. So is an +operator inside a function argument — `starts-with(Name, 'MS-' + $Key)` is +CE0161 although `Name = 'X-' + $Key` is fine: compute the value into a variable +first (`declare $P String = 'MS-' + $Key;`, then `starts-with(Name, $P)`). A member the entity does not have is a reference error in `check -p`, and so is a system member written the way `describe` prints the attribute: XPath spells it `createdDate`, `changedDate`, `owner`, `changedBy` — `[CreatedDate > …]` is CE0161. diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a7e53879..fd508e07e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`call rest service` takes its settings as one property list** (ADR-0013) — the activity's dialog settings go in one `( Key: value, … )` list after the URL, keyed as the consumed REST service names the same concepts: `$Html = call rest service get 'https://example.com' (Headers: ('Accept': 'text/html'), Authentication: basic (Username: $User, Password: $Password), Timeout: 300) returns String;`. `Body:` is `template '…' [with ({1} = …)]`, `mapping M.EMM from $Var`, `binary ` or an expression. The method, URL, `returns …` and `on error …` stay words. An unknown or repeated key, or a value of the wrong shape, is an error. `describe` writes this form. **Migrating a script:** nothing breaks — the clauses `header 'N' = v`, `auth basic $u password $p`, `body …` and `timeout n` (**MDL-DEPR720**) still parse and store the same activity, `check` / `exec` warn, and `mxcli fmt --upgrade` rewrites them; a statement cannot mix the two forms. ADR-0013 makes this the rule for every new microflow activity and every activity with several settings. - **`run --local --page-check` signs in with `--screenshot-user` without `--screenshot`, and checks all pages in one browser** — the sign-in only ran when `--screenshot` was also given, so `--page-check --screenshot-user U` reported every secured page as the login page. The verdict also no longer counts a list view's "No items found" placeholder or a data grid's header as rows, ignores the demo-user switcher's "Select user" heading, and reports a failed same-origin request (`HTTP 560 POST /xas/`) instead of the duplicate "Failed to load resource" console line. The login script used by `--screenshot-user` falls back to `/login.html` when the app root does not show the sign-in form, and finds Playwright the way the page check does (no `playwright` CLI on `PATH` needed). +- **`mxcli rename` takes `java-action` and `workflow`** — the subcommand offered eight of the ten targets `RENAME` accepts; Java actions and workflows could only be renamed from MDL. `mxcli rename -p app.mpr java-action M.JA_Old JA_New` also renames the `.java` source file, and both update `docs/brain/` anchors like the other types. The type list is now held to the grammar by a test, so a new `RENAME` target cannot be left out of the subcommand again. `mxcli syntax rename` documents both forms. (follow-up to mendixlabs/mxcli#1318) + ### Fixed +- **`set $Param = …` on a parameter is refused** — a Change variable cannot target a parameter, and mxbuild rejects it with CE7247 "Parameter 'N' cannot be changed." (measured on 11.14.0 for Integer and String parameters in a microflow, a nanoflow and a rule). `check` and `exec` passed it. MDL-SET01 now refuses it for every parameter but a list (`set` on a list parameter is a Change list Replace, which builds), and `exec` enforces the rule. Copy the parameter into a variable first: `declare $Value Integer = $N;`. +- **`check` refuses a button that passes its own data container by widget name** (mendixlabs/mxcli#1324) — `actionbutton btnOwn (Action: call microflow M.F(Gate = $dvGate))` directly inside data view `dvGate` passed `check --references` and `exec`, then `mx check` reported `[CE0117] "Error(s) in expression." at Action button 'btnOwn'`. A data container's name is a variable only for the containers nested below it; in its own context the object is `$currentObject`. Reported as **MDL-BUTTON02** (error, so `exec` refuses it), for data views, list views, galleries and data grids alike, including attribute paths (`$dvGate/Name`) and a control bar inside the container. A grid's own name from its control bar (the selection) and an enclosing container's name from a nested one are not flagged. The flagged set matches mxbuild 11.14.0's CE0117s widget for widget on a ten-button probe page. +- **`set $Obj = …` on an object variable is refused** (mendixlabs/mxcli#1323) — with both variables single objects (e.g. a Reference retrieved from its FROM entity), `set $Cursor = $Next;` passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable, so `check` (MDL-SET01, for the objects it can see without a project), `check --references` and `exec` now refuse it and name the alternatives (a sub-microflow that returns the next object, `change $Obj (…)`). `check --references` now types a Reference retrieve from its FROM entity as the object it is. `set` on a list variable stays a Change list Replace (ako/mxcli#949). +- **`RENAME JAVA ACTION` renames the Java class, not just the file** — renaming `M.JA_Old` to `JA_New` moved the source to `JA_New.java` but left `public class JA_Old`, its constructor and its `toString` inside it, which javac rejects (`class JA_Old is public, should be declared in a file named JA_Old.java`). A full build hid it, because mxbuild regenerates the stub in its own copy; `run --local --watch` hot reload and IDEs compile the file on disk. Those three places now follow the rename exactly as mxbuild rewrites them; user code and extra code are left as written. Applies to `mxcli rename java-action` too. - **`run --local --watch` starts on Mendix 10.24 and 11.6** — it waited the full web-client timeout (5 minutes) for a bundle that had finished in seconds, then failed with `web client watcher timed out`. The rollup runner those versions ship reports its status as `{"code":"SUCCESS"}` rather than the modern-web-bundler protocol mxcli was reading; both are now understood, including that runner's error reports. - **`run --local --watch` no longer loses a change made while the app boots** — the watch loop took its baseline after the boot, so a model written during the ~15 s boot was never built and the app kept serving the model from before it. The baseline is now the source time the boot build was made from, so that change is built on the first tick. diff --git a/cmd/mxcli/cmd_rename.go b/cmd/mxcli/cmd_rename.go index 9eb0770823..e44f062899 100644 --- a/cmd/mxcli/cmd_rename.go +++ b/cmd/mxcli/cmd_rename.go @@ -26,6 +26,8 @@ Types: enumeration Rename an enumeration association Rename an association constant Rename a constant + java-action Rename a Java action (also renames its .java source file) + workflow Rename a workflow module Rename a module (updates all qualified names) Use --dry-run to preview changes without modifying. @@ -39,6 +41,8 @@ Example: mxcli rename -p app.mpr entity MyModule.Customer Client mxcli rename -p app.mpr microflow MyModule.ACT_Old ACT_New mxcli rename -p app.mpr page MyModule.OldPage NewPage + mxcli rename -p app.mpr java-action MyModule.JA_Old JA_New + mxcli rename -p app.mpr workflow MyModule.WF_Old WF_New mxcli rename -p app.mpr module OldModule NewModule mxcli rename -p app.mpr entity MyModule.Customer Client --dry-run `, @@ -52,36 +56,12 @@ Example: dryRun, _ := cmd.Flags().GetBool("dry-run") - objectType := strings.ToUpper(args[0]) qualifiedName := args[1] newName := args[2] - var mdlCmd string - dryRunSuffix := "" - if dryRun { - dryRunSuffix = " DRY RUN" - } - - switch objectType { - case "ENTITY": - mdlCmd = fmt.Sprintf("RENAME ENTITY %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "MICROFLOW": - mdlCmd = fmt.Sprintf("RENAME MICROFLOW %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "NANOFLOW": - mdlCmd = fmt.Sprintf("RENAME NANOFLOW %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "PAGE": - mdlCmd = fmt.Sprintf("RENAME PAGE %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "ENUMERATION": - mdlCmd = fmt.Sprintf("RENAME ENUMERATION %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "ASSOCIATION": - mdlCmd = fmt.Sprintf("RENAME ASSOCIATION %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "CONSTANT": - mdlCmd = fmt.Sprintf("RENAME CONSTANT %s TO %s%s", qualifiedName, newName, dryRunSuffix) - case "MODULE": - mdlCmd = fmt.Sprintf("RENAME MODULE %s TO %s%s", qualifiedName, newName, dryRunSuffix) - default: - fmt.Fprintf(os.Stderr, "Unknown type: %s\n", args[0]) - fmt.Fprintln(os.Stderr, "Valid types: entity, microflow, nanoflow, page, enumeration, association, constant, module") + mdlCmd, objectType, err := renameStatement(args[0], qualifiedName, newName, dryRun) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } @@ -118,6 +98,50 @@ Example: }, } +// renameTypes maps the subcommand's type argument to the RENAME target keyword. +// +// One table instead of a case per type: the switch it replaces offered eight of +// the grammar's ten targets, JAVA ACTION and WORKFLOW being reachable only from +// MDL, and nothing noticed. cmd_rename_test.go now holds this table to the +// renameTarget rule in MDLParser.g4. A two-word keyword is typed with '-' (or +// '_', or run together), since a space would split the shell argument. +var renameTypes = []struct { + arg string + keyword string +}{ + {"entity", "ENTITY"}, + {"microflow", "MICROFLOW"}, + {"nanoflow", "NANOFLOW"}, + {"page", "PAGE"}, + {"enumeration", "ENUMERATION"}, + {"association", "ASSOCIATION"}, + {"constant", "CONSTANT"}, + {"java-action", "JAVA ACTION"}, + {"workflow", "WORKFLOW"}, + {"module", "MODULE"}, +} + +// renameStatement builds the RENAME statement for one `mxcli rename` call and +// returns it with the target keyword, which is what renameBrainAnchors keys on. +func renameStatement(typeArg, qualifiedName, newName string, dryRun bool) (stmt, keyword string, err error) { + norm := strings.NewReplacer("_", "", "-", "").Replace(strings.ToLower(typeArg)) + var valid []string + for _, t := range renameTypes { + valid = append(valid, t.arg) + if strings.ReplaceAll(t.arg, "-", "") == norm { + keyword = t.keyword + } + } + if keyword == "" { + return "", "", fmt.Errorf("unknown type: %s (valid types: %s)", typeArg, strings.Join(valid, ", ")) + } + stmt = fmt.Sprintf("RENAME %s %s TO %s", keyword, qualifiedName, newName) + if dryRun { + stmt += " DRY RUN" + } + return stmt, keyword, nil +} + // renameBrainAnchors keeps docs/brain/ pointing at the thing that was renamed. // // The model's own cross-references are updated by the RENAME statement above. @@ -185,7 +209,8 @@ func renameBrainAnchors(projectPath, objectType, qualifiedName, newName string, // corrupt entries instead of repairing them. func brainRenameTarget(objectType, qualifiedName string) (string, bool) { switch objectType { - case "ENTITY", "MICROFLOW", "NANOFLOW", "PAGE", "ENUMERATION", "ASSOCIATION", "CONSTANT": + case "ENTITY", "MICROFLOW", "NANOFLOW", "PAGE", "ENUMERATION", "ASSOCIATION", "CONSTANT", + "JAVA ACTION", "WORKFLOW": // These are all Module.Element, which is what an anchor names. if !strings.Contains(qualifiedName, ".") { return "", false diff --git a/cmd/mxcli/cmd_rename_brain_test.go b/cmd/mxcli/cmd_rename_brain_test.go index 75c9f0462c..f936e8a7a5 100644 --- a/cmd/mxcli/cmd_rename_brain_test.go +++ b/cmd/mxcli/cmd_rename_brain_test.go @@ -22,6 +22,8 @@ func TestBrainRenameTargetOnlyClaimsWhatAnAnchorCanName(t *testing.T) { {"ENUMERATION", "Sales.ENUM_Status", "Sales.ENUM_Status", true}, {"ASSOCIATION", "Sales.Order_Customer", "Sales.Order_Customer", true}, {"CONSTANT", "Sales.ApiRoot", "Sales.ApiRoot", true}, + {"JAVA ACTION", "Sales.JA_Hash", "Sales.JA_Hash", true}, + {"WORKFLOW", "Sales.WF_Approve", "Sales.WF_Approve", true}, {"MODULE", "Sales", "Sales", true}, // An element rename with no module cannot be turned into an anchor: diff --git a/cmd/mxcli/cmd_rename_test.go b/cmd/mxcli/cmd_rename_test.go new file mode 100644 index 0000000000..b78bd8dcdb --- /dev/null +++ b/cmd/mxcli/cmd_rename_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// grammarRenameTargets reads the renameTarget rule out of the committed +// grammar, plus MODULE (renameStatement's second alternative), as the words a +// user types: "JAVA ACTION", not two tokens. +func grammarRenameTargets(t *testing.T) []string { + t.Helper() + path := filepath.Join("..", "..", "mdl", "grammar", "MDLParser.g4") + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + rule := regexp.MustCompile(`(?s)\nrenameTarget\s*\n\s*:(.*?)\n\s*;`).FindStringSubmatch(string(b)) + if rule == nil { + t.Fatal("renameTarget rule not found — re-point this guard rather than deleting it") + } + var out []string + for _, alt := range strings.Split(rule[1], "|") { + if alt = strings.Join(strings.Fields(alt), " "); alt != "" { + out = append(out, alt) + } + } + return append(out, "MODULE") +} + +// `mxcli rename` offered eight of the ten targets RENAME accepts: JAVA ACTION +// and WORKFLOW were MDL-only, because the subcommand kept its own switch of +// types and nothing tied it to the grammar (follow-up to mendixlabs/mxcli#1318). +// Every grammar target must be reachable from the shell, spelled as the +// keyword in lower case with '-' for the space, and must build a statement +// that parses to the RENAME it names. +func TestRenameSubcommandCoversEveryGrammarTarget(t *testing.T) { + for _, target := range grammarRenameTargets(t) { + arg := strings.ToLower(strings.ReplaceAll(target, " ", "-")) + t.Run(arg, func(t *testing.T) { + name := "Sales.Old" + if target == "MODULE" { + name = "Sales" + } + stmt, keyword, err := renameStatement(arg, name, "New", false) + if err != nil { + t.Fatalf("mxcli rename %s: %v", arg, err) + } + if keyword != target { + t.Errorf("type %q maps to %q, want %q", arg, keyword, target) + } + prog, errs := visitor.Build(stmt) + if len(errs) > 0 { + t.Fatalf("%q does not parse: %v", stmt, errs[0]) + } + rs, ok := prog.Statements[0].(*ast.RenameStmt) + if !ok { + t.Fatalf("%q parsed as %T, not a RENAME", stmt, prog.Statements[0]) + } + if want := strings.ToLower(strings.ReplaceAll(target, " ", "")); rs.ObjectType != want { + t.Errorf("%q renames a %q, want %q", stmt, rs.ObjectType, want) + } + }) + } +} + +func TestRenameStatementSpellings(t *testing.T) { + for _, tc := range []struct { + arg string + want string + }{ + {"java-action", "RENAME JAVA ACTION Sales.Old TO New DRY RUN"}, + {"javaaction", "RENAME JAVA ACTION Sales.Old TO New DRY RUN"}, + {"java_action", "RENAME JAVA ACTION Sales.Old TO New DRY RUN"}, + {"Workflow", "RENAME WORKFLOW Sales.Old TO New DRY RUN"}, + {"entity", "RENAME ENTITY Sales.Old TO New DRY RUN"}, + } { + got, _, err := renameStatement(tc.arg, "Sales.Old", "New", true) + if err != nil || got != tc.want { + t.Errorf("renameStatement(%q) = %q, %v; want %q", tc.arg, got, err, tc.want) + } + } + if _, _, err := renameStatement("folder", "Sales.Old", "New", false); err == nil { + t.Error("an unknown type must be refused") + } +} diff --git a/cmd/mxcli/cmd_syntax_test.go b/cmd/mxcli/cmd_syntax_test.go index 27f42f6a5c..38c995575b 100644 --- a/cmd/mxcli/cmd_syntax_test.go +++ b/cmd/mxcli/cmd_syntax_test.go @@ -127,3 +127,17 @@ func firstLines(s string, n int) string { } return strings.Join(lines, "\n") } + +// mendixlabs/mxcli#1318, the reported command verbatim: `mxcli syntax rename` +// answered "Unknown topic: rename" while RENAME MICROFLOW parsed and ran. +func TestSyntaxRenameTopic(t *testing.T) { + out := runSyntax(t, "rename") + if strings.Contains(out, "Unknown topic") { + t.Fatalf("mxcli syntax rename reported an unknown topic:\n%s", firstLines(out, 3)) + } + for _, want := range []string{"RENAME MICROFLOW", "RENAME PAGE", "mxcli rename"} { + if !strings.Contains(out, want) { + t.Errorf("mxcli syntax rename does not show %q", want) + } + } +} diff --git a/cmd/mxcli/syntax/features_domain_model.go b/cmd/mxcli/syntax/features_domain_model.go index d9901a517f..eb38e2ea8f 100644 --- a/cmd/mxcli/syntax/features_domain_model.go +++ b/cmd/mxcli/syntax/features_domain_model.go @@ -71,7 +71,7 @@ func init() { }, Syntax: "CREATE PERSISTENT ENTITY Module.Name (\n Attr: Type [constraints],\n ...\n) [INDEX (attr1)];\n\n-- Documentation is the /** … */ doc comment before the statement.\n-- (A `COMMENT 'text'` option existed, set nothing, and has been removed.)\n\nCREATE NON-PERSISTENT ENTITY Module.Name (...);\n\nCREATE PERSISTENT ENTITY Module.Name EXTENDS Module.Parent (...);", Example: "/** Stores customer information. */\nCREATE PERSISTENT ENTITY MyModule.Customer (\n Name: String(100) NOT NULL ERROR MESSAGE 'Name is required',\n Email: String(200) UNIQUE,\n Balance: Decimal DEFAULT 0,\n IsActive: Boolean DEFAULT true,\n Status: Enumeration(MyModule.CustomerType)\n)\nINDEX (Email);", - SeeAlso: []string{"domain-model.entity.create", "domain-model.entity.alter", "domain-model.entity.attributes"}, + SeeAlso: []string{"domain-model.entity.create", "domain-model.entity.alter", "domain-model.entity.attributes", "rename"}, }) Register(SyntaxFeature{ diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index 2af7960b7a..4a413b5ee5 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -12,7 +12,7 @@ func init() { }, Syntax: "CREATE [OR REPLACE | OR MODIFY] MICROFLOW Module.Name ($Param: Type) RETURNS Type AS $Result\nBEGIN\n \nEND;", Example: "mdl 1;\nCREATE MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n COMMIT $NewOrder;\n RETURN $NewOrder;\nEND;\n\n-- Re-runnable: replaces the microflow if it already exists\nCREATE OR MODIFY MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n RETURN $NewOrder;\nEND;", - SeeAlso: []string{"microflow.create", "microflow.variables", "microflow.control-flow", "create-modifiers"}, + SeeAlso: []string{"microflow.create", "microflow.variables", "microflow.control-flow", "create-modifiers", "rename"}, }) Register(SyntaxFeature{ @@ -134,7 +134,12 @@ func init() { "DECLARE $Var Type = expression;\n" + "$Var = expression; -- assign; SET is optional\n" + "$Var/Attribute = expression;\n" + - "SET $Var = expression; -- same statement, explicit form", + "SET $Var = expression; -- same statement, explicit form\n" + + "-- $Var must be a primitive or a list (a list is a Change list Replace).\n" + + "-- An OBJECT variable cannot be reassigned (CE7247, MDL-SET01): return the\n" + + "-- new object from a sub-microflow, or `change $Obj (…)` its members.\n" + + "-- A parameter cannot be reassigned either (CE7247, MDL-SET01) unless it is\n" + + "-- a list: copy it first, `DECLARE $Value Integer = $N;`.", Example: "DECLARE $Count Integer = 0;\nDECLARE $Name String;\nset $Count = $Count + 1;\nset $Name = 'Hello';\nSET $Order/Status = 'Pending';", SeeAlso: []string{"microflow.object-operations"}, }) diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index e0db875202..3853391552 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -1061,7 +1061,72 @@ DROP FOLDER 'OldFolder' IN Module; -- Read the placement back LIST FOLDERS IN MyModule;`, - SeeAlso: []string{"folders"}, + SeeAlso: []string{"folders", "rename"}, + }) + + // ── Rename ────────────────────────────────────────────────────────── + + // RENAME parsed, ran and was in `mxcli help rename` but had no topic here, + // so an agent consulting `syntax` concluded a microflow could not be + // renamed and rebuilt it by hand (mendixlabs/mxcli#1318). + // rename_topic_test.go holds the target list to the grammar. + Register(SyntaxFeature{ + Path: "rename", + Summary: "RENAME — rename a document, entity, association or module and update every reference", + Keywords: []string{ + "rename", "rename microflow", "rename nanoflow", "rename page", + "rename entity", "rename enumeration", "rename association", + "rename constant", "rename java action", "rename workflow", + "rename module", "dry run", "refactor", "update references", + }, + Syntax: `RENAME Module.OldName TO NewName [DRY RUN]; +-- target: ENTITY | MICROFLOW | NANOFLOW | PAGE | ENUMERATION | ASSOCIATION +-- | CONSTANT | JAVA ACTION | WORKFLOW +RENAME MODULE OldModule TO NewModule [DRY RUN]; + +-- The new name is BARE: the element stays in its module (use MOVE to change +-- module). An element of that name already in the module is an error. +-- +-- Every reference is updated in the same statement: each stored string in the +-- project that IS the old qualified name, or starts with it plus '.', is +-- rewritten (Module.Old -> Module.New, Module.Old.Attr -> Module.New.Attr). +-- That covers calls, page and microflow parameters, show-page actions, +-- navigation, security, attribute types, association ends. +-- A name inside free text — a microflow expression, an XPath string — is not +-- such a string and is left as it was; build or 'mxcli docker check' reports +-- what remains. +-- +-- RENAME JAVA ACTION also renames the .java source file and the class in it +-- (class, constructor, toString); the user and extra code are left as written. +-- RENAME MODULE rewrites every 'OldModule.' prefix project-wide. +-- +-- DRY RUN changes nothing and lists each document that would change and how +-- many references it holds. +-- +-- Members are renamed with ALTER, not RENAME: +-- ALTER ENTITY Module.E RENAME ATTRIBUTE Old TO New; +-- ALTER ENUMERATION Module.E RENAME VALUE Old TO New; +-- +-- From the shell, the same statement for one element: +-- mxcli rename -p app.mpr Module.OldName NewName [--dry-run] +-- type: entity | microflow | nanoflow | page | enumeration | association +-- | constant | java-action | workflow | module +-- (it also updates docs/brain/ anchors; see 'mxcli help rename')`, + Example: `mdl 1; +-- See what would change first +RENAME MICROFLOW Shop.ACT_Old TO ACT_ProcessOrder DRY RUN; + +RENAME MICROFLOW Shop.ACT_Old TO ACT_ProcessOrder; +RENAME NANOFLOW Shop.NF_Old TO NF_Validate; +RENAME PAGE Shop.OldPage TO Order_Edit; +RENAME ENTITY Shop.Customer TO Client; +RENAME ENUMERATION Shop.Status TO OrderStatus; +RENAME ASSOCIATION Shop.Order_Customer TO Order_Client; +RENAME CONSTANT Shop.ApiUrl TO ServiceUrl; +RENAME JAVA ACTION Shop.JA_Old TO JA_Hash; +RENAME WORKFLOW Shop.WF_Old TO WF_Approve; +RENAME MODULE Shop TO Store;`, + SeeAlso: []string{"move", "domain-model.entity.alter", "domain-model.enumeration"}, }) // ── Folders ───────────────────────────────────────────────────────── diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index bda76bf879..0e4a53514e 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -14,7 +14,7 @@ func init() { }, Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Page Title',\n Layout: Module.LayoutName\n [, Params: ( $Param: Module.Entity )]\n [, Url: 'page-url']\n [, Variables: ( $var: Boolean = 'true' )]\n [, PopupWidth: 800, PopupHeight: 480, PopupResizable: true]\n [, PopupCloseAction: cancelButton1]\n [, Class: 'css-class', Style: 'css: rule']\n )\n {\n -- widgets\n }", Example: "CREATE PAGE MyModule.EditCustomer\n (\n Params: ( $Customer: MyModule.Customer ),\n Title: 'Edit Customer',\n Layout: Atlas_Core.PopupLayout,\n Class: 'container-fluid'\n )\n {\n DATAVIEW dvCustomer (DataSource: $Customer) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n FOOTER {\n ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE CHANGES, ButtonStyle: Primary)\n ACTIONBUTTON btnCancel (Caption: 'Cancel', Action: CANCEL CHANGES)\n }\n }\n };", - SeeAlso: []string{"page.create", "page.widgets", "page.alter", "snippet"}, + SeeAlso: []string{"page.create", "page.widgets", "page.alter", "snippet", "rename"}, }) Register(SyntaxFeature{ diff --git a/cmd/mxcli/syntax/rename_topic_test.go b/cmd/mxcli/syntax/rename_topic_test.go new file mode 100644 index 0000000000..63892db34b --- /dev/null +++ b/cmd/mxcli/syntax/rename_topic_test.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 + +package syntax + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// mendixlabs/mxcli#1318: "mxcli syntax rename" said "Unknown topic: rename" +// while RENAME MICROFLOW/PAGE/ENTITY parsed, executed, and were documented by +// `mxcli help rename`. An agent consulting `syntax` concluded MDL could not +// rename a microflow and rebuilt it by hand: copy, re-point every caller, +// delete the original. +// +// As with the widget keywords (widget_keywords_drift_test.go), the grammar is +// the authority: every target the renameStatement rule accepts must be named in +// the rename topic, so a target added to the parser cannot ship undocumented. + +// renameTargets reads the renameTarget rule plus the MODULE alternative of +// renameStatement out of the committed grammar, as the words a user types +// ("JAVA ACTION", not "JAVA ACTION" split in two). +func renameTargets(t *testing.T) []string { + t.Helper() + path := filepath.Join("..", "..", "..", "mdl", "grammar", "MDLParser.g4") + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + src := string(b) + rule := regexp.MustCompile(`(?s)\nrenameTarget\s*\n\s*:(.*?)\n\s*;`).FindStringSubmatch(src) + if rule == nil { + t.Fatal("renameTarget rule not found — if the rule was renamed, re-point this guard " + + "rather than deleting it; it exists because RENAME shipped absent from `mxcli syntax` (#1318)") + } + var out []string + for _, alt := range strings.Split(rule[1], "|") { + if alt = strings.Join(strings.Fields(alt), " "); alt != "" { + out = append(out, alt) + } + } + if !regexp.MustCompile(`RENAME MODULE identifierOrKeyword TO`).MatchString(src) { + t.Fatal("RENAME MODULE alternative not found in renameStatement — re-point this guard") + } + return append(out, "MODULE") +} + +func TestRenameTopicIsRegistered(t *testing.T) { + if ByPath(ResolveAlias("rename")) == nil { + t.Fatal(`"rename" resolves to no syntax topic — ` + "`mxcli syntax rename`" + ` reports "Unknown topic: rename" (#1318)`) + } +} + +func TestRenameTopicNamesEveryGrammarTarget(t *testing.T) { + f := ByPath(ResolveAlias("rename")) + if f == nil { + t.Fatal(`no "rename" topic (#1318)`) + } + doc := strings.ToUpper(f.Syntax + "\n" + f.Example) + for _, target := range renameTargets(t) { + if !strings.Contains(doc, "RENAME "+target) { + t.Errorf("the grammar accepts RENAME %s but the rename topic never shows it", target) + } + } +} + +// The topics a reader is on when the question arises point at it, so the +// statement is found from the document type as well as from the verb. +func TestRenameTopicIsCrossReferenced(t *testing.T) { + for _, from := range []string{"microflow", "page", "domain-model.entity", "move"} { + f := ByPath(from) + if f == nil { + t.Errorf("topic %q not registered", from) + continue + } + found := false + for _, ref := range f.SeeAlso { + if ref == "rename" { + found = true + } + } + if !found { + t.Errorf("topic %q has no see_also to rename", from) + } + } +} diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index fce5247594..5e0883e9fa 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -276,6 +276,41 @@ $n = count $Approved; The same applies to both operands of `union`/`intersect`/`subtract`. +### MDL-SET01: `set` on an object variable + +``` +cannot set object variable '$Cursor' (G46.Group): Mendix has no action that reassigns an +object variable — Change variable takes only a primitive, and mxbuild rejects it with +CE7247 "Variable 'Cursor' does not have a primitive type". [MDL-SET01] +``` + +**Cause:** `set $Var = …` is a Change variable activity, which takes only a primitive +variable; on a list it is a Change list Replace. Mendix has no activity that reassigns an +object variable, so the statement used to be written as a Change variable and fail the build +with CE7247 (mendixlabs/mxcli#1323). It comes up naturally when walking a parent chain with +`set $Cursor = $Next` in a `while` loop. On a parameter mxbuild words the same code +`"Parameter 'X' cannot be changed."`. + +Plain `check` reports it for the objects it can see without a project — entity parameters, +`create`, `retrieve … first`, loop iterators, casts, `head`. Whether an association retrieve +is an object or a list depends on the association, so that case is reported by +`check --references` and `exec`. + +**Solution:** Return the new object from a sub-microflow (recursion for a chain walk), +retrieve it into a new variable, or change the object's members with `change $Obj (…)`. + +The same rule refuses `set` on a **parameter** of any type but a list, in a microflow, a +nanoflow or a rule: + +``` +cannot set parameter '$N' (Integer): a Change variable cannot target a parameter, and +mxbuild rejects it with CE7247 "Parameter 'N' cannot be changed". [MDL-SET01] +``` + +Copy the parameter into a variable and change that (`declare $Value Integer = $N;`). A list +parameter is not refused — `set $L = $M` on one is a Change list Replace, which builds — and +neither is a member change, `set $Param/Attr = …`. + ### MDL-EMAIL02 / 03: A `send email` setting Studio Pro would not allow ``` diff --git a/mdl-examples/bug-tests/1317-alter-snippet-insert-flow-arg.mdl b/mdl-examples/bug-tests/1317-alter-snippet-insert-flow-arg.mdl new file mode 100644 index 0000000000..eeb9162b69 --- /dev/null +++ b/mdl-examples/bug-tests/1317-alter-snippet-insert-flow-arg.mdl @@ -0,0 +1,66 @@ +mdl 1; +-- Follow-up to mendixlabs/mxcli#1317 — ALTER SNIPPET INSERT/REPLACE bound a +-- snippet-parameter flow argument as a PAGE parameter. +-- +-- buildWidgetsFromAST / buildColumnSpecsFromAST seeded the stored parameter scope +-- but never set isSnippet, so classifyFlowArgValue returned kind "parameter" and +-- the mapping's Forms$PageVariable carried PageParameter: "Order" inside a +-- snippet. CREATE SNIPPET and ALTER SNIPPET SET (fixed in #1317) bind the same +-- argument as SnippetParameter: "Order". +-- +-- Measured on mxbuild 11.12.2 with this script on a fresh project: +-- before: [CE0115] "The arguments that are passed to microflow 'SN.ACT_Use' do +-- not match the expected parameters and need to be refreshed." at +-- Action button 'btnInsert' -> 1 error, on the INSERTED button only +-- after: 0 errors; all three buttons bind SnippetParameter: "Order" +-- +-- btnCreate (CREATE path) and btnSet (SET path) are the controls. + +create module SN; +create persistent entity SN.Order ( Code: String(20) ); +create or modify microflow SN.ACT_Use ($Order: SN.Order) +begin + log info 'use'; +end; +create snippet SN.SNIPPET_Order ( params: ( $Order: SN.Order ) ) { + container ctn { + actionbutton btnCreate (Caption: 'Create', Action: call microflow SN.ACT_Use(Order = $Order)) + actionbutton btnSet (Caption: 'Set', Action: call microflow SN.ACT_Use(Order = empty)) + } +}; +alter snippet SN.SNIPPET_Order { + insert after btnCreate { + actionbutton btnInsert (Caption: 'Insert', Action: call microflow SN.ACT_Use(Order = $Order)) + } +}; +alter snippet SN.SNIPPET_Order { + set (Action: call microflow SN.ACT_Use(Order = $Order)) on btnSet +}; + +-- The same defect through the DATA GRID COLUMN builder (buildColumnSpecsFromAST): +-- a flow button in a column inserted into a grid inside a snippet. Measured on +-- mxbuild 11.12.2: before, PageParameter: "Order" and CE0115 at btnCol; after, +-- SnippetParameter: "Order" and 0 errors. $currentObject stays an Expression. + +create module SC; +create persistent entity SC.Order ( Code: String(20) ); +create persistent entity SC.Line ( Qty: Integer ); +create or modify microflow SC.ACT_Use ($Order: SC.Order, $Line: SC.Line) +begin + log info 'use'; +end; +create snippet SC.SNIPPET_Lines ( params: ( $Order: SC.Order ) ) { + container ctn { + datagrid dgLines ( datasource: database from SC.Line ) { + column (attribute: Qty, caption: 'Qty') + } + } +}; +alter snippet SC.SNIPPET_Lines { + insert after dgLines.Qty { + column Act ( attribute: Qty, caption: 'Act', ShowContentAs: customContent ) { + actionbutton btnCol (Caption: 'Use', + Action: call microflow SC.ACT_Use(Order = $Order, Line = $currentObject)) + } + } +}; diff --git a/mdl-examples/bug-tests/1317-flow-arg-mapping-null-variable.mdl b/mdl-examples/bug-tests/1317-flow-arg-mapping-null-variable.mdl new file mode 100644 index 0000000000..3c7cbaf3ee --- /dev/null +++ b/mdl-examples/bug-tests/1317-flow-arg-mapping-null-variable.mdl @@ -0,0 +1,71 @@ +mdl 1; +-- mendixlabs/mxcli#1317 — an ALTER PAGE that set a call-microflow action crashed +-- Studio Pro's Changes panel when the change was opened: +-- +-- System.InvalidOperationException: Objects with ID [redacted] of type +-- Forms$MicroflowParameterMapping do not have the same properties. +-- baseNames = Expression, Parameter, Variable; newNames = Parameter, Expression +-- +-- Reported statement (Studio Pro 11.12.2, a pluggable widget's named slot): +-- alter page LearningJourney.LearningJourney_Details { +-- set ('onClickAction': call microflow LearningJourney.ACT_ExcludeItemStep( +-- LearningJourney = $LearningJourney)) on pDSLink_ButtonAndTracking8 }; +-- +-- TWO DEFECTS, both on this statement: +-- +-- 1. An argument bound through Expression (a literal, $currentObject, a path) +-- was written WITHOUT the Variable key. Studio Pro writes all three keys and +-- nulls the unused slot; the merge library compares key sets per $ID and +-- throws. Same class as #1180 (DomainModels$NoGeneralization). Every flow +-- action and nanoflow data source shares the writer, so this was never +-- specific to ALTER. +-- +-- 2. ALTER PAGE SET built the action with an EMPTY parameter scope, so +-- `$LearningJourney` — a page parameter — was not recognised and was written +-- as the Expression "$LearningJourney" instead of Variable -> PageVariable: +-- the #1140 binding, which Studio Pro does not resolve (CE1571 on opening). +-- CREATE PAGE and ALTER PAGE INSERT/REPLACE already seeded the scope. +-- +-- REPRODUCING. `mx diff` runs the same merge library as the Changes panel: +-- +-- mxcli exec 1317-flow-arg-mapping-null-variable.mdl -p app.mpr (up to ALTER) +-- copy app.mpr + mprcontents/ aside as the base, then run the ALTER +-- mx diff base/app.mpr app.mpr out.mpr +-- +-- Measured on mxbuild 11.12.2: before the fix `mx diff` exits 129 with +-- "Objects with ID … of type Forms$MicroflowParameterMapping do not have the same +-- properties. baseNames = Parameter, Expression, Variable; newNames = Parameter, +-- Expression"; after, exit 0. `mx check` is 0 errors on BOTH — the build is not +-- a safety net for either defect. + +create module LJ; + +create persistent entity LJ.LearningJourney ( Name: String(100) ); + +create or modify layout LJ.App_Default ( layouttype: 'Responsive' ) { + scrollcontainer layoutContainer { region center { placeholder Main } } +}; + +create or modify microflow LJ.ACT_ExcludeItemStep + ($LearningJourney: LJ.LearningJourney, $Flag: Boolean) +begin + log info 'exclude'; +end; + +create or modify page LJ.LearningJourney_Details ( + Title: 'Details', Layout: LJ.App_Default, + Params: ( $LearningJourney: LJ.LearningJourney ) +) { + placeholder Main { + dataview dv (DataSource: $LearningJourney) { + actionbutton btnGo (Caption: 'Go', + Action: call microflow LJ.ACT_ExcludeItemStep(LearningJourney = $currentObject, Flag = true)) + } + } +}; + +-- The ALTER. $LearningJourney must bind through Variable (defect 2); the literal +-- `false` stays an Expression and must carry `Variable: null` (defect 1). +alter page LJ.LearningJourney_Details { + set (Action: call microflow LJ.ACT_ExcludeItemStep(LearningJourney = $LearningJourney, Flag = false)) on btnGo +}; diff --git a/mdl-examples/bug-tests/1318-rename-statements-documented.mdl b/mdl-examples/bug-tests/1318-rename-statements-documented.mdl new file mode 100644 index 0000000000..47e2790d89 --- /dev/null +++ b/mdl-examples/bug-tests/1318-rename-statements-documented.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1318 — "mxcli syntax rename" said "Unknown topic: rename" +-- ============================================================================ +-- +-- RENAME MICROFLOW/PAGE/ENTITY parsed and ran, and `mxcli help rename` +-- documented them, but `mxcli syntax` had no rename topic, so an agent +-- concluded MDL could not rename a microflow and rebuilt it by hand. +-- +-- The fix is a syntax topic; the guard that keeps it in step with the grammar +-- is cmd/mxcli/syntax/rename_topic_test.go. This file keeps the documented +-- forms parsing: every RENAME target, DRY RUN, and RENAME MODULE. +-- ============================================================================ + +CREATE MODULE BugRename1318; + +CREATE OR MODIFY PERSISTENT ENTITY BugRename1318.Customer ( Name: String(100) ); + +CREATE OR MODIFY MICROFLOW BugRename1318.ACT_Old () +BEGIN + LOG INFO NODE 'BugRename1318' 'old'; +END; + +RENAME MICROFLOW BugRename1318.ACT_Old TO ACT_New DRY RUN; +RENAME MICROFLOW BugRename1318.ACT_Old TO ACT_New; +RENAME ENTITY BugRename1318.Customer TO Client; +RENAME MODULE BugRename1318 TO BugRename1318b; diff --git a/mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl b/mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl new file mode 100644 index 0000000000..1f0c085a6f --- /dev/null +++ b/mdl-examples/bug-tests/1321-boolean-constant-default-case.mdl @@ -0,0 +1,28 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1321: Boolean constant DefaultValue stored in Studio Pro's case +-- ============================================================================ +-- +-- Before: `DefaultValue: true` and `DefaultValue: True` were stored as "true". +-- Studio Pro stores a Boolean default as "True" / "False"; its constant dialog +-- shows a stored "true" as False while the runtime reads it as true, so the +-- value a developer sees and the value the app runs with differed. `mx check` +-- reports nothing either way and `describe constant` prints both identically, +-- so the difference only shows in the stored unit. +-- +-- After: every spelling below is stored as "True" or "False". + +create module BugTest1321; + +create constant BugTest1321.FlagA (Type: Boolean, DefaultValue: true); +create constant BugTest1321.FlagB (Type: Boolean, DefaultValue: True); +create constant BugTest1321.FlagC (Type: Boolean, DefaultValue: 'True'); +create constant BugTest1321.FlagD (Type: Boolean, DefaultValue: false); + +-- A configuration override of a Boolean constant had the same defect: the +-- value was stored as typed ("true"). It is now stored as "True" / "False"; +-- a String constant's override is stored as written. +create constant BugTest1321.Label (Type: String, DefaultValue: 'x'); +alter settings constant @BugTest1321.FlagD value true; +alter settings constant @BugTest1321.FlagA value 'false'; +alter settings constant @BugTest1321.Label value 'true'; diff --git a/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl b/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl new file mode 100644 index 0000000000..9bac0011ed --- /dev/null +++ b/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- Upstream #1323 (the refusal): `set` on an OBJECT variable is MDL-SET01 +-- ============================================================================ +-- +-- Mendix has no action that reassigns an object variable. Written as a Change +-- variable action, this is CE7247 "Variable 'Cursor' does not have a primitive +-- type." at build time (measured on 11.14.0). Plain `mxcli check` must refuse +-- it. The reported script used association retrieves, whose object-or-list +-- typing needs the project (check --references / exec refuse that form); here +-- the cursor is a `retrieve … first`, which plain check can see is an object. +-- (On a parameter mxbuild words it "Parameter 'X' cannot be changed.", still +-- CE7247.) See the companion 1323-set-object-variable.mdl for the forms that +-- build. +-- +-- This file is expected to FAIL `mxcli check`. +-- ============================================================================ + +create module F1323F; +create persistent entity F1323F.Node ("Name": String(50)); + +create microflow F1323F.Reassign ($Next: F1323F.Node) returns F1323F.Node +begin + retrieve $Cursor from F1323F.Node first; + set $Cursor = $Next; + return $Cursor; +end; diff --git a/mdl-examples/bug-tests/1323-set-object-variable.mdl b/mdl-examples/bug-tests/1323-set-object-variable.mdl new file mode 100644 index 0000000000..160cb1f532 --- /dev/null +++ b/mdl-examples/bug-tests/1323-set-object-variable.mdl @@ -0,0 +1,47 @@ +mdl 1; +-- ============================================================================ +-- Upstream #1323: `set $Obj = $Other` on an OBJECT variable is refused +-- ============================================================================ +-- +-- Reported (v0.24.0 and v0.25.0, Mendix 11.14.0): with $Cursor and $Next both +-- single G46.Group objects (a Reference followed from its FROM entity), +-- +-- set $Cursor = $Next; +-- +-- passed `check --references` and `exec`, and was written as a +-- Microflows$ChangeVariableAction. mx check then answered +-- [CE7247] "Variable 'Cursor' does not have a primitive type." +-- Mendix has no action that reassigns an object variable: Change variable +-- takes only a primitive, and Change list Replace (ako/mxcli#949) only a list. +-- `check` (MDL-SET01), `check --references` and `exec` now refuse it, naming +-- the alternatives; 1323-set-object-variable.fail.mdl is the refusal. +-- +-- This file holds the forms that DO build — each measured on 11.14.0 with +-- mx check reporting 0 errors. +-- ============================================================================ + +create module F1323; +create persistent entity F1323.Node ("Name": String(50)); +create association F1323.Node_Parent from F1323.Node to F1323.Node type Reference; + +-- 1. The workaround for a chain walk: recursion in a sub-microflow that +-- RETURNS the next object, instead of reassigning a cursor variable. +create microflow F1323.GET_Root ($Node: F1323.Node) returns F1323.Node as $Root +begin + retrieve $Parent from $Node/F1323.Node_Parent; + if $Parent = empty then + return $Node; + else + $Root = call microflow F1323.GET_Root(Node = $Parent); + return $Root; + end if; +end; + +-- 2. A self-association retrieve is typed as a LIST by Mendix, so `set` on it +-- is a Change list Replace (ako/mxcli#949) and stays accepted. +create microflow F1323.ReplaceList ($Start: F1323.Node, $Other: F1323.Node) +begin + retrieve $Cursor from $Start/F1323.Node_Parent; + retrieve $Next from $Other/F1323.Node_Parent; + set $Cursor = $Next; +end; diff --git a/mdl-examples/bug-tests/1324-own-data-container-name.fail.mdl b/mdl-examples/bug-tests/1324-own-data-container-name.fail.mdl new file mode 100644 index 0000000000..eefd5f6326 --- /dev/null +++ b/mdl-examples/bug-tests/1324-own-data-container-name.fail.mdl @@ -0,0 +1,33 @@ +-- mendixlabs/mxcli#1324: a button passing its OWN data container by widget name +-- +-- NEGATIVE TEST (.fail.mdl) — EXPECTED to fail `mxcli check`. +-- `make check-mdl` inverts the exit code: an unexpected pass is a regression of +-- the MDL-BUTTON02 rule. +-- +-- Symptom: `check --references` and `exec` were clean, then `mx check` reported +-- [error] [CE0117] "Error(s) in expression." at Action button 'btnOwn' +-- A data container's widget name ($dvGate) is a variable only for the data +-- containers nested below it. Directly inside dvGate (a plain container is +-- transparent) the object is $currentObject. The same holds for a list view, +-- gallery or data grid read by its own name from its item or row, and for a +-- control bar, which takes its context from above its grid. +-- +-- Fix: `mxcli check` flags it as MDL-BUTTON02 (error), so exec refuses it. +-- btnCur and btnOuter build clean (measured, Mendix 11.14.0) and are not flagged. + +create module G52; +create persistent entity G52.Gate (Name: String(50)); +create microflow G52.ACT_Open ($Gate: G52.Gate) +begin + log info node 'G52' 'open'; +end; + +create page G52.GatePage (Title: 'Gate', Layout: Atlas_Core.PopupLayout, Params: ( $Gate: G52.Gate )) { + dataview dvGate (DataSource: $Gate) { + actionbutton btnOwn (Caption: 'Own data view name', Action: call microflow G52.ACT_Open(Gate = $dvGate)) + actionbutton btnCur (Caption: 'Current object', Action: call microflow G52.ACT_Open(Gate = $currentObject)) + dataview dvInner (DataSource: $Gate) { + actionbutton btnOuter (Caption: 'Enclosing data view name', Action: call microflow G52.ACT_Open(Gate = $dvGate)) + } + } +}; diff --git a/mdl-examples/bug-tests/1326-xpath-function-argument-operator.fail.mdl b/mdl-examples/bug-tests/1326-xpath-function-argument-operator.fail.mdl new file mode 100644 index 0000000000..5a13185c0d --- /dev/null +++ b/mdl-examples/bug-tests/1326-xpath-function-argument-operator.fail.mdl @@ -0,0 +1,38 @@ +-- Issue mendixlabs/mxcli#1326: an operator inside an XPath function argument. +-- +-- `mxcli check mdl-examples/bug-tests/1326-xpath-function-argument-operator.fail.mdl` +-- reports MDL091 on SUB_PlusInFunc, and exec refuses it. Before the fix it passed +-- check and exec, and mxbuild 11.14.0 rejected it: CE0161 "Error(s) in XPath +-- constraint." The two microflows after it are the controls, 0 errors on the +-- same mxbuild, and must stay clean. + +create or modify module G1326; + +create or modify persistent entity G1326.Item ( + Name: String(50) +); + +-- MDL091: the concatenation is inside the starts-with() argument. +create or modify microflow G1326.SUB_PlusInFunc ($Key: String) returns Integer as $N +begin + retrieve $L from G1326.Item where starts-with(Name, 'MS-' + $Key); + declare $N Integer = length($L); + return $N; +end; + +-- Clean: the same operator at the top level of the constraint. +create or modify microflow G1326.SUB_PlusTopLevel ($Key: String) returns Integer as $N +begin + retrieve $L from G1326.Item where Name = 'X-' + $Key; + declare $N Integer = length($L); + return $N; +end; + +-- Clean: the value computed beforehand — the fix MDL091 suggests. +create or modify microflow G1326.SUB_PreComputed ($Key: String) returns Integer as $N +begin + declare $P String = 'MS-' + $Key; + retrieve $L from G1326.Item where starts-with(Name, $P); + declare $N Integer = length($L); + return $N; +end; diff --git a/mdl-examples/bug-tests/rename-java-action-renames-class.mdl b/mdl-examples/bug-tests/rename-java-action-renames-class.mdl new file mode 100644 index 0000000000..fe6ed81eca --- /dev/null +++ b/mdl-examples/bug-tests/rename-java-action-renames-class.mdl @@ -0,0 +1,33 @@ +mdl 1; +-- ============================================================================ +-- RENAME JAVA ACTION left `public class JA_Old` inside JA_New.java +-- ============================================================================ +-- +-- The rename moved javasource//actions/JA_Old.java to JA_New.java and +-- left the class, its constructor and toString's "JA_Old" as they were. javac: +-- class JA_Old is public, should be declared in a file named JA_Old.java +-- `mxcli docker build` still succeeded — mxbuild regenerates the stub in its +-- copy — so only the file on disk was wrong, which is the one `run --local +-- --watch` hot reload and an IDE compile. +-- +-- The file on disk is not something `mxcli check` can see, so the assertions +-- are Go tests: sdk/javaactions/rename_test.go (against a golden mxbuild wrote) +-- and mdl/backend/modelsdk/java_rename_source_test.go. This script is the +-- reproduction: exec it, then javac javasource/bugrenameja/actions/JA_New.java +-- against runtime/bundles/com.mendix.public-api.jar. +-- ============================================================================ + +CREATE MODULE BugRenameJa; + +CREATE JAVA ACTION BugRenameJa.JA_Old(Text: String NOT NULL) RETURNS String +AS $$ +return Text.trim(); +$$; + +CREATE OR MODIFY MICROFLOW BugRenameJa.ACT_UsesJava ($t: String) RETURNS String +BEGIN + $r = CALL JAVA ACTION BugRenameJa.JA_Old(Text = $t); + RETURN $r; +END; + +RENAME JAVA ACTION BugRenameJa.JA_Old TO JA_New; diff --git a/mdl-examples/bug-tests/set-on-parameter.fail.mdl b/mdl-examples/bug-tests/set-on-parameter.fail.mdl new file mode 100644 index 0000000000..b974e9bd39 --- /dev/null +++ b/mdl-examples/bug-tests/set-on-parameter.fail.mdl @@ -0,0 +1,21 @@ +mdl 1; +-- ============================================================================ +-- MDL-SET01 (the refusal): `set` on a PARAMETER, any type but a list +-- ============================================================================ +-- +-- A Change variable activity cannot target a parameter. Measured on 11.14.0, +-- an Integer or String parameter set in a microflow, a nanoflow or a rule +-- builds as CE7247 "Parameter 'N' cannot be changed." — check and exec passed +-- it before (found while fixing mendixlabs/mxcli#1323, which covered object +-- variables). See set-on-parameter.mdl for the shapes that build. +-- +-- This file is expected to FAIL `mxcli check`. +-- ============================================================================ + +create module FSP; + +create microflow FSP.Increment ($N: Integer) returns Integer +begin + set $N = $N + 1; + return $N; +end; diff --git a/mdl-examples/bug-tests/set-on-parameter.mdl b/mdl-examples/bug-tests/set-on-parameter.mdl new file mode 100644 index 0000000000..09b58d0966 --- /dev/null +++ b/mdl-examples/bug-tests/set-on-parameter.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- MDL-SET01 (the controls): the parameter shapes mxbuild accepts +-- ============================================================================ +-- +-- Companion to set-on-parameter.fail.mdl. Each of these built with 0 errors on +-- 11.14.0: copying a primitive parameter into a variable and changing that, +-- a Change list Replace (`set`) and an `add` on a LIST parameter, and a member +-- change on an object parameter. +-- ============================================================================ + +create module FSPOK; +create persistent entity FSPOK.E ("Name": String(50)); + +create microflow FSPOK.Increment ($N: Integer) returns Integer +begin + declare $Value Integer = $N; + set $Value = $Value + 1; + return $Value; +end; + +create microflow FSPOK.ListParams ($L: list of FSPOK.E, $M: list of FSPOK.E, $O: FSPOK.E) +begin + set $L = $M; + add $O to $L; + set $O/Name = 'x'; +end; diff --git a/mdl/backend/modelsdk/java_rename_source_test.go b/mdl/backend/modelsdk/java_rename_source_test.go new file mode 100644 index 0000000000..57a426eccf --- /dev/null +++ b/mdl/backend/modelsdk/java_rename_source_test.go @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// RenameJavaSourceFile moved Old.java to New.java and left `public class Old` +// inside it — not valid Java, which `run --local --watch` hot reload and every +// IDE compile from disk. The class, constructor and toString must follow the +// file; the user code, mentions of the old name included, must not. +func TestRenameJavaSourceFileRenamesTheClass(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "javasource", "sales", "actions") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + src := "package sales.actions;\r\n\r\n" + + "public class JA_Old extends UserAction\r\n{\r\n" + + "\tpublic JA_Old(\r\n\t\tIContext context\r\n\t)\r\n\t{\r\n\t\tsuper(context);\r\n\t}\r\n\r\n" + + "\t@java.lang.Override\r\n\tpublic java.lang.String executeAction() throws Exception\r\n\t{\r\n" + + "\t\t// BEGIN USER CODE\r\n\t\treturn \"JA_Old\";\r\n\t\t// END USER CODE\r\n\t}\r\n\r\n" + + "\t@java.lang.Override\r\n\tpublic java.lang.String toString()\r\n\t{\r\n\t\treturn \"JA_Old\";\r\n\t}\r\n}\r\n" + if err := os.WriteFile(filepath.Join(dir, "JA_Old.java"), []byte(src), 0o644); err != nil { + t.Fatal(err) + } + + b := &Backend{path: filepath.Join(root, "App.mpr")} + if err := b.RenameJavaSourceFile("Sales", "JA_Old", "JA_New"); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(filepath.Join(dir, "JA_Old.java")); !os.IsNotExist(err) { + t.Errorf("JA_Old.java still exists (stat err %v)", err) + } + got, err := os.ReadFile(filepath.Join(dir, "JA_New.java")) + if err != nil { + t.Fatal(err) + } + want := strings.NewReplacer( + "public class JA_Old", "public class JA_New", + "public JA_Old(", "public JA_New(", + "\t\treturn \"JA_Old\";\r\n\t}\r\n}", "\t\treturn \"JA_New\";\r\n\t}\r\n}", + ).Replace(src) + if string(got) != want { + t.Errorf("JA_New.java:\n%s\nwant:\n%s", got, want) + } + if !strings.Contains(string(got), "// BEGIN USER CODE\r\n\t\treturn \"JA_Old\";") { + t.Error("the user code's mention of JA_Old was rewritten; mxbuild keeps it") + } +} + +// A missing stub is still not an error: the action may never have had one. +func TestRenameJavaSourceFileMissingIsNotAnError(t *testing.T) { + b := &Backend{path: filepath.Join(t.TempDir(), "App.mpr")} + if err := b.RenameJavaSourceFile("Sales", "JA_Old", "JA_New"); err != nil { + t.Errorf("missing source: %v", err) + } +} diff --git a/mdl/backend/modelsdk/java_write.go b/mdl/backend/modelsdk/java_write.go index 8a36d3c68f..f1b5bb8cc4 100644 --- a/mdl/backend/modelsdk/java_write.go +++ b/mdl/backend/modelsdk/java_write.go @@ -125,7 +125,11 @@ func (b *Backend) DeleteJavaAction(id model.ID) error { return b.writer.DeleteUnit(string(id)) } -// RenameJavaSourceFile renames javasource//actions/.java to .java. +// RenameJavaSourceFile renames javasource//actions/.java to +// .java and renames the class inside it, as mxbuild's regeneration does: +// moving the file alone left `public class Old` in New.java, which is not valid +// Java — a full build hides it by regenerating the stub in its own copy, but +// `run --local --watch` hot reload and every IDE compile the file on disk. // A missing source file is not an error (the action may have no generated stub yet). func (b *Backend) RenameJavaSourceFile(moduleName, oldName, newName string) error { if b.path == "" { @@ -134,9 +138,23 @@ func (b *Backend) RenameJavaSourceFile(moduleName, oldName, newName string) erro dir := filepath.Join(filepath.Dir(b.path), "javasource", strings.ToLower(moduleName), "actions") oldPath := filepath.Join(dir, oldName+".java") newPath := filepath.Join(dir, newName+".java") - if err := os.Rename(oldPath, newPath); err != nil && !os.IsNotExist(err) { + content, err := os.ReadFile(oldPath) + if os.IsNotExist(err) { + return nil + } + if err != nil { return fmt.Errorf("RenameJavaSourceFile: %w", err) } + // Move first, then rewrite in place, so the file keeps its mode and a + // failed rewrite still leaves the source where the model now expects it. + if err := os.Rename(oldPath, newPath); err != nil { + return fmt.Errorf("RenameJavaSourceFile: %w", err) + } + // The move is a write whether or not the text needed rewriting. + b.noteFileWrite(true) + if _, err := javaactions.WriteSourceIfChanged(newPath, javaactions.RenameSource(string(content), oldName, newName)); err != nil { + return fmt.Errorf("RenameJavaSourceFile: rewrite class name: %w", err) + } return nil } diff --git a/mdl/backend/modelsdk/widget_flow_param_null_variable_test.go b/mdl/backend/modelsdk/widget_flow_param_null_variable_test.go new file mode 100644 index 0000000000..d8d2883ccc --- /dev/null +++ b/mdl/backend/modelsdk/widget_flow_param_null_variable_test.go @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "sort" + "strings" + "testing" + + bsonv1 "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// mendixlabs/mxcli#1317 — a flow argument bound through Expression (a literal, +// $currentObject, a path, a primitive parameter) was written WITHOUT the +// Variable key. Studio Pro always writes all three keys and nulls the unused +// slot, so opening the change in the Changes panel crashed with +// +// Objects with ID … of type Forms$MicroflowParameterMapping do not have the +// same properties. baseNames = Expression, Parameter, Variable; +// newNames = Parameter, Expression +// +// mx check and mxbuild are both clean on it; only the Changes-panel diff sees +// the key set. Same class as #1180 (DomainModels$NoGeneralization). + +// mappingKeys returns the non-$ keys of a parameter mapping, sorted. +func mappingKeys(pm bsonv1.D) []string { + var keys []string + for _, e := range pm { + if !strings.HasPrefix(e.Key, "$") { + keys = append(keys, e.Key) + } + } + sort.Strings(keys) + return keys +} + +// assertStudioProMappingShape checks the key set Studio Pro writes for a +// Forms$MicroflowParameterMapping / Forms$NanoflowParameterMapping at 11.12.2 +// (the baseNames of the reported crash), with Variable null when unused. +func assertStudioProMappingShape(t *testing.T, pm bsonv1.D) { + t.Helper() + got := strings.Join(mappingKeys(pm), ", ") + if want := "Expression, Parameter, Variable"; got != want { + t.Fatalf("mapping keys = %s; Studio Pro writes %s — the Changes panel "+ + "throws \"do not have the same properties\" on the difference", got, want) + } + if v := docGet(pm, "Variable"); v != nil { + t.Errorf("Variable = %#v, want null for an Expression-bound argument", v) + } +} + +func TestMicroflowActionExpressionArgWritesNullVariable(t *testing.T) { + a := &pages.MicroflowClientAction{ + MicroflowName: "LearningJourney.ACT_ExcludeItemStep", + ParameterMappings: []*pages.MicroflowParameterMapping{ + {ParameterName: "LearningJourney", Expression: "$LearningJourney"}, + }, + } + // The reported path: a pluggable widget's named action slot, serialized + // through the child serializer the page mutator calls. + doc := codecChildSerializer{}.SerializeClientAction(a) + settings, ok := docGet(doc, "MicroflowSettings").(bsonv1.D) + if !ok { + t.Fatalf("MicroflowSettings missing: %#v", doc) + } + assertStudioProMappingShape(t, firstParamMapping(t, settings)) +} + +func TestNanoflowActionExpressionArgWritesNullVariable(t *testing.T) { + a := &pages.NanoflowClientAction{ + NanoflowName: "MyModule.ACT_Toggle_NF", + ParameterMappings: []*pages.NanoflowParameterMapping{ + {ParameterName: "Flag", Expression: "true"}, + }, + } + assertStudioProMappingShape(t, firstParamMapping(t, encodeAction(t, a))) +} + +// Control: the Variable-bound form already wrote all three keys (#1140) and +// must keep its PageVariable rather than be nulled by the default. +func TestMicroflowActionVariableArgKeepsPageVariable(t *testing.T) { + a := &pages.MicroflowClientAction{ + MicroflowName: "LearningJourney.ACT_ExcludeItemStep", + ParameterMappings: []*pages.MicroflowParameterMapping{ + {ParameterName: "LearningJourney", Variable: "$LearningJourney", VariableKind: "parameter"}, + }, + } + settings, _ := docGet(encodeAction(t, a), "MicroflowSettings").(bsonv1.D) + pm := firstParamMapping(t, settings) + if got := strings.Join(mappingKeys(pm), ", "); got != "Expression, Parameter, Variable" { + t.Fatalf("mapping keys = %s", got) + } + if paramVariable(pm) == nil { + t.Fatalf("Variable = %#v, want a Forms$PageVariable", docGet(pm, "Variable")) + } +} diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index 9b6ebdeb26..e6a03b2aae 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -297,6 +297,18 @@ func init() { codec.RegisterListMarker("Forms$MicroflowParameterMapping", 2) codec.RegisterListMarker("Forms$PageParameterMapping", 2) codec.RegisterListMarker("Forms$SnippetParameterMapping", 2) + // A flow argument fills one of two slots and Studio Pro writes both. The + // Variable side already writes Expression "" (bindParameterMappingValue); + // an Expression-bound argument left Variable unset, so the key was omitted + // and Studio Pro's Changes panel threw "do not have the same properties. + // baseNames = Expression, Parameter, Variable; newNames = Parameter, + // Expression" (mendixlabs/mxcli#1317, the #1180 class). + codec.RegisterTypeDefaults("Forms$MicroflowParameterMapping", codec.TypeDefaults{ + NullFields: []string{"Variable"}, + }) + codec.RegisterTypeDefaults("Forms$NanoflowParameterMapping", codec.TypeDefaults{ + NullFields: []string{"Variable"}, + }) codec.RegisterTypeDefaults("Forms$SnippetCall", codec.TypeDefaults{ MandatoryListMarkers: map[string]int32{"ParameterMappings": 2}, }) diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 90ddb441b1..4498bacc79 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -271,7 +271,7 @@ func applySetPropertyMutator(ctx *ExecContext, mutator backend.PageMutator, op * } else if propName == "Action" { // Action is a polymorphic node, not a scalar — it goes through the // same builder CREATE PAGE uses rather than being written as a value. - action, err := convertASTAction(ctx, value, moduleName, moduleID) + action, err := convertASTAction(ctx, mutator, value, moduleName, moduleID) if err != nil { return err } @@ -284,7 +284,7 @@ func applySetPropertyMutator(ctx *ExecContext, mutator backend.PageMutator, op * // Same builder as `Action`; the mutator checks the key is an // action-typed property of the stored widget. Through // SetWidgetProperty it would be stringified into a PrimitiveValue. - action, err := convertASTAction(ctx, value, moduleName, moduleID) + action, err := convertASTAction(ctx, mutator, value, moduleName, moduleID) if err != nil { return err } @@ -363,21 +363,31 @@ func convertASTDataSource(value interface{}) (pages.DataSource, error) { // alternative is the failure mode #855 documents for DataSource, where SET // carried a narrower vocabulary than REPLACE and each missing case surfaced as // its own bug report. -func convertASTAction(ctx *ExecContext, value any, moduleName string, moduleID model.ID) (pages.ClientAction, error) { +// +// The builder is seeded with the stored document's parameters and variables, as +// buildWidgetsFromAST is. Without them a `$Param` argument cannot be told from an +// expression and is written as one, which Studio Pro does not bind +// (mendixlabs/mxcli#1317, the #1140 rule). +func convertASTAction(ctx *ExecContext, mutator backend.PageMutator, value any, moduleName string, moduleID model.ID) (pages.ClientAction, error) { action, ok := value.(*ast.ActionV3) if !ok { return nil, mdlerrors.NewValidation("Action value must be an action expression, " + "for example `set (Action: microflow Module.MF) on btnSave`") } + paramScope, paramEntityNames := mutator.ParamScope() pb := &pageBuilder{ - ctx: ctx, - backend: ctx.Backend, - moduleID: moduleID, - moduleName: moduleName, - execCache: ctx.Cache, - fragments: ctx.Fragments, - themeRegistry: ctx.GetThemeRegistry(), - widgetBackend: ctx.Backend, + ctx: ctx, + backend: ctx.Backend, + moduleID: moduleID, + moduleName: moduleName, + paramScope: paramScope, + paramEntityNames: paramEntityNames, + execCache: ctx.Cache, + fragments: ctx.Fragments, + themeRegistry: ctx.GetThemeRegistry(), + widgetBackend: ctx.Backend, + localVariables: storedPageVariables(mutator), + isSnippet: mutator.ContainerType() == backend.ContainerSnippet, } return pb.buildClientActionV3(action) } @@ -788,6 +798,7 @@ func buildColumnSpecsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleNa themeRegistry: ctx.GetThemeRegistry(), widgetBackend: ctx.Backend, localVariables: storedPageVariables(mutator), + isSnippet: mutator.ContainerType() == backend.ContainerSnippet, } var result []*backend.DataGridColumnSpec @@ -882,6 +893,7 @@ func buildWidgetsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleName s themeRegistry: ctx.GetThemeRegistry(), widgetBackend: ctx.Backend, localVariables: storedPageVariables(mutator), + isSnippet: mutator.ContainerType() == backend.ContainerSnippet, } var result []pages.Widget diff --git a/mdl/executor/cmd_alter_page_action_test.go b/mdl/executor/cmd_alter_page_action_test.go index 93b0a76ad8..52f30cae54 100644 --- a/mdl/executor/cmd_alter_page_action_test.go +++ b/mdl/executor/cmd_alter_page_action_test.go @@ -171,3 +171,139 @@ func TestAlterPage_SetAction_RejectsNonAction(t *testing.T) { assertError(t, err) assertContainsStr(t, err.Error(), "must be an action expression") } + +// mendixlabs/mxcli#1317 — `alter page … set ('onClickAction': call microflow +// M.F(P = $P)) on w` built the action with an empty parameter scope, so a page +// parameter argument fell through classifyFlowArgValue and was written as the +// Expression "$P" — the form #1140 showed Studio Pro does not bind (CE1571 on +// opening the page). CREATE PAGE and ALTER PAGE INSERT/REPLACE already seed the +// scope from the stored page; SET must too. +func TestAlterPage_SetNamedAction_PageParamBindsThroughVariable(t *testing.T) { + for _, tc := range []struct { + name string + container backend.ContainerKind + wantKind string + }{ + {"page", backend.ContainerPage, "parameter"}, + {"snippet", backend.ContainerSnippet, "snippet"}, + } { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("LearningJourney") + pg := mkPage(mod.ID, "LearningJourney_Details") + mf := mkMicroflow(mod.ID, "ACT_ExcludeItemStep") + var got pages.ClientAction + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + OpenPageForMutationFunc: func(unitID model.ID) (backend.PageMutator, error) { + return &mock.MockPageMutator{ + ContainerTypeFunc: func() backend.ContainerKind { return tc.container }, + ParamScopeFunc: func() (map[string]model.ID, map[string]string) { + return map[string]model.ID{"LearningJourney": "p1"}, + map[string]string{"LearningJourney": "LearningJourney.LearningJourney"} + }, + SetWidgetNamedActionFunc: func(widgetRef, key string, action pages.ClientAction) error { + got = action + return nil + }, + SaveFunc: func() error { return nil }, + }, nil + }, + } + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + withContainer(h, mf.ContainerID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + assertNoError(t, execAlterPage(ctx, &ast.AlterPageStmt{ + PageName: ast.QualifiedName{Module: "LearningJourney", Name: "LearningJourney_Details"}, + Operations: []ast.AlterPageOperation{ + &ast.SetPropertyOp{ + Target: ast.WidgetRef{Widget: "pDSLink_ButtonAndTracking8"}, + Properties: map[string]any{"onClickAction": &ast.ActionV3{ + Type: "microflow", + Target: "LearningJourney.ACT_ExcludeItemStep", + Args: []ast.FlowArgV3{{Name: "LearningJourney", Value: "$LearningJourney"}}, + }}, + }, + }, + })) + + mfa, ok := got.(*pages.MicroflowClientAction) + if !ok { + t.Fatalf("action = %T, want *pages.MicroflowClientAction", got) + } + if len(mfa.ParameterMappings) != 1 { + t.Fatalf("mappings = %d, want 1", len(mfa.ParameterMappings)) + } + pm := mfa.ParameterMappings[0] + if pm.VariableKind != tc.wantKind || pm.Variable != "$LearningJourney" { + t.Errorf("mapping = Variable %q kind %q, want $LearningJourney kind %q — "+ + "an unclassified $-argument is written as an Expression Studio Pro does not bind", + pm.Variable, pm.VariableKind, tc.wantKind) + } + }) + } +} + +// The INSERT/REPLACE half of #1317's scope fix. buildWidgetsFromAST and +// buildColumnSpecsFromAST seeded the parameter scope but never isSnippet, so a +// button inserted into a snippet bound its `$Order` argument as a PageParameter. +// mxbuild 11.12.2 reports it as CE0115 ("arguments … do not match the expected +// parameters") on the inserted button only; CREATE SNIPPET and ALTER SET bind +// the same argument as SnippetParameter. +func TestAlterSnippet_InsertedFlowArgBindsSnippetParameter(t *testing.T) { + mod := mkModule("SN") + mf := mkMicroflow(mod.ID, "ACT_Use") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + } + h := mkHierarchy(mod) + withContainer(h, mf.ContainerID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + for _, tc := range []struct { + container backend.ContainerKind + wantKind string + }{ + {backend.ContainerSnippet, "snippet"}, + {backend.ContainerPage, "parameter"}, // control: a page keeps PageParameter + } { + t.Run(string(tc.container), func(t *testing.T) { + mut := &mock.MockPageMutator{ + ContainerTypeFunc: func() backend.ContainerKind { return tc.container }, + ParamScopeFunc: func() (map[string]model.ID, map[string]string) { + return map[string]model.ID{"Order": "p1"}, map[string]string{"Order": "SN.Order"} + }, + WidgetScopeFunc: func() map[string]model.ID { return map[string]model.ID{} }, + } + btn := &ast.WidgetV3{Type: "actionbutton", Name: "btnInsert", Properties: map[string]any{ + "Caption": "Insert", + "Action": &ast.ActionV3{Type: "microflow", Target: "SN.ACT_Use", + Args: []ast.FlowArgV3{{Name: "Order", Value: "$Order"}}}, + }} + ws, err := buildWidgetsFromAST(ctx, []*ast.WidgetV3{btn}, "SN", mod.ID, "", mut) + assertNoError(t, err) + if len(ws) != 1 { + t.Fatalf("widgets = %d, want 1", len(ws)) + } + b, ok := ws[0].(*pages.ActionButton) + if !ok { + t.Fatalf("widget = %T, want *pages.ActionButton", ws[0]) + } + mfa, ok := b.Action.(*pages.MicroflowClientAction) + if !ok || len(mfa.ParameterMappings) != 1 { + t.Fatalf("action = %#v, want one microflow mapping", b.Action) + } + if got := mfa.ParameterMappings[0].VariableKind; got != tc.wantKind { + t.Errorf("VariableKind = %q, want %q — mxbuild reports CE0115 on the button", got, tc.wantKind) + } + }) + } +} diff --git a/mdl/executor/cmd_constant_boolean_default_test.go b/mdl/executor/cmd_constant_boolean_default_test.go new file mode 100644 index 0000000000..cce4c426f4 --- /dev/null +++ b/mdl/executor/cmd_constant_boolean_default_test.go @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" +) + +// A Boolean constant's DefaultValue is stored as "True" / "False" — that is what +// Studio Pro writes, and its constant dialog reads a stored "true" as False while +// the runtime reads it as true (mendixlabs/mxcli#1321). The literal `true` reached +// the backend as Go's fmt rendering "true"; only the quoted 'True' was right. + +var booleanConstantDefaults = []struct { + name, mdl, want string +}{ + {"lower true", "true", "True"}, + {"title True", "True", "True"}, + {"lower false", "false", "False"}, + {"upper FALSE", "FALSE", "False"}, + {"quoted lower", "'true'", "True"}, + // Already in Studio Pro's form: the control. + {"quoted True", "'True'", "True"}, +} + +func TestCreateConstant_BooleanDefaultStoredAsStudioProCase(t *testing.T) { + for _, tc := range booleanConstantDefaults { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("MyModule") + var captured *model.Constant + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListConstantsFunc: func() ([]*model.Constant, error) { return nil, nil }, + CreateConstantFunc: func(c *model.Constant) error { + captured = c + return nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + prog := parseMDL(t, "create constant MyModule.Flag (Type: Boolean, DefaultValue: "+tc.mdl+");") + if err := createConstant(ctx, prog.Statements[0].(*ast.CreateConstantStmt)); err != nil { + t.Fatalf("createConstant: %v", err) + } + if captured == nil { + t.Fatal("CreateConstant was not called") + } + if captured.DefaultValue != tc.want { + t.Errorf("DefaultValue: %s stored as %q, want %q", tc.mdl, captured.DefaultValue, tc.want) + } + }) + } +} + +// `create or modify` rewrites an existing constant through UpdateConstant — the +// second write path the same value takes. +func TestCreateOrModifyConstant_BooleanDefaultStoredAsStudioProCase(t *testing.T) { + mod := mkModule("MyModule") + existing := &model.Constant{ + ContainerID: mod.ID, + Name: "Flag", + Type: model.ConstantDataType{Kind: "Boolean"}, + DefaultValue: "False", + } + var captured *model.Constant + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListConstantsFunc: func() ([]*model.Constant, error) { return []*model.Constant{existing}, nil }, + UpdateConstantFunc: func(c *model.Constant) error { + captured = c + return nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + prog := parseMDL(t, "create or modify constant MyModule.Flag (Type: Boolean, DefaultValue: true);") + _ = createConstant(ctx, prog.Statements[0].(*ast.CreateConstantStmt)) // folder handling may warn; the write is under test + if captured == nil { + t.Fatal("UpdateConstant was not called") + } + if captured.DefaultValue != "True" { + t.Errorf("DefaultValue: true stored as %q, want %q", captured.DefaultValue, "True") + } +} + +// A configuration override of a Boolean constant is stored the same way as its +// default, and `alter settings constant @M.Flag value true` wrote the token text +// "true" verbatim — the #1321 defect on the second write path. Only a Boolean +// constant is normalised: a String constant holding the text 'true' is left alone. +func TestAlterSettingsConstant_BooleanValueStoredAsStudioProCase(t *testing.T) { + cases := []struct { + name, constantID, value, want string + existing bool + }{ + {"new override true", "Mod.Flag", "true", "True", false}, + {"new override FALSE", "Mod.Flag", "FALSE", "False", false}, + {"existing override true", "Mod.Flag", "true", "True", true}, + // Already in Studio Pro's form: the control. + {"already True", "Mod.Flag", "True", "True", false}, + // Not a Boolean constant: must pass through unchanged. + {"string constant", "Mod.Label", "true", "true", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mod := mkModule("Mod") + cfg := &model.ServerConfiguration{Name: "Default"} + if tc.existing { + cfg.ConstantValues = []*model.ConstantValue{{ConstantId: tc.constantID, Value: "False"}} + } + var wrote *model.ProjectSettings + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListConstantsFunc: func() ([]*model.Constant, error) { + return []*model.Constant{ + {ContainerID: mod.ID, Name: "Flag", Type: model.ConstantDataType{Kind: "Boolean"}}, + {ContainerID: mod.ID, Name: "Label", Type: model.ConstantDataType{Kind: "String"}}, + }, nil + }, + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { + ps := &model.ProjectSettings{ + Configuration: &model.ConfigurationSettings{ + Configurations: []*model.ServerConfiguration{cfg}, + }, + } + ps.RawParts = []map[string]any{{"$Type": "Settings$ConfigurationSettings"}} + return ps, nil + }, + UpdateProjectSettingsFunc: func(ps *model.ProjectSettings) error { + wrote = ps + return nil + }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + if err := alterSettings(ctx, &ast.AlterSettingsStmt{ + Section: "constant", + ConfigName: "Default", + ConstantId: tc.constantID, + Value: tc.value, + }); err != nil { + t.Fatalf("alterSettings: %v", err) + } + if wrote == nil { + t.Fatal("UpdateProjectSettings was not called") + } + var got *model.ConstantValue + for _, cv := range wrote.Configuration.Configurations[0].ConstantValues { + if cv.ConstantId == tc.constantID { + got = cv + } + } + if got == nil { + t.Fatalf("no override for %s written", tc.constantID) + } + if got.Value != tc.want { + t.Errorf("value %s stored as %q, want %q", tc.value, got.Value, tc.want) + } + }) + } +} diff --git a/mdl/executor/cmd_constants.go b/mdl/executor/cmd_constants.go index 5cad44f591..abbfcdb4e5 100644 --- a/mdl/executor/cmd_constants.go +++ b/mdl/executor/cmd_constants.go @@ -264,6 +264,22 @@ func formatDefaultValue(dt model.ConstantDataType, value string) string { } } +// storedConstantDefault puts a default value into the form Studio Pro stores. A +// Boolean is "True" / "False": Studio Pro's constant dialog reads a stored "true" as +// False while the runtime reads it as true, so the literal `true` (rendered by fmt +// as "true") made the app run with a value the developer never sees (#1321). +func storedConstantDefault(dt model.ConstantDataType, value string) string { + if dt.Kind == "Boolean" { + switch { + case strings.EqualFold(value, "true"): + return "True" + case strings.EqualFold(value, "false"): + return "False" + } + } + return value +} + // createConstant handles CREATE CONSTANT command. func createConstant(ctx *ExecContext, stmt *ast.CreateConstantStmt) error { if !ctx.ConnectedForWrite() { @@ -289,6 +305,7 @@ func createConstant(ctx *ExecContext, stmt *ast.CreateConstantStmt) error { if stmt.DefaultValue != nil { defaultValue = fmt.Sprintf("%v", stmt.DefaultValue) } + defaultValue = storedConstantDefault(constType, defaultValue) // Check if constant already exists in this module existingConstants, err := ctx.Backend.ListConstants() diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index dc96d1f1cd..bf6e62e1f3 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -23,6 +23,15 @@ type flowBuilder struct { // validator scoped names per branch and counted every call output, so it // was wrong both ways. Rules keep it — MDL063 does not run on them. duplicateNamesOwnedElsewhere bool + // checkAssocShapes is check --references' view of the project's and the + // script's associations, so the validator types a forward Reference + // retrieve as the object it yields (validateFlowBody). nil on the exec path, + // which resolves associations through the backend instead. + checkAssocShapes map[string]assocShape + // assocObjectVars are the variables checkAssocShapes typed as one object, + // the only objects whose `set` the validator reports itself (MDL-SET01 + // reports the rest without a project). + assocObjectVars map[string]bool objects []microflows.MicroflowObject flows []*microflows.SequenceFlow diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index c7466830cb..5fe9c8f83f 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -2117,6 +2117,43 @@ func (fb *flowBuilder) isListVariable(name string) bool { return strings.HasPrefix(fb.varTypes[strings.TrimPrefix(name, "$")], "List of ") } +// objectVariableType returns the entity of a variable this flow knows to hold +// a single OBJECT (a parameter, a retrieve's object range or forward Reference, +// a create object, a cast, a head/find), and false for a primitive, a list, a +// member path or a variable of unknown type. +func (fb *flowBuilder) objectVariableType(name string) (string, bool) { + name = strings.TrimPrefix(name, "$") + if strings.Contains(name, "/") { + return "", false + } + if _, primitive := fb.declaredVars[name]; primitive { + return "", false + } + t := fb.varTypes[name] + if t == "" || strings.HasPrefix(t, "List of ") || !strings.Contains(t, ".") { + return "", false + } + return t, true +} + +// refuseSetOnObject reports `set $Obj = …` on an object variable. Mendix has no +// action that reassigns one: Change variable takes only a primitive, and +// mxbuild answers CE7247 "Variable 'X' does not have a primitive type" +// (mendixlabs/mxcli#1323). A list has Change list Replace (ako/mxcli#949); an +// object has nothing, so the statement is refused rather than written. +func (fb *flowBuilder) refuseSetOnObject(target string) bool { + entity, ok := fb.objectVariableType(target) + if !ok { + return false + } + name := strings.TrimPrefix(target, "$") + fb.addError("cannot set object variable '$%s' (%s): Mendix has no action that reassigns an object variable — "+ + "Change variable takes only a primitive, and mxbuild rejects it with CE7247 \"Variable '%s' does not have a primitive type\". "+ + "Return the new object from a sub-microflow instead (recursion for a chain walk), retrieve it into a new variable, "+ + "or change the object's members with `change $%s (…)`", name, entity, name, name) + return true +} + // addChangeListAction appends a Change list activity of the given operation. // eh is the statement's `on error` clause, nil for the statements that take // none; the caller finishes a custom handler. diff --git a/mdl/executor/cmd_microflows_builder_graph.go b/mdl/executor/cmd_microflows_builder_graph.go index b83567a126..90dbaf9bcc 100644 --- a/mdl/executor/cmd_microflows_builder_graph.go +++ b/mdl/executor/cmd_microflows_builder_graph.go @@ -656,6 +656,7 @@ func (fb *flowBuilder) addStatement(stmt ast.MicroflowStatement) model.ID { if fb.isListVariable(s.Target) { return fb.addReplaceListAction(s) } + fb.refuseSetOnObject(s.Target) return fb.addChangeVariableAction(s) case *ast.ReturnStmt: return fb.addEndEventWithReturn(s) diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index fc7d7d9d8f..fa34082360 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -5,6 +5,7 @@ package executor import ( "fmt" + "strings" "github.com/mendixlabs/mxcli/mdl/ast" ) @@ -12,19 +13,21 @@ import ( // ValidateMicroflowBody validates the microflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateMicroflowBody(s *ast.CreateMicroflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true) + return validateFlowBody(s.Parameters, s.Body, true, nil) } // ValidateNanoflowBody validates the nanoflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true) + return validateFlowBody(s.Parameters, s.Body, true, nil) } // validateFlowBody validates parameters and body statements for semantic errors. // duplicatesOwnedElsewhere leaves duplicate variable names to MDL063 — see -// flowBuilder.duplicateNamesOwnedElsewhere. -func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool) []string { +// flowBuilder.duplicateNamesOwnedElsewhere. assocs, when check --references has +// a project, types an association retrieve as the object or list it yields; +// nil leaves every association retrieve a list, as without a project. +func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { varTypes := make(map[string]string) declaredVars := make(map[string]string) @@ -56,6 +59,8 @@ func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement declaredVars: declaredVars, errors: []string{}, duplicateNamesOwnedElsewhere: duplicatesOwnedElsewhere, + checkAssocShapes: assocs, + assocObjectVars: map[string]bool{}, } fb.validateStatements(body) @@ -101,6 +106,11 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { fb.addErrorWithExample( fmt.Sprintf("variable '%s' is not declared", s.Target), errorExampleDeclareVariable(s.Target)) + } else if fb.assocObjectVars[strings.TrimPrefix(s.Target, "$")] { + // Only the association-retrieved object: every other object + // producer is visible without a project, and MDL-SET01 reports + // those once (checkSetOnObjectVariable). + fb.refuseSetOnObject(s.Target) } case *ast.IfStmt: @@ -308,8 +318,12 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { // Register retrieved variable if s.Variable != "" && s.Source.Module != "" { if s.StartVariable != "" { - // Association retrieve always returns a list - fb.varTypes[s.Variable] = "List of " + s.Source.Module + "." + s.Source.Name + if to, ok := fb.forwardReferenceTarget(s); ok { + fb.varTypes[s.Variable] = to + fb.assocObjectVars[s.Variable] = true + } else { + fb.varTypes[s.Variable] = "List of " + s.Source.Module + "." + s.Source.Name + } } else if s.First { fb.varTypes[s.Variable] = s.Source.Module + "." + s.Source.Name } else { @@ -397,5 +411,21 @@ func (fb *flowBuilder) validateOutputVariable(varName, statement string) { // counterpart of ValidateMicroflowBody. What a rule may not *contain* is a // separate question, answered by validateRule. func ValidateRuleBody(s *ast.CreateRuleStmt) []string { - return validateFlowBody(s.Parameters, s.Body, false) + return validateFlowBody(s.Parameters, s.Body, false, nil) +} + +// forwardReferenceTarget reports the entity a retrieve over association yields +// when that is one object: a Reference followed from its FROM entity, the same +// reading the builder makes. A self-association, a reverse traversal, a +// ReferenceSet or an association check cannot resolve stays a list, so the +// rules keyed on it never see an object that is not one. +func (fb *flowBuilder) forwardReferenceTarget(s *ast.RetrieveStmt) (string, bool) { + shape, ok := fb.checkAssocShapes[strings.ToLower(s.Source.Module+"."+s.Source.Name)] + if !ok || shape.refSet || strings.EqualFold(shape.from, shape.to) { + return "", false + } + if !strings.EqualFold(fb.varTypes[s.StartVariable], shape.from) { + return "", false + } + return shape.to, true } diff --git a/mdl/executor/cmd_rules_create.go b/mdl/executor/cmd_rules_create.go index bc37c90447..1e0f254f45 100644 --- a/mdl/executor/cmd_rules_create.go +++ b/mdl/executor/cmd_rules_create.go @@ -219,6 +219,9 @@ func execCreateRule(ctx *ExecContext, s *ast.CreateRuleStmt) error { if errMsg := validateRule(qualifiedName, s.Body, s.ReturnType); errMsg != "" { return fmt.Errorf("%s", errMsg) } + if err := validateRuleSetTargets(qualifiedName, s.Parameters, s.Body); err != nil { + return err + } // Build flow graph from body statements varTypes := make(map[string]string) diff --git a/mdl/executor/cmd_settings.go b/mdl/executor/cmd_settings.go index c706538229..8f530c071c 100644 --- a/mdl/executor/cmd_settings.go +++ b/mdl/executor/cmd_settings.go @@ -726,6 +726,31 @@ func alterSettingsConfiguration(ctx *ExecContext, ps *model.ProjectSettings, stm return nil } +// settingsConstantType returns the data type of the constant a configuration +// override names (Module.Name), or the zero type when it cannot be resolved — an +// unresolved constant's value is then stored as written. +func settingsConstantType(ctx *ExecContext, constantID string) model.ConstantDataType { + modName, name, ok := strings.Cut(constantID, ".") + if !ok { + return model.ConstantDataType{} + } + constants, err := ctx.Backend.ListConstants() + if err != nil { + return model.ConstantDataType{} + } + h, err := getHierarchy(ctx) + if err != nil { + return model.ConstantDataType{} + } + for _, c := range constants { + if strings.EqualFold(c.Name, name) && + strings.EqualFold(h.GetModuleName(h.FindModuleID(c.ContainerID)), modName) { + return c.Type + } + } + return model.ConstantDataType{} +} + func alterSettingsConstant(ctx *ExecContext, ps *model.ProjectSettings, stmt *ast.AlterSettingsStmt) error { if ps.Configuration == nil { return mdlerrors.NewNotFound("settings section", "configuration") @@ -769,6 +794,9 @@ func alterSettingsConstant(ctx *ExecContext, ps *model.ProjectSettings, stmt *as return mdlerrors.NewNotFoundMsg("constant", stmt.ConstantId, fmt.Sprintf("constant '%s' not found in configuration '%s'", stmt.ConstantId, targetConfig)) } + // A Boolean override is stored as "True" / "False", like the default (#1321). + value := storedConstantDefault(settingsConstantType(ctx, stmt.ConstantId), stmt.Value) + // Find or create the constant value found := false for _, cv := range cfg.ConstantValues { @@ -785,7 +813,7 @@ func alterSettingsConstant(ctx *ExecContext, ps *model.ProjectSettings, stmt *as "or use `alter settings drop constant @%s in configuration '%s'` to remove the override", stmt.ConstantId, targetConfig, stmt.ConstantId, targetConfig) } - cv.Value = stmt.Value + cv.Value = value found = true break } @@ -793,7 +821,7 @@ func alterSettingsConstant(ctx *ExecContext, ps *model.ProjectSettings, stmt *as if !found { cv := &model.ConstantValue{ ConstantId: stmt.ConstantId, - Value: stmt.Value, + Value: value, } cv.TypeName = "Settings$ConstantValue" cfg.ConstantValues = append(cfg.ConstantValues, cv) @@ -804,7 +832,7 @@ func alterSettingsConstant(ctx *ExecContext, ps *model.ProjectSettings, stmt *as } ctx.reportWrite(fmt.Sprintf("constant '%s' in configuration '%s'", stmt.ConstantId, targetConfig), - "Updated constant '%s' = '%s' in configuration '%s'", stmt.ConstantId, stmt.Value, targetConfig) + "Updated constant '%s' = '%s' in configuration '%s'", stmt.ConstantId, value, targetConfig) return nil } diff --git a/mdl/executor/rule_validation.go b/mdl/executor/rule_validation.go index 243f3d4aaf..6ce6fc20dd 100644 --- a/mdl/executor/rule_validation.go +++ b/mdl/executor/rule_validation.go @@ -149,3 +149,18 @@ func validateRule(name string, body []ast.MicroflowStatement, retType *ast.Micro } return errMsg.String() } + +// validateRuleSetTargets is exec's MDL-SET01 gate for a rule: a `set` on a +// rule parameter is CE7247 "Parameter 'N' cannot be changed." (measured on +// 11.14.0). check reports it through ValidateProgram, so it is not part of +// validateRule, which check --references also runs — that would print it twice. +func validateRuleSetTargets(name string, params []ast.MicroflowParam, body []ast.MicroflowStatement) error { + var msgs []string + for _, v := range setTargetViolations("rule", name, params, body) { + msgs = append(msgs, fmt.Sprintf("[%s] %s", v.RuleID, v.Message)) + } + if len(msgs) == 0 { + return nil + } + return fmt.Errorf("rule '%s' has validation errors:\n - %s", name, strings.Join(msgs, "\n - ")) +} diff --git a/mdl/executor/set_object_variable_test.go b/mdl/executor/set_object_variable_test.go new file mode 100644 index 0000000000..02a3736723 --- /dev/null +++ b/mdl/executor/set_object_variable_test.go @@ -0,0 +1,306 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// The repro from mendixlabs/mxcli#1323, verbatim: two Reference retrieves +// followed from the association's FROM entity, so both $Cursor and $Next are +// single G46.Group objects, then `set $Cursor = $Next`. It passed +// `check --references` and `exec`, and mx check answered +// [CE7247] "Variable 'Cursor' does not have a primitive type." +const setObjectRepro = `create module "G46"; +create persistent entity "G46"."Group" ("Name": String(50)); +create persistent entity "G46"."Node" ("Name": String(50)); +create association "G46"."Node_Group" from "G46"."Node" to "G46"."Group" type Reference; +create microflow "G46"."GET_OtherGroup" ($A: "G46"."Node", $B: "G46"."Node") returns "G46"."Group" as $Cursor +begin + retrieve $Cursor from $A/G46.Node_Group; + retrieve $Next from $B/G46.Node_Group; + set $Cursor = $Next; + return $Cursor; +end; +/ +` + +// checkScriptMicroflows runs check --references' per-statement validation over +// every microflow in src and returns the joined errors. +func checkScriptMicroflows(t *testing.T, src string) string { + t.Helper() + ctx := assocShapeCtx(t) + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + sc := newScriptContext() + sc.collectDefinitions(prog) + var out []string + for _, st := range prog.Statements { + if _, ok := st.(*ast.CreateMicroflowStmt); !ok { + continue + } + if err := validateWithContext(ctx, st, sc); err != nil { + out = append(out, err.Error()) + } + } + return strings.Join(out, "\n") +} + +// check --references refuses the #1323 repro, naming the CE7247 it prevents. +func TestCheckRefusesSetOnAssociationRetrievedObject(t *testing.T) { + got := checkScriptMicroflows(t, setObjectRepro) + if !strings.Contains(got, "CE7247") || !strings.Contains(got, "$Cursor") { + t.Fatalf("check accepted `set` on object variable $Cursor; got %q", got) + } +} + +// Controls for the check path: the shapes where the retrieve yields a LIST +// keep passing, because `set` on a list is a Change list Replace (ako/mxcli#949). +func TestCheckAcceptsSetOnAssociationRetrievedList(t *testing.T) { + for name, mf := range map[string]string{ + // Reference followed from its TO entity: the reverse is a list. + "reverse Reference": `create microflow C88.M ($X: C88.B, $Y: C88.B) +begin + retrieve $L from $X/C88.R_def; + retrieve $M from $Y/C88.R_def; + set $L = $M; +end; +/ +`, + // ReferenceSet from the FROM entity: a list. + "ReferenceSet": `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + retrieve $L from $X/C88.RS_def; + retrieve $M from $Y/C88.RS_def; + set $L = $M; +end; +/ +`, + } { + t.Run(name, func(t *testing.T) { + if got := checkScriptMicroflows(t, mf); got != "" { + t.Errorf("check refused `set` on a list: %s", got) + } + }) + } +} + +// The same refusal for a stored association, from the project rather than the +// script. +func TestCheckRefusesSetOnObjectVariable(t *testing.T) { + mf := `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + retrieve $O from $X/C88.R_def; + retrieve $P from $Y/C88.R_def; + set $O = $P; +end; +/ +` + if got := checkScriptMicroflows(t, mf); !strings.Contains(got, "CE7247") { + t.Errorf("check accepted `set` on an object variable; got %q", got) + } +} + +// An object plain check can see (a parameter) is MDL-SET01's to report; the +// reference validator, which check --references runs as well, stays quiet on +// it so the author sees the refusal once. +func TestCheckReferencesLeavesVisibleObjectsToMDLSET01(t *testing.T) { + mf := `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + set $X = $Y; +end; +/ +` + if got := checkScriptMicroflows(t, mf); got != "" { + t.Errorf("reference validator reported what MDL-SET01 owns: %q", got) + } + if hits := setObjectRuleHits(t, mf); len(hits) != 1 { + t.Errorf("want one %s, got %q", setObjectRule, hits) + } +} + +// exec: the builder refuses `set` on an object variable rather than writing a +// Change variable action mxbuild rejects with CE7247. +func TestSetOnObjectVariableRefusedByBuilder(t *testing.T) { + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"Cursor": "G46.Group", "Next": "G46.Group"}, + declaredVars: map[string]string{}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ + &ast.MfSetStmt{Target: "Cursor", Value: &ast.VariableExpr{Name: "Next"}}, + }, nil) + if got := strings.Join(fb.errors, "\n"); !strings.Contains(got, "CE7247") || !strings.Contains(got, "$Cursor") { + t.Fatalf("builder accepted `set` on object variable $Cursor; errors = %q", got) + } +} + +// setObjectRuleHits parses src and returns the MDL-SET01 messages plain check +// (ValidateMicroflow / ValidateNanoflow, no project) reports for it. +func setObjectRuleHits(t *testing.T, src string) []string { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var hits []string + for _, st := range prog.Statements { + var vs []linter.Violation + switch s := st.(type) { + case *ast.CreateMicroflowStmt: + vs = ValidateMicroflow(s) + case *ast.CreateNanoflowStmt: + vs = ValidateNanoflow(s) + } + for _, v := range vs { + if v.RuleID == setObjectRule { + hits = append(hits, v.Message) + } + } + } + return hits +} + +// Plain check, with no project, refuses `set` on a variable it can see is an +// object without one: the object producers that need no association lookup. +func TestPlainCheckRefusesSetOnObjectVariable(t *testing.T) { + for name, body := range map[string]string{ + "parameter": `set $A = $B;`, + "create object": `$N = create M.E (Name = 'x'); + set $N = $B;`, + "retrieve first": `retrieve $F from M.E first; + set $F = $B;`, + "loop iterator": `retrieve $L from M.E; + loop $It in $L begin + set $It = $B; + end loop;`, + "head": `retrieve $L from M.E; + $H = head($L); + set $H = $B;`, + } { + t.Run(name, func(t *testing.T) { + src := "create microflow M.MF ($A: M.E, $B: M.E)\nbegin\n " + body + "\nend;\n" + hits := setObjectRuleHits(t, src) + if len(hits) != 1 || !strings.Contains(hits[0], "CE7247") { + t.Errorf("want one %s naming CE7247, got %q", setObjectRule, hits) + } + }) + } + nf := "create nanoflow M.NF ($A: M.E, $B: M.E)\nbegin\n set $A = $B;\nend;\n" + if hits := setObjectRuleHits(t, nf); len(hits) != 1 { + t.Errorf("nanoflow: want one %s, got %q", setObjectRule, hits) + } +} + +// Controls: what plain check cannot or must not call an object stays accepted +// — a primitive, a list (Change list Replace, ako/mxcli#949), a member path, +// and an association retrieve, whose cardinality needs the project. +func TestPlainCheckAcceptsSetOnNonObject(t *testing.T) { + for name, body := range map[string]string{ + "primitive": `declare $N Integer = 0; + set $N = 1;`, + "list parameter": `set $Ls = $Ls2;`, + "list retrieve": `retrieve $L from M.E; + set $L = $Ls;`, + "member path": `set $A/Name = 'x';`, + "association retrieve": `retrieve $C from $A/M.E_Other; + retrieve $D from $B/M.E_Other; + set $C = $D;`, + } { + t.Run(name, func(t *testing.T) { + src := "create microflow M.MF ($A: M.E, $B: M.E, $Ls: list of M.E, $Ls2: list of M.E)\nbegin\n " + body + "\nend;\n" + if hits := setObjectRuleHits(t, src); len(hits) != 0 { + t.Errorf("refused a set that is not on a known object: %q", hits) + } + }) + } +} + +// mxbuild words CE7247 differently for a parameter (measured on 11.14.0), and +// the message quotes what the author will see at build time. +func TestPlainCheckQuotesParameterRejection(t *testing.T) { + hits := setObjectRuleHits(t, "create microflow M.MF ($A: M.E, $B: M.E)\nbegin\n set $A = $B;\nend;\n") + if len(hits) != 1 || !strings.Contains(hits[0], `"Parameter 'A' cannot be changed"`) { + t.Errorf("want the parameter wording of CE7247, got %q", hits) + } +} + +// A Change variable cannot target ANY parameter: measured on 11.14.0, `set` on +// an Integer or String parameter is CE7247 "Parameter 'N' cannot be changed." +// in a microflow, a nanoflow and a rule. A LIST parameter is not refused — +// `set $L = $M` is a Change list Replace, and mxbuild accepts it. +func TestCheckRefusesSetOnPrimitiveParameter(t *testing.T) { + for name, src := range map[string]string{ + "microflow Integer": "create microflow M.MF ($N: Integer)\nbegin\n set $N = 1;\nend;\n", + "microflow String": "create microflow M.MF ($S: String)\nbegin\n set $S = 'x';\nend;\n", + "nanoflow Integer": "create nanoflow M.NF ($N: Integer)\nbegin\n set $N = 1;\nend;\n", + } { + t.Run(name, func(t *testing.T) { + hits := setObjectRuleHits(t, src) + if len(hits) != 1 || !strings.Contains(hits[0], "cannot be changed") { + t.Errorf("want one %s quoting \"Parameter '…' cannot be changed\", got %q", setObjectRule, hits) + } + }) + } +} + +// Controls: the parameter shapes mxbuild accepts. +func TestCheckAcceptsSetOnListParameterAndMembers(t *testing.T) { + src := `create microflow M.MF ($L: list of M.E, $M: list of M.E, $O: M.E, $N: Integer) +begin + set $L = $M; + set $O/Name = 'x'; + declare $Local Integer = $N; + set $Local = 2; +end; +` + if hits := setObjectRuleHits(t, src); len(hits) != 0 { + t.Errorf("refused a set mxbuild accepts: %q", hits) + } +} + +// A rule never goes through ValidateMicroflow: plain check reports MDL-SET01 +// from ValidateProgram, and exec refuses it through validateRuleSetTargets. +func TestRuleRefusesSetOnParameter(t *testing.T) { + src := "create rule M.R ($N: Integer) returns Boolean\nbegin\n set $N = 1;\n return true;\nend;\n" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + r := prog.Statements[0].(*ast.CreateRuleStmt) + if err := validateRuleSetTargets(r.Name.String(), r.Parameters, r.Body); err == nil || + !strings.Contains(err.Error(), "cannot be changed") { + t.Errorf("exec's rule gate accepted `set` on a rule parameter: %v", err) + } + var plain []string + for _, v := range ValidateProgram(prog, "") { + if v.RuleID == setObjectRule { + plain = append(plain, v.Message) + } + } + if len(plain) != 1 { + t.Errorf("plain check: want one %s for the rule, got %q", setObjectRule, plain) + } +} + +// exec refuses it too: MDL-SET01 is exec-enforced, so a script that skips +// check does not write the Change variable mxbuild rejects. +func TestExecEnforcesSetOnParameter(t *testing.T) { + prog, errs := visitor.Build("create microflow M.MF ($N: Integer)\nbegin\n set $N = 1;\nend;\n") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if err := validateMicroflowRules(prog.Statements[0].(*ast.CreateMicroflowStmt)); err == nil || + !strings.Contains(err.Error(), setObjectRule) { + t.Errorf("exec's rule gate let `set` on a parameter through: %v", err) + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 747bf4cd8e..54ec83fcf9 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -694,7 +694,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := ValidateMicroflowBody(s) + validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside microflow body (pages, microflows, java actions, entities) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if len(refErrors) > 0 && s.Excluded { @@ -737,7 +737,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := ValidateNanoflowBody(s) + validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside nanoflow body (an excluded nanoflow's are warnings) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if len(refErrors) > 0 && s.Excluded { @@ -1843,6 +1843,10 @@ var execEnforcedMicroflowRules = map[string]bool{ // MDL-WF17: `lock workflow all` / `unlock workflow all` is CE1825, measured // on 11.13.0 and 11.14.0 (mendixlabs/mxcli#870). "MDL-WF17": true, + // MDL-SET01: `set` on an object variable or on a parameter is CE7247, + // measured on 11.14.0 (mendixlabs/mxcli#1323). The builder refuses an + // object itself; a primitive parameter it would write. + "MDL-SET01": true, } // validateMicroflowRules runs the MDL0xx microflow rule set (ValidateMicroflow) diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 1b07758ff4..9263a38b52 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -120,6 +120,7 @@ func (v *microflowValidator) addViolation(ruleID string, severity linter.Severit func (v *microflowValidator) validate(body []ast.MicroflowStatement) { v.checkListOperationIterator(body) v.checkRetrieveLimitOneAsList(body) + v.checkSetOnObjectVariable(v.params, body) v.checkListOperationSource(body) v.checkMergeJoinLabels(body) v.checkAnnotationLabels(body) @@ -392,6 +393,7 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { xp := expressionToXPath(stmt.Where) v.checkXPathAssociationEmpty(stmt.Variable, xp) v.checkXPathFunctionNames(stmt.Variable, xp) + v.checkXPathFunctionArgumentOperators(stmt.Variable, stmt.Where) v.checkXPathIdConstraint(stmt.Variable, xp) v.checkXPathVariableTraversal(stmt.Variable, xp) } @@ -829,6 +831,89 @@ func (v *microflowValidator) checkXPathFunctionNames(variable, xpath string) { } } +// xpathArithmeticOperators are the operators an XPath function argument cannot +// carry. Measured on mxbuild 11.14.0 (mendixlabs/mxcli#1326): +// +// starts-with(Name, 'MS-' + $Key) CE0161 "Error(s) in XPath constraint" +// not(contains(Name, $Key + $Key)) CE0161 +// contains(Name, $Key - 'a') CE0161 +// Name = 'X-' + $Key clean — the same operator at top level +// year-from-dateTime(Due) = $N - 1 clean — an operator beside a call, not in it +// starts-with(Name, $P) clean — the value computed beforehand +// +// `*`, `div` and `mod` are left out on purpose: the only argument they could +// appear in is numeric, and `contains(Name, $N)` with an Integer $N is already +// CE0161 with no operator at all, so no measurement isolates the operator. This +// rule blocks exec, so it covers only what was shown to fail. +var xpathArithmeticOperators = map[string]bool{"+": true, "-": true} + +// xpathFunctionArgumentOperators returns, for each XPath function call in a +// retrieve constraint whose argument is itself a `+` or `-` operation, +// the function name and the operator. Only an argument that IS the operation +// counts: `not(Price > $A + 1)` has a comparison for an argument and is not +// flagged — that shape was not measured. +func xpathFunctionArgumentOperators(expr ast.Expression) (hits [][2]string) { + var walk func(ast.Expression) + walk = func(e ast.Expression) { + switch n := e.(type) { + case *ast.FunctionCallExpr: + for _, arg := range n.Arguments { + inner := unwrapXPathArgument(arg) + if b, ok := inner.(*ast.BinaryExpr); ok && xpathArithmeticOperators[strings.ToLower(b.Operator)] { + hits = append(hits, [2]string{mendixFunctionName(n.Name), b.Operator}) + } + walk(arg) + } + case *ast.BinaryExpr: + walk(n.Left) + walk(n.Right) + case *ast.UnaryExpr: + walk(n.Operand) + case *ast.ParenExpr: + walk(n.Inner) + case *ast.SourceExpr: + walk(n.Expression) + case *ast.XPathPathExpr: + for _, s := range n.Steps { + walk(s.Expr) + walk(s.Predicate) + } + } + } + walk(expr) + return hits +} + +// unwrapXPathArgument strips the parentheses and source wrappers around an +// argument, down to the expression that is its value. +func unwrapXPathArgument(e ast.Expression) ast.Expression { + for { + switch n := e.(type) { + case *ast.ParenExpr: + e = n.Inner + case *ast.SourceExpr: + e = n.Expression + default: + return e + } + } +} + +// checkXPathFunctionArgumentOperators flags (MDL091) an operator inside an XPath +// function argument — the other way an expression construct gets into a +// retrieve constraint and out as CE0161 (mendixlabs/mxcli#1326). +func (v *microflowValidator) checkXPathFunctionArgumentOperators(variable string, where ast.Expression) { + for _, h := range xpathFunctionArgumentOperators(where) { + v.addViolation("MDL091", linter.SeverityError, + fmt.Sprintf("retrieve '$%s' constraint computes `%s` inside an argument of `%s()` — XPath does not "+ + "evaluate an operator in a function argument, and mxbuild reports CE0161 \"Error(s) in XPath constraint\"", + variable, h[1], h[0]), + fmt.Sprintf("Compute the value into a variable first, then pass the variable: "+ + "`declare $P String = ;` and `%s(, $P)`. An operator at the top "+ + "level of the constraint (`Name = 'X-' + $Key`) is fine.", h[0])) + } +} + // xpathVarTraversalRe matches a path rooted at a $variable with TWO OR MORE // segments (`$P/Mod.Assoc/Name`). One segment is deliberately not matched: both // `$P/Code` (the parameter's own attribute) and `$P/Mod.Assoc` (one hop to the diff --git a/mdl/executor/validate_microflow_set_object.go b/mdl/executor/validate_microflow_set_object.go new file mode 100644 index 0000000000..7a999e8fba --- /dev/null +++ b/mdl/executor/validate_microflow_set_object.go @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// setObjectRule is the rule ID for "set on an object variable". +const setObjectRule = "MDL-SET01" + +// checkSetOnObjectVariable flags `set $X = …` where $X is a single OBJECT. +// Mendix has no action that reassigns an object variable: Change variable +// takes only a primitive, and mxbuild answers CE7247 "Variable 'X' does not +// have a primitive type" (mendixlabs/mxcli#1323, measured on 11.14.0). +// +// This is plain check's half, which runs without a project, so it judges only +// the object producers it can see in the text: an entity parameter, a create +// object, a database `retrieve … first`, a loop iterator, a cast, a head. An +// association retrieve is an object or a list by the association's type and +// direction, which needs the project — check --references and exec decide that +// one (flowBuilder.refuseSetOnObject), and this rule leaves it alone rather +// than guess. +// +// The same rule refuses `set` on a PRIMITIVE parameter: a Change variable +// cannot target any parameter, and mxbuild answers CE7247 "Parameter 'N' +// cannot be changed." — measured on 11.14.0 for an Integer and a String +// parameter in a microflow, a nanoflow and a rule. A LIST parameter is not +// refused: `set $L = $M` on one is a Change list Replace, which mxbuild +// accepts, and so are `add`/`remove` and a member change `set $P/Attr = …`. +func (v *microflowValidator) checkSetOnObjectVariable(params []ast.MicroflowParam, body []ast.MicroflowStatement) { + // objects maps each variable currently known to hold one object to its entity. + objects := map[string]string{} + // isParam marks the object parameters: mxbuild words their CE7247 + // differently ("Parameter 'A' cannot be changed."), measured on 11.14.0. + isParam := map[string]bool{} + for _, p := range params { + if p.Type.EntityRef != nil && p.Type.Kind != ast.TypeListOf { + objects[p.Name] = p.Type.EntityRef.String() + isParam[p.Name] = true + } + } + lists := map[string]string{} + for _, p := range params { + if p.Type.EntityRef != nil && p.Type.Kind == ast.TypeListOf { + lists[p.Name] = p.Type.EntityRef.String() + } + } + // primitiveParams maps each primitive parameter to its type's name. + primitiveParams := map[string]string{} + for _, p := range params { + if p.Type.EntityRef == nil && p.Type.Kind != ast.TypeListOf { + primitiveParams[p.Name] = p.Type.Kind.String() + } + } + + forEachMicroflowStatement(body, func(s ast.MicroflowStatement) { + if set, ok := s.(*ast.MfSetStmt); ok && !strings.Contains(set.Target, "/") { + name := strings.TrimPrefix(set.Target, "$") + if entity, ok := objects[name]; ok { + rejection := fmt.Sprintf("CE7247 \"Variable '%s' does not have a primitive type\"", name) + if isParam[name] { + rejection = fmt.Sprintf("CE7247 \"Parameter '%s' cannot be changed\"", name) + } + v.addViolation(setObjectRule, linter.SeverityError, + fmt.Sprintf("cannot set object variable '$%s' (%s): Mendix has no action that reassigns an "+ + "object variable — Change variable takes only a primitive, and mxbuild rejects it with "+ + "%s.", name, entity, rejection), + fmt.Sprintf("Return the new object from a sub-microflow instead (recursion for a chain walk), "+ + "retrieve it into a new variable, or change the object's members with `change $%s (…)`.", name)) + } else if typ, ok := primitiveParams[name]; ok { + v.addViolation(setObjectRule, linter.SeverityError, + fmt.Sprintf("cannot set parameter '$%s' (%s): a Change variable cannot target a parameter, "+ + "and mxbuild rejects it with CE7247 \"Parameter '%s' cannot be changed\".", name, typ, name), + fmt.Sprintf("Copy it into a variable and change that: `declare $%sValue %s = $%s;`.", name, typ, name)) + } + } + + // Rebinding first: a name a statement produces is whatever that + // statement makes it, and nothing it was before. + for _, p := range statementProducedVars(s) { + delete(objects, p.name) + delete(lists, p.name) + delete(isParam, p.name) + delete(primitiveParams, p.name) + } + switch st := s.(type) { + case *ast.CreateObjectStmt: + if st.Variable != "" && st.EntityType.Module != "" { + objects[st.Variable] = st.EntityType.String() + } + case *ast.RetrieveStmt: + if st.Variable != "" && st.StartVariable == "" && st.Source.Module != "" { + if st.First { + objects[st.Variable] = st.Source.String() + } else { + lists[st.Variable] = st.Source.String() + } + } + case *ast.CreateListStmt: + if st.Variable != "" && st.EntityType.Module != "" { + lists[st.Variable] = st.EntityType.String() + } + case *ast.LoopStmt: + if entity, ok := lists[st.ListVariable]; ok && st.LoopVariable != "" { + objects[st.LoopVariable] = entity + } + case *ast.ListOperationStmt: + // head is list-only; find is also the String function, so it is left out. + if entity, ok := lists[st.InputVariable]; ok && st.OutputVariable != "" { + switch st.Operation { + case ast.ListOpHead: + objects[st.OutputVariable] = entity + case ast.ListOpFilter, ast.ListOpSort, ast.ListOpTail: + lists[st.OutputVariable] = entity + } + } + } + }) +} + +// setTargetViolations runs MDL-SET01 alone over a flow that does not go +// through ValidateMicroflow — a rule, which plain check validates only for +// its parameter annotations and validateRule checks for check --references +// and exec. +func setTargetViolations(docType, name string, params []ast.MicroflowParam, body []ast.MicroflowStatement) []linter.Violation { + v := µflowValidator{mfName: name, docType: docType} + v.checkSetOnObjectVariable(params, body) + return v.violations +} diff --git a/mdl/executor/validate_nanoflow.go b/mdl/executor/validate_nanoflow.go index bace87a205..5729126a95 100644 --- a/mdl/executor/validate_nanoflow.go +++ b/mdl/executor/validate_nanoflow.go @@ -49,6 +49,8 @@ func validateNanoflowWith(stmt *ast.CreateNanoflowStmt, voids *voidCodeActions) v.seedPrimitiveKinds(stmt.Parameters, stmt.Body) v.checkDuplicateVariableNames(v.params, stmt.Body) v.checkVoidCallOutputUse(v.params, stmt.Body) + // MDL-SET01: a nanoflow's Change variable is as primitive-only (CE7247). + v.checkSetOnObjectVariable(v.params, stmt.Body) // The nanoflow restrictions exec's build refuses (validateNanoflow): an // action a nanoflow cannot hold, an error-handling clause its activity // rejects (CE6035), a Binary return. They ran only inside exec, so `check` diff --git a/mdl/executor/validate_nanoflow_test.go b/mdl/executor/validate_nanoflow_test.go index 56fca0c3fb..bfe3442b5e 100644 --- a/mdl/executor/validate_nanoflow_test.go +++ b/mdl/executor/validate_nanoflow_test.go @@ -52,8 +52,9 @@ end;`, name: "unknown call nested in an if branch", src: `create nanoflow Test.NF_Dev ($x: String) begin + declare $y String = $x; if $x != empty then - set $x = trunc(1.5); + set $y = trunc(1.5); end if; end;`, wantMDL: true, diff --git a/mdl/executor/validate_page_button_context.go b/mdl/executor/validate_page_button_context.go index fe9071b091..d8ea51bc60 100644 --- a/mdl/executor/validate_page_button_context.go +++ b/mdl/executor/validate_page_button_context.go @@ -25,7 +25,7 @@ func ValidatePageButtonContext(prog *ast.Program) []linter.Violation { var out []linter.Violation for _, stmt := range prog.Statements { if label, widgets, ok := documentWidgets(stmt); ok { - out = append(out, checkButtonContextTree(widgets, "", false, label)...) + out = append(out, checkButtonContextTree(widgets, "", false, "", label)...) } } return out @@ -46,29 +46,102 @@ func ValidatePageButtonContext(prog *ast.Program) []linter.Violation { // context, and its control bar's $currentObject is that object — mxbuild builds // it clean. The grid's OWN data source never scopes its control bar, so the // control bar inherits the context from above the grid, not from the grid. -func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inContext bool, locationPrefix string) []linter.Violation { +// +// nearest is the name of the data container whose object the widget sits in +// ("" at the top), for MDL-BUTTON02 (mendixlabs/mxcli#1324). It moves with the +// same rule as inContext: a control bar keeps the context from above its grid. +func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inContext bool, nearest, locationPrefix string) []linter.Violation { var out []linter.Violation for _, w := range widgets { if w == nil { continue } - if controlBarOf != "" && !inContext { - if a := w.GetAction(); a != nil { + if a := w.GetAction(); a != nil { + if controlBarOf != "" && !inContext { out = append(out, checkControlBarAction(a, w.Name, controlBarOf, locationPrefix)...) } + if nearest != "" { + out = append(out, checkOwnContainerName(a, w.Name, nearest, locationPrefix)...) + } } childContext := inContext || isObjectContextContainer(w) + childNearest := nearest + if isObjectContextContainer(w) && w.Name != "" { + childNearest = w.Name + } for _, c := range w.Children { - childOf, ctx := controlBarOf, childContext + childOf, ctx, near := controlBarOf, childContext, childNearest if c != nil && strings.EqualFold(c.Type, "controlbar") { - childOf, ctx = w.Name, inContext + childOf, ctx, near = w.Name, inContext, nearest } - out = append(out, checkButtonContextTree([]*ast.WidgetV3{c}, childOf, ctx, locationPrefix)...) + out = append(out, checkButtonContextTree([]*ast.WidgetV3{c}, childOf, ctx, near, locationPrefix)...) } } return out } +// checkOwnContainerName flags an action argument (and its chained THEN action) +// that reads the nearest data container by its widget name — `$dvGate` from a +// button directly inside dvGate. A container's name is a variable only for the +// containers nested BELOW it; in its own context the object is +// $currentObject, and mxbuild reports the name as CE0117 "Error(s) in +// expression." (mendixlabs/mxcli#1324, measured on 11.14.0 for data views, +// list views, galleries and data grids alike). +func checkOwnContainerName(a *ast.ActionV3, widgetName, nearest, locationPrefix string) []linter.Violation { + var out []linter.Violation + for a != nil { + for _, arg := range a.Args { + s, ok := arg.Value.(string) + if !ok || !exprReadsVariable(s, nearest) { + continue + } + out = append(out, linter.Violation{ + RuleID: "MDL-BUTTON02", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: `%s` passes `%s` to its %s action, but `%s` is the data container the widget sits in directly — its name is a variable only inside a data container nested below it (CE0117)", + locationPrefix, widgetName, s, a.Type, nearest), + Suggestion: fmt.Sprintf( + "Use $currentObject for the object of `%s` here (`$currentObject/Attr` for an attribute); `$%s` works from a data view, list or grid nested inside it.", + nearest, nearest), + }) + } + a = a.ThenAction + } + return out +} + +// exprReadsVariable reports whether expression source reads $name as a +// variable: a `$name` token outside a string literal, ending at a +// non-identifier character. Case-insensitive, as widget names are unique +// case-insensitively on a page. +func exprReadsVariable(expr, name string) bool { + inString := false + for i := 0; i < len(expr); i++ { + c := expr[i] + if c == '\'' { + inString = !inString // '' inside a literal toggles twice + continue + } + if inString || c != '$' { + continue + } + j := i + 1 + for j < len(expr) && isExprIdentByte(expr[j]) { + j++ + } + if strings.EqualFold(expr[i+1:j], name) { + return true + } + i = j - 1 + } + return false +} + +func isExprIdentByte(c byte) bool { + return c == '_' || c >= 'A' && c <= 'Z' || c >= 'a' && c <= 'z' || c >= '0' && c <= '9' +} + // isObjectContextContainer reports whether w gives its (non-control-bar) // children a current object: a data view's object, a list view or gallery // item, a grid row (column content). diff --git a/mdl/executor/validate_page_own_container_name_test.go b/mdl/executor/validate_page_own_container_name_test.go new file mode 100644 index 0000000000..d2f0629570 --- /dev/null +++ b/mdl/executor/validate_page_own_container_name_test.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// ownContainerFlagged returns the widgets MDL-BUTTON02 names, sorted. +func ownContainerFlagged(t *testing.T, src string) []string { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + var names []string + for _, v := range ValidatePageButtonContext(prog) { + if v.RuleID != "MDL-BUTTON02" { + continue + } + start := strings.Index(v.Message, "`") + end := strings.Index(v.Message[start+1:], "`") + names = append(names, v.Message[start+1:start+1+end]) + } + sort.Strings(names) + return names +} + +// mendixlabs/mxcli#1324: a data container's widget-name variable ($dvGate) is +// only in scope for widgets nested in ANOTHER data container below it. From the +// container's own direct context it is not a variable at all, and mxbuild +// 11.14.0 reports `[CE0117] "Error(s) in expression." at Action button 'btnOwn'` +// while check and exec were clean. This is the issue's reproduction verbatim. +func TestValidatePageButtonContext_OwnDataViewName_Issue1324(t *testing.T) { + src := `create page "G52"."GatePage" (Title: 'Gate', Layout: Atlas_Core.PopupLayout, Params: { $Gate: "G52"."Gate" }) { + dataview dvGate (DataSource: $Gate) { + actionbutton btnOwn (Caption: 'Own data view name', Action: microflow "G52"."ACT_Open"(Gate: $dvGate)) + actionbutton btnCur (Caption: 'Current object', Action: microflow "G52"."ACT_Open"(Gate: $currentObject)) + dataview dvInner (DataSource: $Gate) { + actionbutton btnOuter (Caption: 'Enclosing data view name', Action: microflow "G52"."ACT_Open"(Gate: $dvGate)) + } + } +};` + got := ownContainerFlagged(t, src) + if strings.Join(got, ",") != "btnOwn" { + t.Fatalf("want only btnOwn flagged (btnCur and btnOuter build clean), got %v", got) + } +} + +// Every verdict below was measured with `mx check` on Mendix 11.14.0: the +// flagged set is exactly the set mxbuild reported CE0117 for. The nearest +// data container decides — a plain container is transparent, a control bar +// takes the context from ABOVE its grid (so a grid's own selection stays +// spellable there), and a row or item of a nested list is a new context. +func TestValidatePageButtonContext_OwnContainerName_MeasuredShapes(t *testing.T) { + src := `create page G52.Probe (Title: 'Probe', Layout: Atlas_Core.PopupLayout, Params: ( $Gate: G52.Gate )) { + dataview dvP (DataSource: $Gate) { + container cWrap { + actionbutton btnInContainer (Caption: 'c', Action: microflow G52.ACT_Open(Gate = $dvP)) + } + actionbutton btnOwnPath (Caption: 'p', Action: microflow G52.ACT_Str(S = $dvP/Name)) + actionbutton btnLiteral (Caption: 'l', Action: microflow G52.ACT_Str(S = '$dvP')) + datagrid dgIn (DataSource: database from G52.Gate) { + controlbar cb1 { + actionbutton btnCtlBar (Caption: 'cb', Action: microflow G52.ACT_Open(Gate = $dvP)) + } + column Name (Attribute: Name) { + actionbutton btnInColumn (Caption: 'col', Action: microflow G52.ACT_Open(Gate = $dvP)) + } + } + listview lvIn (DataSource: database from G52.Gate) { + actionbutton btnInList (Caption: 'li', Action: microflow G52.ACT_Open(Gate = $dvP)) + actionbutton btnLvOwn (Caption: 'lo', Action: microflow G52.ACT_Open(Gate = $lvIn)) + } + } + datagrid dgSelf (DataSource: database from G52.Gate, Selection: Single) { + controlbar cb2 { + actionbutton btnSelCtl (Caption: 'sel', Action: microflow G52.ACT_Open(Gate = $dgSelf)) + } + column Name (Attribute: Name) { + actionbutton btnDgOwnCol (Caption: 'dg', Action: microflow G52.ACT_Open(Gate = $dgSelf)) + } + } + gallery gaSelf (DataSource: database from G52.Gate) { + template t1 { + actionbutton btnGaOwn (Caption: 'ga', Action: microflow G52.ACT_Open(Gate = $gaSelf)) + } + } +};` + got := strings.Join(ownContainerFlagged(t, src), ",") + want := "btnCtlBar,btnDgOwnCol,btnGaOwn,btnInContainer,btnLvOwn,btnOwnPath" + if got != want { + t.Fatalf("flagged %q, want %q (mx check 11.14.0's CE0117 set)", got, want) + } +} + +// The suggestion is the spelling that works where the button is. +func TestValidatePageButtonContext_OwnContainerName_Suggestion(t *testing.T) { + prog, errs := visitor.Build(`create page P.X (Title: 'x', Layout: Atlas_Core.PopupLayout, Params: ( $O: P.O )) { + dataview dvO (DataSource: $O) { + actionbutton b1 (Caption: 'b', Action: microflow P.F(O = $dvO)) + } +};`) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + vs := ValidatePageButtonContext(prog) + if len(vs) != 1 || vs[0].RuleID != "MDL-BUTTON02" { + t.Fatalf("want one MDL-BUTTON02, got %+v", vs) + } + if !strings.Contains(vs[0].Message, "CE0117") || !strings.Contains(vs[0].Suggestion, "$currentObject") { + t.Errorf("message should name CE0117 and suggest $currentObject: %+v", vs[0]) + } +} diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 82213720a0..6ea4f17089 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -120,6 +120,8 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { if ruleStmt, ok := stmt.(*ast.CreateRuleStmt); ok { violations = append(violations, ValidateFlowParameterAnnotations("rule '"+ruleStmt.Name.String()+"'", ruleStmt.Parameters)...) + violations = append(violations, + setTargetViolations("rule", ruleStmt.Name.String(), ruleStmt.Parameters, ruleStmt.Body)...) } // Check workflow for constructs MxBuild rejects (missing page, // single-outcome-with-activities, invalid decision outcome names) diff --git a/mdl/executor/xpath_function_operator_test.go b/mdl/executor/xpath_function_operator_test.go new file mode 100644 index 0000000000..6b31e4625f --- /dev/null +++ b/mdl/executor/xpath_function_operator_test.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// mendixlabs/mxcli#1326: an operator inside an XPath function argument passed +// check and exec, then failed the build with CE0161 on mxbuild 11.14.0 — the +// measured pairs are on xpathArithmeticOperators. The bracketed `[…]` form +// does not parse an operator in a function argument at all, so only the +// unbracketed form reached the writer. +func TestValidateMicroflow_XPathFunctionArgumentOperator(t *testing.T) { + cases := []struct { + name string + where string + wantMDL bool + }{ + {"concatenation in starts-with", "starts-with(Name, 'MS-' + $Key)", true}, + {"parenthesised argument", "contains(Name, ($Key + 'x'))", true}, + {"inside not()", "not(ends-with(Name, $Key + '-X'))", true}, + {"subtraction in contains", "contains(Name, $Key - 'a')", true}, + {"inside and", "Name != empty and ends-with(Name, '-' + $Key)", true}, + {"top-level concatenation is fine", "Name = 'X-' + $Key", false}, + {"operator beside a call is fine", "year-from-dateTime(Due) = $N - 1", false}, + {"pre-computed argument is fine", "starts-with(Name, $Key)", false}, + {"operator in a literal is text", "starts-with(Name, 'a + b')", false}, + {"comparison inside not() is fine", "not(Name = $Key)", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($Key: String, $N: Integer)\nreturns list of M.Item\nbegin\n retrieve $L from M.Item where " + tc.where + ";\n return $L;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var msgs []string + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL091" { + msgs = append(msgs, vi.Message) + } + } + if got := len(msgs) > 0; got != tc.wantMDL { + t.Fatalf("MDL091 fired=%v, want %v (where: %q) %v", got, tc.wantMDL, tc.where, msgs) + } + if tc.wantMDL && !strings.Contains(strings.Join(msgs, "\n"), "CE0161") { + t.Errorf("message must name CE0161: %v", msgs) + } + }) + } +} + +// check and exec must agree: the report had both accepting the constraint. +func TestCheckExecAgree_RetrieveConstraintFunctionArgumentOperator(t *testing.T) { + const head = `mdl 1; +create module ModG; +create persistent entity ModG.Item (Name: String(50)); +create microflow ModG.SUB_PlusInFunc ($Key: String) +begin + retrieve $L from ModG.Item where %s; +end;` + exec, _, dir := openPedAppCopy(t) + bad := strings.Replace(head, "%s", "starts-with(Name, 'MS-' + $Key)", 1) + got := strings.Join(agreeCheck(t, exec, dir, bad), "\n") + if !strings.Contains(got, "MDL091") || !strings.Contains(got, "starts-with()") { + t.Errorf("check -p must report MDL091 for the operator, reported:\n%s", got) + } + if err := agreeExec(t, exec, bad); err == nil || !strings.Contains(err.Error(), "MDL091") { + t.Errorf("exec must refuse the operator with MDL091, got: %v", err) + } + + // The two shapes the report measured clean must stay accepted. + for _, ok := range []string{"Name = 'X-' + $Key", "starts-with(Name, $Key)"} { + exec2, _, dir2 := openPedAppCopy(t) + assertAgree(t, exec2, dir2, strings.Replace(head, "%s", ok, 1), "") + } +} diff --git a/sdk/javaactions/rename.go b/sdk/javaactions/rename.go new file mode 100644 index 0000000000..73081dc116 --- /dev/null +++ b/sdk/javaactions/rename.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package javaactions + +import ( + "regexp" + "strings" +) + +// RenameSource rewrites a Java action's source for a rename from oldName to +// newName the way mxbuild regenerates it: the three generated places that carry +// the action's name — `public class Old`, the constructor `public Old(` and +// toString's `return "Old";` — and nothing else. The user code and extra code +// are the author's and are left byte-for-byte, a mention of the old name +// included; so is everything else, CRLF line endings among it. +// +// Measured against mxbuild 11.14.0 (testdata/mxbuild/JA_Renamed*): renaming the +// file alone left `public class Old` in New.java, which is not valid Java. +func RenameSource(source, oldName, newName string) string { + if oldName == newName || oldName == "" { + return source + } + old := regexp.QuoteMeta(oldName) + patterns := []*regexp.Regexp{ + regexp.MustCompile(`(\bclass\s+)` + old + `\b`), + regexp.MustCompile(`(\bpublic\s+)` + old + `(\s*\()`), + regexp.MustCompile(`(\breturn\s+)"` + old + `"(\s*;)`), + } + replacements := []string{"${1}" + newName, "${1}" + newName + "${2}", `${1}"` + newName + `"${2}`} + + rewrite := func(generated string) string { + for i, re := range patterns { + generated = re.ReplaceAllString(generated, replacements[i]) + } + return generated + } + + // Rewrite only the generated stretches between the author's sections. + var b strings.Builder + rest := source + for { + start, end, ok := nextAuthoredSection(rest) + if !ok { + b.WriteString(rewrite(rest)) + return b.String() + } + b.WriteString(rewrite(rest[:start])) + b.WriteString(rest[start:end]) + rest = rest[end:] + } +} + +// authoredSections are the marker pairs whose contents mxbuild keeps as written. +var authoredSections = [][2]string{ + {"// BEGIN USER CODE", "// END USER CODE"}, + {"// BEGIN EXTRA CODE", "// END EXTRA CODE"}, +} + +// nextAuthoredSection finds the first authored section in s, markers included. +// Markers match case-insensitively, as in RetainedSections. +func nextAuthoredSection(s string) (start, end int, ok bool) { + lower := asciiLower(s) + start = -1 + for _, m := range authoredSections { + bi := strings.Index(lower, asciiLower(m[0])) + if bi == -1 || (start != -1 && bi > start) { + continue + } + ei := strings.Index(lower[bi:], asciiLower(m[1])) + if ei == -1 { + continue + } + start, end = bi, bi+ei+len(m[1]) + } + return start, end, start != -1 +} diff --git a/sdk/javaactions/rename_test.go b/sdk/javaactions/rename_test.go new file mode 100644 index 0000000000..880c070db9 --- /dev/null +++ b/sdk/javaactions/rename_test.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package javaactions + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// RENAME JAVA ACTION moved javasource//actions/.java to .java +// and left `public class Old`, its constructor and toString's "Old" inside, so +// the project's own source was not valid Java: javac requires a public class to +// live in a file of its name. A full build hides it — mxbuild regenerates the +// stub in its copy — but `run --local --watch` hot reload compiles the file on +// disk, and so does every IDE. +// +// The goldens are measured, not written: JA_Renamed.before is what mxcli left +// after `mxcli rename java-action R1318.JA_Old JA_New` on a fresh 11.14.0 app, +// hand-edited to add an import, extra code, and user code that mentions JA_Old +// in a string and in comments; JA_Renamed is the same file after mxbuild ran on +// the project in place. mxbuild changed exactly three lines — class, constructor, +// toString — and kept the user code, extra code and CRLF as they were. +func TestRenameSourceMatchesMxbuild(t *testing.T) { + read := func(name string) string { + b, err := os.ReadFile(filepath.Join("testdata", "mxbuild", name)) + if err != nil { + t.Fatal(err) + } + return string(b) + } + before := read("JA_Renamed.before.java.golden") + want := read("JA_Renamed.java.golden") + + got := RenameSource(before, "JA_Old", "JA_New") + if got != want { + gl, wl := strings.Split(got, "\n"), strings.Split(want, "\n") + for i := 0; i < len(gl) && i < len(wl); i++ { + if gl[i] != wl[i] { + t.Fatalf("line %d differs from mxbuild's regeneration:\n got: %q\nwant: %q", i+1, gl[i], wl[i]) + } + } + t.Fatalf("length differs from mxbuild's regeneration: %d lines, want %d", len(gl), len(wl)) + } +} + +// A name that is a prefix of another identifier, or of the new name, must not +// be rewritten inside it. +func TestRenameSourceWholeWordsOnly(t *testing.T) { + src := "public class JA extends UserAction\n{\n\tpublic JA(\n\t)\n\t{\n\t}\n\tpublic java.lang.String toString()\n\t{\n\t\treturn \"JA\";\n\t}\n}\n" + want := "public class JA_X extends UserAction\n{\n\tpublic JA_X(\n\t)\n\t{\n\t}\n\tpublic java.lang.String toString()\n\t{\n\t\treturn \"JA_X\";\n\t}\n}\n" + if got := RenameSource(src, "JA", "JA_X"); got != want { + t.Errorf("got:\n%s\nwant:\n%s", got, want) + } +} diff --git a/sdk/javaactions/testdata/mxbuild/JA_Renamed.before.java.golden b/sdk/javaactions/testdata/mxbuild/JA_Renamed.before.java.golden new file mode 100644 index 0000000000..a543c4db13 --- /dev/null +++ b/sdk/javaactions/testdata/mxbuild/JA_Renamed.before.java.golden @@ -0,0 +1,53 @@ +// This file was generated by Mendix Studio Pro. +// +// WARNING: Only the following code will be retained when actions are regenerated: +// - the import list +// - the code between BEGIN USER CODE and END USER CODE +// - the code between BEGIN EXTRA CODE and END EXTRA CODE +// Other code you write will be lost the next time you deploy the project. +// Special characters, e.g., é, ö, à, etc. are supported in comments. + +package r1318.actions; + +import com.mendix.systemwideinterfaces.core.IContext; +import com.mendix.systemwideinterfaces.core.UserAction; +import java.util.Locale; + +public class JA_Old extends UserAction +{ + private final java.lang.String Text; + + public JA_Old( + IContext context, + java.lang.String _text + ) + { + super(context); + this.Text = _text; + } + + @java.lang.Override + public java.lang.String executeAction() throws Exception + { + // BEGIN USER CODE + + String who = "JA_Old"; // JA_Old named in user code + return helper(Text) + who.toLowerCase(Locale.ROOT).length() * 0; + + // END USER CODE + } + + /** + * Returns a string representation of this action + * @return a string representation of this action + */ + @java.lang.Override + public java.lang.String toString() + { + return "JA_Old"; + } + + // BEGIN EXTRA CODE + private static String helper(String s) { return s.trim(); } // was JA_Old + // END EXTRA CODE +} diff --git a/sdk/javaactions/testdata/mxbuild/JA_Renamed.java.golden b/sdk/javaactions/testdata/mxbuild/JA_Renamed.java.golden new file mode 100644 index 0000000000..6ecb10ab9d --- /dev/null +++ b/sdk/javaactions/testdata/mxbuild/JA_Renamed.java.golden @@ -0,0 +1,53 @@ +// This file was generated by Mendix Studio Pro. +// +// WARNING: Only the following code will be retained when actions are regenerated: +// - the import list +// - the code between BEGIN USER CODE and END USER CODE +// - the code between BEGIN EXTRA CODE and END EXTRA CODE +// Other code you write will be lost the next time you deploy the project. +// Special characters, e.g., é, ö, à, etc. are supported in comments. + +package r1318.actions; + +import com.mendix.systemwideinterfaces.core.IContext; +import com.mendix.systemwideinterfaces.core.UserAction; +import java.util.Locale; + +public class JA_New extends UserAction +{ + private final java.lang.String Text; + + public JA_New( + IContext context, + java.lang.String _text + ) + { + super(context); + this.Text = _text; + } + + @java.lang.Override + public java.lang.String executeAction() throws Exception + { + // BEGIN USER CODE + + String who = "JA_Old"; // JA_Old named in user code + return helper(Text) + who.toLowerCase(Locale.ROOT).length() * 0; + + // END USER CODE + } + + /** + * Returns a string representation of this action + * @return a string representation of this action + */ + @java.lang.Override + public java.lang.String toString() + { + return "JA_New"; + } + + // BEGIN EXTRA CODE + private static String helper(String s) { return s.trim(); } // was JA_Old + // END EXTRA CODE +}