From 218be6f3544e96199adfaace528edeb43dd6340d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 08:16:15 +0000 Subject: [PATCH 1/2] fix(mdl): let MDL document mappings, published REST services and data transformers (mendixlabs/mxcli#1510) lint QUAL002 asks for documentation on import and export mappings, published REST services and data transformers, and MDL could not write it: the four create statements had no Documentation field, so the `/** ... */` doc comment above them was dropped with MDL089 and the findings were permanent. Two more defects kept them that way: - every create or modify wrote an empty Documentation over the stored text (the executors never set it; the data transformer writer wrote a literal ""), so describe -> exec of a Studio Pro-documented mapping or transformer deleted its documentation; - the catalog wrote "" for published_rest_services.Documentation, so QUAL002 reported every published REST service, documented or not. The doc comment is now each statement's documentation, as on a JSON structure: the visitor reads it with its presence bit, the executors set it and carry the stored text when no comment is written (#1018), the modelsdk backend reads and writes the top-level `Documentation` key for published REST services and data transformers (verified against modelsdk/gen and against Mendix's own reader with `mx dump-mpr` on 11.6.6 and 11.15), describe prints it back, and the catalog stores it. PublicDocumentation (the OpenAPI text) keeps being carried as stored. Java action parameter descriptions were already writable (#1588); the bug-test script covers them alongside. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...-rest-data-transformer-undocumentable.json | 1 + .../json-structures-and-mappings/SKILL.md | 43 ++-- cmd/mxcli/syntax/features_integration.go | 14 +- docs-site/src/examples/rest-integration.md | 3 + .../integration/create-data-transformer.md | 4 + .../integration/create-export-mapping.md | 4 + .../integration/create-import-mapping.md | 4 + ...mentation-for-mappings-rest-javaparams.mdl | 68 ++++++ mdl/ast/ast_datatransformer.go | 5 + mdl/ast/ast_import_export_mapping.go | 8 + mdl/ast/ast_rest.go | 5 + .../modelsdk/data_transformer_write.go | 5 +- mdl/backend/modelsdk/integration_read.go | 2 + mdl/backend/modelsdk/published_rest_write.go | 6 +- mdl/catalog/builder_rest.go | 2 +- .../builder_rest_documentation_test.go | 30 +++ mdl/executor/cmd_datatransformer.go | 7 +- mdl/executor/cmd_export_mappings.go | 8 +- mdl/executor/cmd_import_mappings.go | 8 +- mdl/executor/cmd_published_rest.go | 5 + mdl/executor/documentation_carry.go | 30 ++- mdl/executor/documentation_coverage_test.go | 6 + mdl/executor/documentation_preserved_test.go | 30 +++ ...ntegration_documentation_issue1510_test.go | 199 ++++++++++++++++++ mdl/visitor/visitor_datatransformer.go | 1 + mdl/visitor/visitor_import_export_mapping.go | 2 + .../visitor_integration_documentation_test.go | 80 +++++++ mdl/visitor/visitor_rest.go | 1 + model/types.go | 8 + 29 files changed, 559 insertions(+), 30 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-16-qual002-mappings-published-rest-data-transformer-undocumentable.json create mode 100644 mdl-examples/bug-tests/1510-documentation-for-mappings-rest-javaparams.mdl create mode 100644 mdl/catalog/builder_rest_documentation_test.go create mode 100644 mdl/executor/integration_documentation_issue1510_test.go create mode 100644 mdl/visitor/visitor_integration_documentation_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-qual002-mappings-published-rest-data-transformer-undocumentable.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-qual002-mappings-published-rest-data-transformer-undocumentable.json new file mode 100644 index 0000000000..82ad800791 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-qual002-mappings-published-rest-data-transformer-undocumentable.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-16", "symptom": "mendixlabs/mxcli#1510: lint QUAL002 reported import/export mappings, published REST services and data transformers as undocumented (45 findings on one project), and nothing in MDL could close them: a `/** … */` above `create or modify published rest service` was dropped with MDL089. Every published REST service was reported even when Studio Pro HAD documented it, and `create or modify` of a Studio Pro-documented mapping, published REST service or data transformer wrote an empty Documentation over the stored text.", "cause": "The four create statements' AST had no Documentation field, so storesDocumentation() reported the comment as detached and the visitor never read it. The executors never set the model's Documentation (the mapping writers wrote whatever the model held, i.e. \"\"; the data transformer writer wrote a literal \"\"; the published REST writer only carried the stored value via keepStoredTopLevel). model.PublishedRestService and model.DataTransformer had no Documentation, and the catalog builder wrote \"\" for published_rest_services.Documentation (\"not stored on published REST\").", "file": "mdl/ast/ast_import_export_mapping.go, mdl/ast/ast_rest.go, mdl/ast/ast_datatransformer.go, mdl/visitor/visitor_import_export_mapping.go, mdl/visitor/visitor_rest.go, mdl/visitor/visitor_datatransformer.go, mdl/executor/cmd_import_mappings.go, mdl/executor/cmd_export_mappings.go, mdl/executor/cmd_published_rest.go, mdl/executor/cmd_datatransformer.go, mdl/executor/documentation_carry.go, mdl/backend/modelsdk/published_rest_write.go, mdl/backend/modelsdk/integration_read.go, mdl/backend/modelsdk/data_transformer_write.go, mdl/catalog/builder_rest.go, model/types.go", "fix": "Documentation + DocumentationSet on the four statements, read with findDocComment; the executors set it and carry the stored value when no comment is written (carriedDocumentation, #1018); model + modelsdk read and write the top-level `Documentation` key for published REST services and data transformers (published REST drops it from the keep-stored list); describe prints it with writeDocumentDocComment (`--` lines when the text holds `*/`); the catalog stores the service's documentation. Java action parameter descriptions were already writable (#1588).", "test": "mdl/visitor TestIntegrationDocumentsReadDocComment_Issue1510; mdl/executor TestIntegrationDocumentation_Issue1510 (PedApp copy: stored key, describe round trip, carry, `/** */` clears) and TestIntegrationDocumentation_StudioProMappingKeepsIt_Issue1510; mdl/catalog TestPublishedRestServiceDocumentationReachesCatalog_Issue1510; the integration TestDocumentation_SurvivesRewrite gains the four doctypes. Revert check: without the fix every new test fails with the reported symptom; stubbing only the carry fails the rewrite and Studio Pro cases.", "insight": "Verify the BSON key against Mendix's own reader, not only modelsdk/gen: `mx dump-mpr --module-names=X` prints the model as Mendix deserialises it, so the doc comment appearing as the unit's `documentation` there (11.6.6 and 11.15) proves the key is the one Studio Pro reads. A QUAL002 finding nobody can close has two suspects, the writer and the catalog reader, and here both were broken: the catalog's hard-coded \"\" made the rule fire on Studio Pro-documented services too.", "refs": ["mdl-examples/bug-tests/1510-documentation-for-mappings-rest-javaparams.mdl", "mdl/executor/integration_documentation_issue1510_test.go", "mdl/visitor/visitor_integration_documentation_test.go", "mdl/catalog/builder_rest_documentation_test.go"]} diff --git a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md index edb2b347de..877b345d24 100644 --- a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md +++ b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md @@ -10,19 +10,20 @@ description: "Create and manage JSON structures, import mappings and export mapp | lines | section | |---|---| -| 32-109 | Key Concepts | -| 110-121 | Export Mappings | -| 122-186 | JSON Structures | -| 187-228 | Message Definitions | -| 229-389 | Import Mappings | -| 390-487 | Export Mappings | -| 488-505 | Starting a Mapping Below the Payload Root | -| 506-575 | Microflow Actions | -| 576-588 | Browse | -| 589-636 | Export Workflow: PE → NPE → JSON | -| 637-643 | Realistic Example: Countries REST API | -| 644-667 | Placing Documents in Folders | -| 668-682 | Common Mistakes | +| 33-110 | Key Concepts | +| 111-122 | Export Mappings | +| 123-187 | JSON Structures | +| 188-229 | Message Definitions | +| 230-390 | Import Mappings | +| 391-488 | Export Mappings | +| 489-506 | Starting a Mapping Below the Payload Root | +| 507-576 | Microflow Actions | +| 577-589 | Browse | +| 590-637 | Export Workflow: PE → NPE → JSON | +| 638-644 | Realistic Example: Countries REST API | +| 645-668 | Placing Documents in Folders | +| 669-684 | Documentation | +| 685-699 | Common Mistakes | # JSON Structures, Import Mappings & Export Mappings @@ -665,6 +666,22 @@ adding a folder to an existing script is safe and removing one is a no-op. See `organize-project` for `move` and the full folder story. +## Documentation + +A `/** … */` doc comment directly above the `create` is the document's +documentation — for JSON structures, import and export mappings alike (and for +published REST services and data transformers). It is what `lint` QUAL002 asks +for, and `describe` prints it back. On `create or modify`, **no** comment keeps +the stored text and an empty `/** */` clears it: + +```mdl +mdl 1; +/** Maps the order payload of the shop API onto Sales.Order. */ +create or modify import mapping Sales.IMM_Order + with json structure Sales.JSON_Order +{ create Sales.Order { OrderId = id, Total = total } }; +``` + ## Common Mistakes | Mistake | Fix | diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index b2422f2565..1fae5af288 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -348,7 +348,7 @@ func init() { "basic authentication", "active session", "authentication microflow", "authentication parameter", "header parameter", "api key", "token", }, - Syntax: "CREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API',\n [Authentication: none | ( basic, session, microflow Module.Authenticate [( $Param: query|header ['HttpName'] [description '...'], ... )] )]\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create\n [IMPORT MAPPING Module.IMM] [EXPORT MAPPING Module.EMM]\n [COMMIT Yes | YesWithoutEvents | No]; -- no COMMIT clause: Yes\n }\n};\n\n" + + Syntax: "[/** documentation */]\nCREATE [OR MODIFY] PUBLISHED REST SERVICE Module.Name (\n Path: 'rest/api/v1',\n Version: '1.0.0',\n ServiceName: 'My API',\n [Authentication: none | ( basic, session, microflow Module.Authenticate [( $Param: query|header ['HttpName'] [description '...'], ... )] )]\n)\n{\n RESOURCE 'name' {\n GET '' MICROFLOW Module.GetAll;\n GET '{id}' MICROFLOW Module.GetById;\n POST '' MICROFLOW Module.Create\n [IMPORT MAPPING Module.IMM] [EXPORT MAPPING Module.EMM]\n [COMMIT Yes | YesWithoutEvents | No]; -- no COMMIT clause: Yes\n }\n};\n\n" + "-- Operation parameters come from the microflow, as Studio Pro derives them:\n" + "-- a parameter named in the path ('{id}') -> path parameter\n" + "-- an object or a list -> the body\n" + @@ -374,6 +374,9 @@ func init() { "-- list only when it differs from that default.\n" + "-- With authentication on, grant at least one module role (CE0338);\n" + "-- custom authentication also needs app security on (CE6600).\n\n" + + "-- Documentation: the /** ... */ doc comment above CREATE, the service's own\n" + + "-- documentation (not the OpenAPI description, which MDL keeps unchanged).\n" + + "-- On OR MODIFY no comment keeps the stored text, /** */ clears it.\n\n" + "ALTER PUBLISHED REST SERVICE Module.Name SET ( Version: '2.0.0', Authentication: (session) );\nALTER PUBLISHED REST SERVICE Module.Name SET Version = '2.0.0';\nALTER PUBLISHED REST SERVICE Module.Name ADD RESOURCE 'items' { ... };\nALTER PUBLISHED REST SERVICE Module.Name DROP RESOURCE 'legacy';\nDROP PUBLISHED REST SERVICE Module.Name;", Example: "mdl 1;\nCREATE PUBLISHED REST SERVICE Module.OrderAPI (\n Path: 'rest/orders/v1',\n Version: '1.0.0',\n ServiceName: 'Order API',\n Authentication: (basic, session)\n)\n{\n RESOURCE 'orders' {\n GET '' MICROFLOW Module.GetAllOrders;\n GET '{id}' MICROFLOW Module.GetOrderById;\n POST '' MICROFLOW Module.CreateOrder;\n DELETE '{id}' MICROFLOW Module.DeleteOrder;\n }\n};\n\nGRANT ACCESS ON PUBLISHED REST SERVICE Module.OrderAPI\n TO Module.User, Module.Admin;", SeeAlso: []string{"rest", "rest.consumed"}, @@ -784,6 +787,7 @@ DESCRIBE DATABASE CONNECTION Ops.Erp;`, "object handling", "converter", "value transform", "custom handling", "by microflow", }, Syntax: "LIST IMPORT MAPPINGS [IN Module];\nDESCRIBE IMPORT MAPPING Module.Name;\n" + + "[/** documentation */]\n" + "CREATE [OR MODIFY] IMPORT MAPPING Module.Name [FOLDER 'path']\n" + " WITH JSON STRUCTURE Module.JsonStruct [ROOT a/b/c]\n" + " | WITH MESSAGE DEFINITION Module.Collection.Definition\n" + @@ -794,6 +798,8 @@ DESCRIBE DATABASE CONNECTION Ops.Erp;`, " Attr = a/b/c,\n" + " Assoc/Module.Child = nestedKey { ... }\n }\n};\nDROP IMPORT MAPPING Module.Name;\n\n" + "OR MODIFY: updates mapping in-place, preserves UUID.\n\n" + + "Documentation: the /** ... */ doc comment above CREATE (QUAL002 asks for\n" + + "it). On OR MODIFY no comment keeps the stored text, /** */ clears it.\n\n" + "Nested members (Attr = a/b/c):\n" + " Reaches a leaf several levels down with NO entity for the levels in\n" + " between — the shape Studio Pro produces when you tick a nested leaf\n" + @@ -848,7 +854,9 @@ DESCRIBE DATABASE CONNECTION Ops.Erp;`, "with json structure", "with message definition", "null values", "as jsonKey", "converter", "value transform", }, - Syntax: "LIST EXPORT MAPPINGS [IN Module];\nDESCRIBE EXPORT MAPPING Module.Name;\nCREATE [OR MODIFY] EXPORT MAPPING Module.Name [FOLDER 'path']\n WITH JSON STRUCTURE Module.JsonStruct [ROOT a/b/c]\n | WITH MESSAGE DEFINITION Module.Collection.Definition\n | WITH XML SCHEMA Module.Schema\n [NULL VALUES LeaveOutElement|SendAsNil]\n{\n Module.Entity {\n jsonField = Attr,\n jsonField = Module.MF(Attr), -- value transform\n group AS key { ... }, -- entity-less grouping node\n Assoc/Module.Child AS nestedKey { ... }\n }\n};\nDROP EXPORT MAPPING Module.Name;\n\nOR MODIFY: updates mapping in-place, preserves UUID.\n\n" + + Syntax: "LIST EXPORT MAPPINGS [IN Module];\nDESCRIBE EXPORT MAPPING Module.Name;\n[/** documentation */]\nCREATE [OR MODIFY] EXPORT MAPPING Module.Name [FOLDER 'path']\n WITH JSON STRUCTURE Module.JsonStruct [ROOT a/b/c]\n | WITH MESSAGE DEFINITION Module.Collection.Definition\n | WITH XML SCHEMA Module.Schema\n [NULL VALUES LeaveOutElement|SendAsNil]\n{\n Module.Entity {\n jsonField = Attr,\n jsonField = Module.MF(Attr), -- value transform\n group AS key { ... }, -- entity-less grouping node\n Assoc/Module.Child AS nestedKey { ... }\n }\n};\nDROP EXPORT MAPPING Module.Name;\n\nOR MODIFY: updates mapping in-place, preserves UUID.\n\n" + + "Documentation: the /** ... */ doc comment above CREATE, as on an import\n" + + "mapping. On OR MODIFY no comment keeps the stored text, /** */ clears it.\n\n" + "Grouping nodes (group as key { ... }):\n" + " A JSON object with no Mendix object behind it — Studio Pro's\n" + " entity-less object element. It may hold OBJECT elements only: a\n" + @@ -874,7 +882,7 @@ DESCRIBE DATABASE CONNECTION Ops.Erp;`, "data transformer", "create data transformer", "drop data transformer", "list data transformers", "jslt", "xslt", "transform", }, - Syntax: "LIST DATA TRANSFORMERS [IN Module];\nDESCRIBE DATA TRANSFORMER Module.Name;\nCREATE [OR MODIFY] DATA TRANSFORMER Module.Name [FOLDER 'path']\n SOURCE JSON '{ ... }'\n{\n JSLT 'single-line-expression';\n -- or multi-line:\n JSLT $$\n{ ... }\n $$;\n};\nDROP DATA TRANSFORMER Module.Name;\n\nOR MODIFY: updates transformer in-place, preserves UUID.", + Syntax: "LIST DATA TRANSFORMERS [IN Module];\nDESCRIBE DATA TRANSFORMER Module.Name;\n[/** documentation */]\nCREATE [OR MODIFY] DATA TRANSFORMER Module.Name [FOLDER 'path']\n SOURCE JSON '{ ... }'\n{\n JSLT 'single-line-expression';\n -- or multi-line:\n JSLT $$\n{ ... }\n $$;\n};\nDROP DATA TRANSFORMER Module.Name;\n\nOR MODIFY: updates transformer in-place, preserves UUID.\n\nDocumentation: the /** ... */ doc comment above CREATE. On OR MODIFY no\ncomment keeps the stored text, /** */ clears it.", Example: "mdl 1;\nCREATE DATA TRANSFORMER ETL.FlattenOrder\n SOURCE JSON '{\"order\": {\"id\": 1, \"total\": 99.0}}'\n{\n JSLT '{\"id\": .order.id, \"total\": .order.total}';\n};\n\n-- Multi-line JSLT\nCREATE OR MODIFY DATA TRANSFORMER ETL.WeatherSummary\n SOURCE JSON '{\"current\": {\"temp\": 12.8, \"wind\": 18.3}}'\n{\n JSLT $$\n{\n \"temperature\": .current.temp,\n \"wind_speed\": .current.wind\n}\n $$;\n};", SeeAlso: []string{"integration"}, }) diff --git a/docs-site/src/examples/rest-integration.md b/docs-site/src/examples/rest-integration.md index f8bbe03414..465286b4dc 100644 --- a/docs-site/src/examples/rest-integration.md +++ b/docs-site/src/examples/rest-integration.md @@ -309,6 +309,7 @@ CREATE OR MODIFY CONSUMED REST SERVICE Integration.OrdersApi ( Create a published REST service with CRUD operations backed by microflows. ```sql +/** Order management API for the shop front end. */ CREATE PUBLISHED REST SERVICE Module.OrderAPI ( Path: 'rest/orders/v1', Version: '1.0.0', @@ -325,6 +326,8 @@ CREATE PUBLISHED REST SERVICE Module.OrderAPI ( }; ``` +**Documentation:** the `/** … */` doc comment above `CREATE` is the service's documentation — what `lint` QUAL002 asks for. On `CREATE OR MODIFY` no comment keeps the stored text and `/** */` clears it. The OpenAPI description consumers read (`PublicDocumentation`) has no MDL spelling and is kept as stored. + **Operation paths:** Use empty string `''` for the root, `'{paramName}'` for path parameters. Do NOT start or end with `/`. Path parameters must match a microflow parameter name exactly (case-sensitive) — `'{id}'` binds the microflow's `$id`, whatever its type. **Operation parameters** come from the microflow, the way Studio Pro derives them. Create the microflow before the service: diff --git a/docs-site/src/reference/integration/create-data-transformer.md b/docs-site/src/reference/integration/create-data-transformer.md index 4f6b7dde1a..30c53ddbe8 100644 --- a/docs-site/src/reference/integration/create-data-transformer.md +++ b/docs-site/src/reference/integration/create-data-transformer.md @@ -3,6 +3,7 @@ ## Synopsis ```sql +[ /** description */ ] CREATE [ OR MODIFY ] DATA TRANSFORMER module.Name [ FOLDER 'folder_path' ] SOURCE { JSON | XML } 'sample' @@ -41,6 +42,9 @@ The `SOURCE` clause provides a representative input document. Mendix uses it to `FOLDER 'folder_path'` : Optional. Places the document in the named module folder, creating missing folders in the path. On `CREATE OR MODIFY` this **moves** an existing document; omitting the clause leaves placement alone rather than returning the document to the module root. See [MOVE](../organization/move.md). +`/** description */` +: Optional. The documentation of the data transformer, as a doc comment before the statement. `lint` QUAL002 reports a data transformer without it, and `DESCRIBE` prints it back. On `CREATE OR MODIFY`, no doc comment keeps the stored documentation and an empty `/** */` clears it. + `SOURCE JSON 'sample'` : A representative JSON input document. Multi-line samples can use `$$...$$` quoting. diff --git a/docs-site/src/reference/integration/create-export-mapping.md b/docs-site/src/reference/integration/create-export-mapping.md index a665bf384a..e4a84956ca 100644 --- a/docs-site/src/reference/integration/create-export-mapping.md +++ b/docs-site/src/reference/integration/create-export-mapping.md @@ -3,6 +3,7 @@ ## Synopsis ```sql +[ /** description */ ] CREATE [ OR MODIFY ] EXPORT MAPPING module.Name [ FOLDER 'folder_path' ] [ WITH JSON STRUCTURE module.JsonStructure ] @@ -57,6 +58,9 @@ If `OR MODIFY` is specified and the mapping already exists, it is updated in pla `FOLDER 'folder_path'` : Optional. Places the document in the named module folder, creating missing folders in the path. On `CREATE OR MODIFY` this **moves** an existing document; omitting the clause leaves placement alone rather than returning the document to the module root. See [MOVE](../organization/move.md). +`/** description */` +: Optional. The documentation of the export mapping, as a doc comment before the statement. `lint` QUAL002 reports a export mapping without it, and `DESCRIBE` prints it back. On `CREATE OR MODIFY`, no doc comment keeps the stored documentation and an empty `/** */` clears it. + `WITH JSON STRUCTURE module.JsonStructure` : Associates the mapping with the named JSON structure. diff --git a/docs-site/src/reference/integration/create-import-mapping.md b/docs-site/src/reference/integration/create-import-mapping.md index b38366be66..d412b34674 100644 --- a/docs-site/src/reference/integration/create-import-mapping.md +++ b/docs-site/src/reference/integration/create-import-mapping.md @@ -3,6 +3,7 @@ ## Synopsis ```sql +[ /** description */ ] CREATE [ OR MODIFY ] IMPORT MAPPING module.Name [ FOLDER 'folder_path' ] [ WITH JSON STRUCTURE module.JsonStructure ] @@ -80,6 +81,9 @@ If `OR MODIFY` is specified and the mapping already exists, it is updated in pla `FOLDER 'folder_path'` : Optional. Places the document in the named module folder, creating missing folders in the path. On `CREATE OR MODIFY` this **moves** an existing document; omitting the clause leaves placement alone rather than returning the document to the module root. See [MOVE](../organization/move.md). +`/** description */` +: Optional. The documentation of the import mapping, as a doc comment before the statement. `lint` QUAL002 reports a import mapping without it, and `DESCRIBE` prints it back. On `CREATE OR MODIFY`, no doc comment keeps the stored documentation and an empty `/** */` clears it. + `WITH JSON STRUCTURE module.JsonStructure` : Associates the mapping with the named JSON structure. The structure defines the JSON shape. diff --git a/mdl-examples/bug-tests/1510-documentation-for-mappings-rest-javaparams.mdl b/mdl-examples/bug-tests/1510-documentation-for-mappings-rest-javaparams.mdl new file mode 100644 index 0000000000..b429548d76 --- /dev/null +++ b/mdl-examples/bug-tests/1510-documentation-for-mappings-rest-javaparams.mdl @@ -0,0 +1,68 @@ +-- mendixlabs/mxcli#1510: `lint` QUAL002 asks for documentation on import and +-- export mappings, published REST services, data transformers and Java action +-- parameters. MDL could not write the first four — the doc comment above their +-- create was dropped with MDL089 — so the findings were permanent. After the +-- fix each doc comment below is stored as the document's Documentation (a Java +-- action parameter's as its Description), `describe` prints it back, and QUAL002 +-- reports none of these elements. +-- +-- Check: exec, then `describe` each document, then `lint` — no QUAL002 for +-- BugTest1510. The data transformer needs Mendix 11.9+; drop it on older ones. +mdl 1; + +create module BugTest1510; + +/** One order line as the shop sends it. */ +create persistent entity BugTest1510.OrderLine (Code: String(20), Amount: Integer); + +/** The order line payload of the shop API. */ +create json structure BugTest1510.JSON_OrderLine sample '{"code": "A1", "amount": 2}'; + +/** + * Reads an order line from the shop API. + * + * One object per payload. + */ +create import mapping BugTest1510.IMM_OrderLine + with json structure BugTest1510.JSON_OrderLine +{ + create BugTest1510.OrderLine { + Code = code, + Amount = amount + } +}; + +/** Writes an order line for the shop API. */ +create export mapping BugTest1510.EMM_OrderLine + with json structure BugTest1510.JSON_OrderLine +{ + BugTest1510.OrderLine { + code = Code, + amount = Amount + } +}; + +/** The order line API other apps call. */ +create published rest service BugTest1510.OrderLineApi ( + Path: 'rest/orderlines/v1', + Version: '1.0.0', + ServiceName: 'Order lines' +) +{ +}; + +/** Formats an amount for display. */ +create java action BugTest1510.FormatAmount ( + /** The amount in cents. */ + Amount: Integer not null +) returns String +as $$ +return String.valueOf(Amount); +$$; + +/** Renames the amount field. */ +create data transformer BugTest1510.DT_Rename + source json '{"amount": 1}' +{ + jslt '{"total": .amount}'; +}; diff --git a/mdl/ast/ast_datatransformer.go b/mdl/ast/ast_datatransformer.go index bc4958d62d..59d4564320 100644 --- a/mdl/ast/ast_datatransformer.go +++ b/mdl/ast/ast_datatransformer.go @@ -13,6 +13,11 @@ type CreateDataTransformerStmt struct { SourceJSON string // the source content Steps []DataTransformerStepDef CreateOrModify bool // true for CREATE OR MODIFY / CREATE OR REPLACE + // Documentation is the `/** … */` doc comment before the statement + // (mendixlabs/mxcli#1510). DocumentationSet says one was written: absent + // keeps the stored documentation on a rewrite, empty clears it (#1018). + Documentation string + DocumentationSet bool } func (s *CreateDataTransformerStmt) isStatement() {} diff --git a/mdl/ast/ast_import_export_mapping.go b/mdl/ast/ast_import_export_mapping.go index ef25a666ca..9455594107 100644 --- a/mdl/ast/ast_import_export_mapping.go +++ b/mdl/ast/ast_import_export_mapping.go @@ -32,6 +32,11 @@ type CreateImportMappingStmt struct { RootElement *ImportMappingElementDef CreateOrModify bool // true for CREATE OR MODIFY / CREATE OR REPLACE Excluded bool // @excluded — document excluded from project + // Documentation is the `/** … */` doc comment before the statement + // (mendixlabs/mxcli#1510). DocumentationSet says one was written: absent + // keeps the stored documentation on a rewrite, empty clears it (#1018). + Documentation string + DocumentationSet bool } func (s *CreateImportMappingStmt) isStatement() {} @@ -112,6 +117,9 @@ type CreateExportMappingStmt struct { RootElement *ExportMappingElementDef CreateOrModify bool // true for CREATE OR MODIFY / CREATE OR REPLACE Excluded bool // @excluded — document excluded from project + // Documentation — see the note on CreateImportMappingStmt (#1510). + Documentation string + DocumentationSet bool } func (s *CreateExportMappingStmt) isStatement() {} diff --git a/mdl/ast/ast_rest.go b/mdl/ast/ast_rest.go index 3a442261d9..6ce15e8a33 100644 --- a/mdl/ast/ast_rest.go +++ b/mdl/ast/ast_rest.go @@ -118,6 +118,11 @@ type CreatePublishedRestServiceStmt struct { Authentication *PublishedRestAuthentication Resources []*PublishedRestResourceDef CreateOrModify bool + // Documentation is the `/** … */` doc comment before the statement + // (mendixlabs/mxcli#1510). DocumentationSet says one was written: absent + // keeps the stored documentation on a rewrite, empty clears it (#1018). + Documentation string + DocumentationSet bool } // PublishedRestAuthentication is `Authentication: none | ( method, … )`. diff --git a/mdl/backend/modelsdk/data_transformer_write.go b/mdl/backend/modelsdk/data_transformer_write.go index 2aaf5e1fef..e4cff3f1e6 100644 --- a/mdl/backend/modelsdk/data_transformer_write.go +++ b/mdl/backend/modelsdk/data_transformer_write.go @@ -26,7 +26,8 @@ func (b *Backend) ListDataTransformers() ([]*model.DataTransformer, error) { out := make([]*model.DataTransformer, 0, len(units)) for _, u := range units { g := u.Element - dt := &model.DataTransformer{ContainerID: model.ID(u.ContainerID), Name: g.Name(), Excluded: g.Excluded()} + dt := &model.DataTransformer{ContainerID: model.ID(u.ContainerID), Name: g.Name(), Excluded: g.Excluded(), + Documentation: g.Documentation()} // mendixlabs/mxcli#1510 dt.ID = model.ID(g.ID()) dt.TypeName = "DataTransformers$DataTransformer" if src := g.Source(); src != nil { @@ -154,7 +155,7 @@ func dataTransformerToGen(dt *model.DataTransformer) element.Element { g := newElem("DataTransformers$DataTransformer", string(dt.ID)) addStr(g, "Name", dt.Name) - addStr(g, "Documentation", "") + addStr(g, "Documentation", dt.Documentation) addBool(g, "Excluded", dt.Excluded) addStr(g, "ExportLevel", "Hidden") addPart(g, "Source", source) diff --git a/mdl/backend/modelsdk/integration_read.go b/mdl/backend/modelsdk/integration_read.go index 5f1a0190b1..019a120fc9 100644 --- a/mdl/backend/modelsdk/integration_read.go +++ b/mdl/backend/modelsdk/integration_read.go @@ -411,6 +411,8 @@ func (b *Backend) ListPublishedRestServices() ([]*model.PublishedRestService, er ServiceName: g.ServiceName(), Excluded: g.Excluded(), AllowedRoles: append([]string(nil), g.AllowedRolesQualifiedNames()...), + // mendixlabs/mxcli#1510 + Documentation: g.Documentation(), AuthenticationTypes: append([]string(nil), g.AuthenticationTypesItems()...), AuthenticationMicroflow: g.AuthenticationMicroflowQualifiedName(), diff --git a/mdl/backend/modelsdk/published_rest_write.go b/mdl/backend/modelsdk/published_rest_write.go index d40a3a4edd..eb41a0773d 100644 --- a/mdl/backend/modelsdk/published_rest_write.go +++ b/mdl/backend/modelsdk/published_rest_write.go @@ -138,17 +138,17 @@ func (b *Backend) UpdatePublishedRestServiceRoles(unitID model.ID, roles []strin // output of a Studio Pro service turned its Basic and Session authentication // off (ako/mxcli#571). They are written from the model now, and an unstated // setting is carried by the executor, which reads it with the service. So are -// the authentication Parameters, since mendixlabs/mxcli#1417. +// the authentication Parameters, since mendixlabs/mxcli#1417, and the +// Documentation, since the doc comment states it (mendixlabs/mxcli#1510). var publishedRestServiceUnauthored = []string{ "CorsConfiguration", - "Documentation", "PublicDocumentation", } func publishedRestServiceToGen(svc *model.PublishedRestService) element.Element { g := newElem("Rest$PublishedRestService", string(svc.ID)) addStr(g, "Name", svc.Name) - addStr(g, "Documentation", "") + addStr(g, "Documentation", svc.Documentation) addBool(g, "Excluded", svc.Excluded) addStr(g, "ExportLevel", "Hidden") addStr(g, "Path", svc.Path) diff --git a/mdl/catalog/builder_rest.go b/mdl/catalog/builder_rest.go index 767a27601a..5b6f33a47c 100644 --- a/mdl/catalog/builder_rest.go +++ b/mdl/catalog/builder_rest.go @@ -178,7 +178,7 @@ func (b *Builder) buildPublishedRestServices() error { svc.ServiceName, len(svc.Resources), opCount, - "", // Documentation not stored on published REST + svc.Documentation, // was always "", so QUAL002 could never close (mendixlabs/mxcli#1510) projectID, snapshotID, ) if err != nil { diff --git a/mdl/catalog/builder_rest_documentation_test.go b/mdl/catalog/builder_rest_documentation_test.go new file mode 100644 index 0000000000..be3f6bfc36 --- /dev/null +++ b/mdl/catalog/builder_rest_documentation_test.go @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import "testing" + +// mendixlabs/mxcli#1510: the catalog wrote an empty Documentation for every +// published REST service ("not stored on published REST"), so QUAL002 reported +// each one as undocumented whatever Studio Pro or MDL had written — a finding +// nobody could close. The column carries the service's own documentation. +func TestPublishedRestServiceDocumentationReachesCatalog_Issue1510(t *testing.T) { + svc := oneService() + svc.Documentation = "The public orders API." + cat := restFixture(t, svc) + + rows := queryRows(t, cat, `SELECT Documentation FROM published_rest_services WHERE QualifiedName = 'Sales.OrdersApi'`) + if len(rows) != 1 { + t.Fatalf("want 1 service row, got %d", len(rows)) + } + if got := rows[0][0]; got != "The public orders API." { + t.Errorf("Documentation = %v, want the service's documentation", got) + } + + // Control: an undocumented service stays empty, so the rule still fires. + cat = restFixture(t, oneService()) + rows = queryRows(t, cat, `SELECT Documentation FROM published_rest_services`) + if len(rows) != 1 || rows[0][0] != "" { + t.Errorf("undocumented service: got %v, want one empty Documentation", rows) + } +} diff --git a/mdl/executor/cmd_datatransformer.go b/mdl/executor/cmd_datatransformer.go index ebbde4951b..44905ec503 100644 --- a/mdl/executor/cmd_datatransformer.go +++ b/mdl/executor/cmd_datatransformer.go @@ -75,7 +75,8 @@ func describeDataTransformer(ctx *ExecContext, name ast.QualifiedName) error { w := ctx.Output - // Emit re-executable MDL + // Emit re-executable MDL, the documentation first (mendixlabs/mxcli#1510). + writeDocumentDocComment(w, dt.Documentation) fmt.Fprintf(w, "create or modify data transformer %s.%s%s\n", modName, dt.Name, describeFolderClause(ctx, dt.ContainerID)) // Source — collapse newlines into spaces for single-line string @@ -137,10 +138,14 @@ func execCreateDataTransformer(ctx *ExecContext, s *ast.CreateDataTransformerStm Name: s.Name.Name, SourceType: s.SourceType, SourceJSON: s.SourceJSON, + // The doc comment (mendixlabs/mxcli#1510). + Documentation: s.Documentation, } if existing != nil { // Excluded is model state, not script state (#914). dt.Excluded = existing.Excluded + // A rewrite that carried no doc comment keeps the stored one (#1018). + dt.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation) } for _, step := range s.Steps { diff --git a/mdl/executor/cmd_export_mappings.go b/mdl/executor/cmd_export_mappings.go index 35fc037e7c..7be3d3fa25 100644 --- a/mdl/executor/cmd_export_mappings.go +++ b/mdl/executor/cmd_export_mappings.go @@ -94,9 +94,7 @@ func describeExportMapping(ctx *ExecContext, name ast.QualifiedName) error { return mdlerrors.NewBackend("get export mapping", err) } - if em.Documentation != "" { - fmt.Fprintf(ctx.Output, "/**\n * %s\n */\n", strings.ReplaceAll(em.Documentation, "\n", "\n * ")) - } + writeDocumentDocComment(ctx.Output, em.Documentation) // The same prefix a microflow or page carries: without it an excluded // mapping reads as live, and beside a live twin of the same name the two // are indistinguishable (#1185). @@ -328,11 +326,15 @@ func execCreateExportMapping(ctx *ExecContext, s *ast.CreateExportMappingStmt) e NullValueOption: nullValueOption, // @excluded, which DESCRIBE prints for an excluded mapping (#1185). Excluded: s.Excluded, + // The doc comment (mendixlabs/mxcli#1510). + Documentation: s.Documentation, } if existing != nil { // Excluded is model state, not script state: an absent @excluded must // not clear a stored exclusion (#914). em.Excluded = s.Excluded || existing.Excluded + // A rewrite that carried no doc comment keeps the stored one (#1018). + em.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation) } if em.NullValueOption == "" { em.NullValueOption = "LeaveOutElement" diff --git a/mdl/executor/cmd_import_mappings.go b/mdl/executor/cmd_import_mappings.go index 64a1be8a5b..9dd996e296 100644 --- a/mdl/executor/cmd_import_mappings.go +++ b/mdl/executor/cmd_import_mappings.go @@ -94,9 +94,7 @@ func describeImportMapping(ctx *ExecContext, name ast.QualifiedName) error { return mdlerrors.NewBackend("get import mapping", err) } - if im.Documentation != "" { - fmt.Fprintf(ctx.Output, "/**\n * %s\n */\n", strings.ReplaceAll(im.Documentation, "\n", "\n * ")) - } + writeDocumentDocComment(ctx.Output, im.Documentation) // The same prefix a microflow or page carries: without it an excluded // mapping reads as live, and beside a live twin of the same name the two // are indistinguishable (#1185). @@ -421,11 +419,15 @@ func execCreateImportMapping(ctx *ExecContext, s *ast.CreateImportMappingStmt) e ExportLevel: "Hidden", // @excluded, which DESCRIBE prints for an excluded mapping (#1185). Excluded: s.Excluded, + // The doc comment (mendixlabs/mxcli#1510). + Documentation: s.Documentation, } if existing != nil { // Excluded is model state, not script state: an absent @excluded must // not clear a stored exclusion (#914). im.Excluded = s.Excluded || existing.Excluded + // A rewrite that carried no doc comment keeps the stored one (#1018). + im.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation) } // The mapping's input object (#265), which `Param: parameter` binds. diff --git a/mdl/executor/cmd_published_rest.go b/mdl/executor/cmd_published_rest.go index 160c854eca..2538339632 100644 --- a/mdl/executor/cmd_published_rest.go +++ b/mdl/executor/cmd_published_rest.go @@ -105,6 +105,7 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro if folderPath := h.BuildFolderPath(svc.ContainerID); folderPath != "" { folder = " folder " + mdlQuote(ctx, folderPath) } + writeDocumentDocComment(ctx.Output, svc.Documentation) // mendixlabs/mxcli#1510 fmt.Fprintf(ctx.Output, "create or modify published rest service %s%s (\n", qualifiedName, folder) fmt.Fprintf(ctx.Output, " Path: %s", mdlQuoted(svc.Path)) if svc.Version != "" { @@ -577,8 +578,12 @@ func execCreatePublishedRestService(ctx *ExecContext, s *ast.CreatePublishedRest Path: s.Path, Version: s.Version, ServiceName: s.ServiceName, + // The doc comment (mendixlabs/mxcli#1510). + Documentation: s.Documentation, } if existing != nil { + // A rewrite that carried no doc comment keeps the stored one (#1018). + svc.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation) svc.ID = existing.ID svc.AllowedRoles = existing.AllowedRoles // Excluded is model state, not script state (#914). diff --git a/mdl/executor/documentation_carry.go b/mdl/executor/documentation_carry.go index 976fa1d69c..1094ebab4e 100644 --- a/mdl/executor/documentation_carry.go +++ b/mdl/executor/documentation_carry.go @@ -2,7 +2,11 @@ package executor -import "strings" +import ( + "fmt" + "io" + "strings" +) // carriedDocumentation returns the documentation a rewrite should store. // @@ -54,3 +58,27 @@ func docCommentNormalForm(s string) string { } return strings.Join(kept, "\n") } + +// writeDocumentDocComment prints a document's documentation as the `/** … */` +// doc comment its create reads (mendixlabs/mxcli#1510). A text holding `*/` +// would end the comment early and the rest would not parse, so it is shown as +// `--` lines instead: the statement then states none, and executing it keeps +// the stored text (#1018) rather than truncating it. +func writeDocumentDocComment(w io.Writer, doc string) { + if doc == "" { + return + } + doc = strings.ReplaceAll(doc, "\r\n", "\n") + if strings.Contains(doc, "*/") { + fmt.Fprintln(w, "-- Documentation (kept on execute; a doc comment cannot hold `*/`):") + for line := range strings.SplitSeq(doc, "\n") { + fmt.Fprintf(w, "-- %s\n", line) + } + return + } + fmt.Fprintln(w, "/**") + for line := range strings.SplitSeq(doc, "\n") { + fmt.Fprintln(w, strings.TrimRight(" * "+line, " ")) + } + fmt.Fprintln(w, " */") +} diff --git a/mdl/executor/documentation_coverage_test.go b/mdl/executor/documentation_coverage_test.go index e0ff3ecdec..8d1b1aa383 100644 --- a/mdl/executor/documentation_coverage_test.go +++ b/mdl/executor/documentation_coverage_test.go @@ -56,6 +56,12 @@ var docCarryDone = map[string]bool{ "CreateConsumedMCPServiceStmt": true, "CreateAgentStmt": true, "CreateModuleStmt": true, + // mendixlabs/mxcli#1510: covered by TestIntegrationDocumentation_Issue1510 + // (unit, on PedApp) as well as TestDocumentation_SurvivesRewrite. + "CreateImportMappingStmt": true, + "CreateExportMappingStmt": true, + "CreatePublishedRestServiceStmt": true, + "CreateDataTransformerStmt": true, } // docCarryPending is empty: every statement type in scope is carried AND diff --git a/mdl/executor/documentation_preserved_test.go b/mdl/executor/documentation_preserved_test.go index f1e7193fd8..120821e45d 100644 --- a/mdl/executor/documentation_preserved_test.go +++ b/mdl/executor/documentation_preserved_test.go @@ -274,6 +274,36 @@ func docPreserveCases() []docPreserveCase { rewrite: "create or replace enumeration TestModule.DocEnum ( A 'A', B 'B', C 'C' );", describe: "describe enumeration TestModule.DocEnum", }, + { + name: "import mapping", + create: "create entity TestModule.DocImEnt ( Code: String );\n" + + "create json structure TestModule.DocImJs sample '{\"code\": \"x\"}';\n" + doc + + "create import mapping TestModule.DocIm\n with json structure TestModule.DocImJs\n{\n create TestModule.DocImEnt {\n Code = code\n }\n};", + rewrite: "create or modify import mapping TestModule.DocIm\n with json structure TestModule.DocImJs\n{\n find TestModule.DocImEnt or create {\n Code = code key\n }\n};", + describe: "describe import mapping TestModule.DocIm", + }, + { + name: "export mapping", + create: "create entity TestModule.DocEmEnt ( Code: String );\n" + + "create json structure TestModule.DocEmJs sample '{\"code\": \"x\"}';\n" + doc + + "create export mapping TestModule.DocEm\n with json structure TestModule.DocEmJs\n{\n TestModule.DocEmEnt {\n code = Code\n }\n};", + rewrite: "create or modify export mapping TestModule.DocEm\n with json structure TestModule.DocEmJs\n null values SendAsNil\n{\n TestModule.DocEmEnt {\n code = Code\n }\n};", + describe: "describe export mapping TestModule.DocEm", + }, + { + name: "published rest service", + create: doc + "create published rest service MyFirstModule.DocPrs (\n Path: 'rest/doc/v1',\n Version: '1.0.0'\n)\n{\n};", + rewrite: "create or modify published rest service MyFirstModule.DocPrs (\n Path: 'rest/doc/v2',\n Version: '2.0.0'\n)\n{\n};", + describe: "describe published rest service MyFirstModule.DocPrs", + }, + { + name: "data transformer", + minMajor: 11, + minMinor: 9, + create: doc + "create data transformer MyFirstModule.DocDt\n source json '{\"a\": 1}'\n{\n jslt '{\"b\": .a}';\n};", + rewrite: "create or modify data transformer MyFirstModule.DocDt\n source json '{\"a\": 2}'\n{\n jslt '{\"c\": .a}';\n};", + describe: "describe data transformer MyFirstModule.DocDt", + }, { // The module's documentation is its domain model's // (mendixlabs/mxcli#1314). diff --git a/mdl/executor/integration_documentation_issue1510_test.go b/mdl/executor/integration_documentation_issue1510_test.go new file mode 100644 index 0000000000..10a2e924fc --- /dev/null +++ b/mdl/executor/integration_documentation_issue1510_test.go @@ -0,0 +1,199 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1510: QUAL002 reports import and export mappings, +// published REST services and data transformers without documentation, and MDL +// had no way to write it — the doc comment above their create was dropped with +// MDL089. Worse, a rewrite wrote an empty Documentation over whatever Studio Pro +// had stored, so copying a mapping through describe -> exec deleted its text. +// +// Measured on a copy of the Studio Pro-authored PedApp (11.13), with the real +// engine, so what is asserted is the stored document's own `Documentation` key. + +type issue1510Doctype struct { + name string + create string // without its doc comment + rewrite string // a create or modify that states none + describe string + unitID func(t *testing.T, ctx *ExecContext) model.ID +} + +func issue1510Doctypes() []issue1510Doctype { + return []issue1510Doctype{{ + name: "import mapping", + create: "create import mapping MyFirstModule.IMM_Doc1510\n with json structure MyFirstModule.JSON_Doc1510\n" + + "{\n create MyFirstModule.Doc1510 {\n Code = code\n }\n};", + rewrite: "create or modify import mapping MyFirstModule.IMM_Doc1510\n with json structure MyFirstModule.JSON_Doc1510\n" + + "{\n create MyFirstModule.Doc1510 {\n Code = code,\n Amount = amount\n }\n};", + describe: "describe import mapping MyFirstModule.IMM_Doc1510", + unitID: func(t *testing.T, ctx *ExecContext) model.ID { + m, err := ctx.Backend.GetImportMappingByQualifiedName("MyFirstModule", "IMM_Doc1510") + if err != nil { + t.Fatal(err) + } + return m.ID + }, + }, { + name: "export mapping", + create: "create export mapping MyFirstModule.EMM_Doc1510\n with json structure MyFirstModule.JSON_Doc1510\n" + + "{\n MyFirstModule.Doc1510 {\n code = Code\n }\n};", + rewrite: "create or modify export mapping MyFirstModule.EMM_Doc1510\n with json structure MyFirstModule.JSON_Doc1510\n" + + "{\n MyFirstModule.Doc1510 {\n code = Code,\n amount = Amount\n }\n};", + describe: "describe export mapping MyFirstModule.EMM_Doc1510", + unitID: func(t *testing.T, ctx *ExecContext) model.ID { + m, err := ctx.Backend.GetExportMappingByQualifiedName("MyFirstModule", "EMM_Doc1510") + if err != nil { + t.Fatal(err) + } + return m.ID + }, + }, { + name: "published rest service", + create: "create published rest service MyFirstModule.Api_Doc1510 (\n Path: 'rest/doc1510/v1',\n Version: '1.0.0'\n)\n{\n};", + rewrite: "create or modify published rest service MyFirstModule.Api_Doc1510 (\n Path: 'rest/doc1510/v2',\n Version: '2.0.0'\n)\n{\n};", + describe: "describe published rest service MyFirstModule.Api_Doc1510", + unitID: func(t *testing.T, ctx *ExecContext) model.ID { + svc, err := findPublishedRestService(ctx, "MyFirstModule", "Api_Doc1510") + if err != nil { + t.Fatal(err) + } + return svc.ID + }, + }, { + name: "data transformer", + create: "create data transformer MyFirstModule.DT_Doc1510\n source json '{\"a\": 1}'\n{\n jslt '{\"b\": .a}';\n};", + rewrite: "create or modify data transformer MyFirstModule.DT_Doc1510\n source json '{\"a\": 2}'\n{\n jslt '{\"c\": .a}';\n};", + describe: "describe data transformer MyFirstModule.DT_Doc1510", + unitID: func(t *testing.T, ctx *ExecContext) model.ID { + _, id := findDataTransformer(ctx, "MyFirstModule", "DT_Doc1510") + if id == "" { + t.Fatal("data transformer not found") + } + return id + }, + }} +} + +const issue1510Prereqs = "create persistent entity MyFirstModule.Doc1510 (Code: String(20), Amount: Integer);\n" + + "create json structure MyFirstModule.JSON_Doc1510 sample '{\"code\": \"x\", \"amount\": 1}';\n" + +// storedDocumentation is the unit's top-level Documentation key, and whether +// the key is there at all. +func storedDocumentation(t *testing.T, ctx *ExecContext, id model.ID) (string, bool) { + t.Helper() + raw, err := ctx.Backend.GetRawUnitBytes(id) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + for _, e := range d { + if e.Key == "Documentation" { + s, _ := e.Value.(string) + return s, true + } + } + return "", false +} + +func TestIntegrationDocumentation_Issue1510(t *testing.T) { + const doc = "Doc 1510: the orders payload.\n\nSecond paragraph." + const comment = "/**\n * Doc 1510: the orders payload.\n *\n * Second paragraph.\n */\n" + for _, tc := range issue1510Doctypes() { + t.Run(tc.name, func(t *testing.T) { + exec, out := openPedAppFixture(t) + ctx := exec.newExecContext(context.Background()) + run := func(src string) { + t.Helper() + if err := afRun(t, exec, src); err != nil { + t.Fatalf("%v\n%s", err, src) + } + } + describe := func() string { + t.Helper() + out.Reset() + run(tc.describe + ";") + return out.String() + } + run(issue1510Prereqs) + + // 1. The doc comment reaches the stored document's Documentation. + run(comment + tc.create) + id := tc.unitID(t, ctx) + if got, ok := storedDocumentation(t, ctx, id); !ok || got != doc { + t.Fatalf("stored Documentation = %q (key present %v), want %q", got, ok, doc) + } + + // 2. Describe prints it back as the doc comment, and the output + // re-executes to the same text: the round trip. + described := describe() + if !strings.Contains(described, comment) { + t.Fatalf("describe does not print the documentation as a doc comment:\n%s", described) + } + run(described) + if got, _ := storedDocumentation(t, ctx, id); got != doc { + t.Errorf("describe -> exec changed the documentation to %q", got) + } + + // 3. A rewrite that states none keeps the stored text (#1018). + run(tc.rewrite) + if got, _ := storedDocumentation(t, ctx, id); got != doc { + t.Errorf("a rewrite with no doc comment changed the documentation to %q, want it kept", got) + } + + // 4. Control: an empty doc comment clears it, so the carry is the + // absence of a comment and not a write path that ignores the text. + run("/** */\n" + tc.rewrite) + if got, ok := storedDocumentation(t, ctx, id); !ok || got != "" { + t.Errorf("an empty doc comment left Documentation = %q (key present %v), want empty", got, ok) + } + }) + } +} + +// The Studio Pro-authored mappings in PedApp: a rewrite through describe -> +// exec, and one with no doc comment, must not delete documentation Studio Pro +// wrote. Before #1510 the executor never set it, so every rewrite stored "". +func TestIntegrationDocumentation_StudioProMappingKeepsIt_Issue1510(t *testing.T) { + exec, out := openPedAppFixture(t) + ctx := exec.newExecContext(context.Background()) + im, err := ctx.Backend.GetImportMappingByQualifiedName("FeedbackModule", "IMM_PostResponse") + if err != nil { + t.Fatalf("fixture mapping: %v", err) + } + // Give the Studio Pro mapping documentation the way Studio Pro would store + // it, without going through the executor being tested. + im.Documentation = "Written in Studio Pro." + if err := ctx.Backend.UpdateImportMapping(im); err != nil { + t.Fatal(err) + } + out.Reset() + if err := afRun(t, exec, "describe import mapping FeedbackModule.IMM_PostResponse;"); err != nil { + t.Fatal(err) + } + described := out.String() + if !strings.Contains(described, "Written in Studio Pro.") { + t.Fatalf("precondition: describe does not show the documentation:\n%s", described) + } + // Strip the doc comment: the statement now states none. + body := described[strings.Index(described, "create or modify"):] + if err := afRun(t, exec, body); err != nil { + t.Fatalf("re-execute describe body: %v\n%s", err, body) + } + if got, _ := storedDocumentation(t, ctx, im.ID); got != "Written in Studio Pro." { + t.Errorf("a rewrite with no doc comment changed Studio Pro's documentation to %q", got) + } +} diff --git a/mdl/visitor/visitor_datatransformer.go b/mdl/visitor/visitor_datatransformer.go index 169b20ab9e..9144ae9c92 100644 --- a/mdl/visitor/visitor_datatransformer.go +++ b/mdl/visitor/visitor_datatransformer.go @@ -17,6 +17,7 @@ func (b *Builder) ExitCreateDataTransformerStatement(ctx *parser.CreateDataTrans stmt := &ast.CreateDataTransformerStmt{ Name: buildQualifiedName(qn), } + stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) // mendixlabs/mxcli#1510 // Source type: JSON or XML if ctx.JSON() != nil { diff --git a/mdl/visitor/visitor_import_export_mapping.go b/mdl/visitor/visitor_import_export_mapping.go index d378ceb58d..7c4e52220d 100644 --- a/mdl/visitor/visitor_import_export_mapping.go +++ b/mdl/visitor/visitor_import_export_mapping.go @@ -16,6 +16,7 @@ func (b *Builder) ExitCreateImportMappingStatement(ctx *parser.CreateImportMappi stmt := &ast.CreateImportMappingStmt{ Name: buildQualifiedName(ctx.QualifiedName()), } + stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) // mendixlabs/mxcli#1510 if lit := ctx.STRING_LITERAL(); lit != nil { stmt.Folder = unquoteStringLit(lit) } @@ -156,6 +157,7 @@ func (b *Builder) ExitCreateExportMappingStatement(ctx *parser.CreateExportMappi stmt := &ast.CreateExportMappingStmt{ Name: buildQualifiedName(ctx.QualifiedName()), } + stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) // mendixlabs/mxcli#1510 if lit := ctx.STRING_LITERAL(); lit != nil { stmt.Folder = unquoteStringLit(lit) } diff --git a/mdl/visitor/visitor_integration_documentation_test.go b/mdl/visitor/visitor_integration_documentation_test.go new file mode 100644 index 0000000000..39b887215d --- /dev/null +++ b/mdl/visitor/visitor_integration_documentation_test.go @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// mendixlabs/mxcli#1510: QUAL002 asks for documentation on import and export +// mappings, published REST services and data transformers, and a `/** … */` +// doc comment above their create was dropped with MDL089 — the statements had +// nowhere to put it, so the finding could never be closed from MDL. Each now +// reads the comment, and records whether one was written at all, so a rewrite +// that states none keeps the stored documentation (#1018). +func TestIntegrationDocumentsReadDocComment_Issue1510(t *testing.T) { + for _, tc := range []struct { + name string + src string + doc func(ast.Statement) (string, bool) + }{{ + name: "import mapping", + src: "create import mapping M.IMM_Order\n with json structure M.JSON_Order\n" + + "{\n create M.Order {\n Id = id\n }\n};", + doc: func(s ast.Statement) (string, bool) { + st := s.(*ast.CreateImportMappingStmt) + return st.Documentation, st.DocumentationSet + }, + }, { + name: "export mapping", + src: "create export mapping M.EMM_Order\n with json structure M.JSON_Order\n" + + "{\n M.Order {\n id = Id\n }\n};", + doc: func(s ast.Statement) (string, bool) { + st := s.(*ast.CreateExportMappingStmt) + return st.Documentation, st.DocumentationSet + }, + }, { + name: "published rest service", + src: "create or modify published rest service M.OrdersApi (\n Path: 'rest/orders/v1'\n)\n{\n};", + doc: func(s ast.Statement) (string, bool) { + st := s.(*ast.CreatePublishedRestServiceStmt) + return st.Documentation, st.DocumentationSet + }, + }, { + name: "data transformer", + src: "create data transformer M.Flatten\n source json '{\"a\": 1}'\n{\n jslt '{\"b\": .a}';\n};", + doc: func(s ast.Statement) (string, bool) { + st := s.(*ast.CreateDataTransformerStmt) + return st.Documentation, st.DocumentationSet + }, + }} { + t.Run(tc.name, func(t *testing.T) { + prog, errs := Build("/** Line one.\n *\n * Line two. */\n" + tc.src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if len(prog.DetachedDocComments) != 0 { + t.Errorf("doc comment reported as lost (MDL089): %+v", prog.DetachedDocComments) + } + if len(prog.Statements) != 1 { + t.Fatalf("got %d statements, want 1", len(prog.Statements)) + } + doc, set := tc.doc(prog.Statements[0]) + if !set || doc != "Line one.\n\nLine two." { + t.Errorf("documentation = %q (set %v), want %q", doc, set, "Line one.\n\nLine two.") + } + + // Control: no comment is "not stated", which a rewrite must read as + // "keep what is stored", not as "clear it". + prog, errs = Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if doc, set := tc.doc(prog.Statements[0]); set || doc != "" { + t.Errorf("without a comment: documentation = %q (set %v), want unset", doc, set) + } + }) + } +} diff --git a/mdl/visitor/visitor_rest.go b/mdl/visitor/visitor_rest.go index a260f55be2..8a90ac38af 100644 --- a/mdl/visitor/visitor_rest.go +++ b/mdl/visitor/visitor_rest.go @@ -349,6 +349,7 @@ func (b *Builder) ExitCreatePublishedRestServiceStatement(ctx *parser.CreatePubl stmt := &ast.CreatePublishedRestServiceStmt{ Name: buildQualifiedName(ctx.QualifiedName()), } + stmt.Documentation, stmt.DocumentationSet = findDocComment(ctx) // mendixlabs/mxcli#1510 // `folder '…'` after the name (R9); `Folder:` in the list is its alias. folderClause := ctx.STRING_LITERAL() != nil if folderClause { diff --git a/model/types.go b/model/types.go index deb794c9d9..a075ca33fb 100644 --- a/model/types.go +++ b/model/types.go @@ -793,6 +793,11 @@ type PublishedRestService struct { Excluded bool `json:"excluded,omitempty"` AllowedRoles []string `json:"allowedRoles,omitempty"` Resources []*PublishedRestResource `json:"resources,omitempty"` + // Documentation is the service's own Documentation property — the + // developer-facing text Studio Pro shows on the document, not + // PublicDocumentation, the OpenAPI description consumers read + // (mendixlabs/mxcli#1510). + Documentation string `json:"documentation,omitempty"` // AuthenticationTypes are the methods a caller may authenticate with, in // the order stored: "Basic", "Session", "Microflow". Empty is "Requires @@ -989,6 +994,9 @@ type DataTransformer struct { SourceJSON string `json:"sourceJson,omitempty"` // source content Steps []*DataTransformerStep `json:"steps,omitempty"` Excluded bool `json:"excluded,omitempty"` + // Documentation is the transformer's Documentation property + // (mendixlabs/mxcli#1510). + Documentation string `json:"documentation,omitempty"` } // GetName returns the transformer's name. From ff2eeb0985666f91867df85104341289ba13f67f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 08:16:29 +0000 Subject: [PATCH 2/2] fix(check): MDL089 no longer sends a create's doc comment to the next create (mendixlabs/mxcli#1510) A doc comment above a create that stores no documentation was warned with "Move the doc comment directly above `create json structure ...`, after the `create or modify published rest service ...`" -- moving text written for one element onto a different one. The next-statement suggestion is right when a drop, grant, revoke, set or alter stands between a comment and its create (ako/mxcli#877), but a create is itself the element the comment describes. For a create, MDL089 now says MDL cannot store documentation for that element and suggests a `--` comment; the move suggestion stays for the other statements. The check-syntax skill says the same (and no longer lists `create module`, which stores documentation since #1314). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W3x8Fyvir8kLyPgJs1BDmF --- ...-moving-doc-comment-to-another-create.json | 1 + .claude/skills/mendix/check-syntax/SKILL.md | 6 ++-- .../validate_detached_doc_comments.go | 21 +++++++++-- .../validate_detached_doc_comments_test.go | 35 +++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-16-mdl089-suggests-moving-doc-comment-to-another-create.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-mdl089-suggests-moving-doc-comment-to-another-create.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-mdl089-suggests-moving-doc-comment-to-another-create.json new file mode 100644 index 0000000000..70f6e083fc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-mdl089-suggests-moving-doc-comment-to-another-create.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-16", "symptom": "mendixlabs/mxcli#1510: MDL089 on a doc comment above a create that stores no documentation told the user to \"Move the doc comment directly above `create json structure …`, after the `create or modify published rest service …`\" — moving text written for one element onto a different one.", "cause": "ValidateDetachedDocComments built the move suggestion from DetachedDocComment.Next for every statement. That is right for a drop/grant standing between a comment and its create (#877), but a create IS the element the comment describes; the next storing statement is a different element.", "file": "mdl/executor/validate_detached_doc_comments.go", "fix": "When the statement head is a create, the warning says MDL cannot store documentation for that element and suggests a `--` comment; the next-statement suggestion stays for drop/grant/revoke/set/alter.", "test": "mdl/executor TestValidateProgram_DetachedDocCommentOnCreate_Issue1510 (create module role followed by a create entity); control TestValidateProgram_DetachedDocComment keeps the drop suggestion. Revert check: with the create branch disabled the test reports the move-to-next-create suggestion.", "insight": "A suggestion derived from position (\"the next statement that can take it\") is only right when the position is the mistake. Ask what the comment was written FOR before telling the user where to put it.", "refs": ["mdl/executor/validate_detached_doc_comments_test.go"]} diff --git a/.claude/skills/mendix/check-syntax/SKILL.md b/.claude/skills/mendix/check-syntax/SKILL.md index 2668849e38..0a0d2a741b 100644 --- a/.claude/skills/mendix/check-syntax/SKILL.md +++ b/.claude/skills/mendix/check-syntax/SKILL.md @@ -191,9 +191,9 @@ keeps the header off a file it would make refuse (ako/mxcli#876). A `/** … */` doc comment documents the statement right after it, and only a `create` of something with documentation stores one. Above a `drop`, `grant`, -`revoke`, `set`, `alter` or `create module` it is ignored, so **MDL089** warns -and names the next statement that could have taken it. In drop-then-create, put -the comment between the `drop` and the `create` (ako/mxcli#877). +`revoke`, `set` or `alter` it is ignored, so **MDL089** warns and names the next +statement that could take it — in drop-then-create, put it between the two +(ako/mxcli#877). Above a `create` that stores none, make it a `--` comment. ### It resolves MEMBER names too, where it can establish the entity diff --git a/mdl/executor/validate_detached_doc_comments.go b/mdl/executor/validate_detached_doc_comments.go index 7b13b34c4b..94a0bc65d8 100644 --- a/mdl/executor/validate_detached_doc_comments.go +++ b/mdl/executor/validate_detached_doc_comments.go @@ -4,6 +4,7 @@ package executor import ( "fmt" + "strings" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/linter" @@ -20,7 +21,8 @@ const detachedDocCommentRule = "MDL089" // wrote `drop microflow if exists X;` between six flows' comments and their // creates, and only lint's QUAL002 noticed the missing documentation. The // warning names the next statement that would have stored it, which is almost -// always the one it was meant for. +// always the one it was meant for — except after a create, which is itself the +// element the comment describes (mendixlabs/mxcli#1510). // // A warning, not an error, under every language version: the statement does // what it says, and only the comment is lost. @@ -30,7 +32,16 @@ func ValidateDetachedDocComments(prog *ast.Program) []linter.Violation { msg := fmt.Sprintf("line %d: the doc comment before `%s` is lost: that statement cannot store "+ "documentation, so the comment is ignored", d.Line, d.Statement) suggestion := "Move the doc comment above the statement it documents, or make it a `--` comment." - if d.Next != "" { + switch { + case isCreateHead(d.Statement): + // The comment is ON the element it describes, which simply has no + // documentation MDL can write. The next statement documents a + // different element, so pointing there would move text written for + // this one onto that one (mendixlabs/mxcli#1510). + msg = fmt.Sprintf("line %d: the doc comment before `%s` is lost: MDL cannot store documentation "+ + "for that element, so the comment is ignored", d.Line, d.Statement) + suggestion = "Make it a `--` comment, which keeps the note without implying it is stored." + case d.Next != "": msg += fmt.Sprintf("; the next statement that can take it is `%s` (line %d)", d.Next, d.NextLine) suggestion = fmt.Sprintf("Move the doc comment directly above `%s`, after the `%s`.", d.Next, d.Statement) } @@ -43,3 +54,9 @@ func ValidateDetachedDocComments(prog *ast.Program) []linter.Violation { } return out } + +// isCreateHead reports whether a statement head, as statementHead records it, +// is a create — the element the doc comment above it describes. +func isCreateHead(head string) bool { + return strings.HasPrefix(strings.ToLower(head), "create ") +} diff --git a/mdl/executor/validate_detached_doc_comments_test.go b/mdl/executor/validate_detached_doc_comments_test.go index c643d69295..949384ff28 100644 --- a/mdl/executor/validate_detached_doc_comments_test.go +++ b/mdl/executor/validate_detached_doc_comments_test.go @@ -43,3 +43,38 @@ func TestValidateProgram_DetachedDocComment(t *testing.T) { t.Errorf("a comment on its create was reported: %v", got) } } + +// mendixlabs/mxcli#1510: a doc comment above a create that stores none was +// told to move "directly above" the next create — documenting a different +// element with text written for this one. A drop or grant stands between a +// comment and its create; a create IS the element the comment describes, so +// the next statement is not where it belongs. Control: a drop still gets the +// move suggestion. +func TestValidateProgram_DetachedDocCommentOnCreate_Issue1510(t *testing.T) { + const src = "/** Sales staff. */\ncreate module role Sales.User;\n" + + "/** An order. */\ncreate persistent entity Sales.Order (Name: String(20));\n" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var got []string + for _, v := range ValidateProgram(prog, "") { + if v.RuleID == detachedDocCommentRule { + got = append(got, v.Message+" | "+v.Suggestion) + } + } + if len(got) != 1 { + t.Fatalf("got %d MDL089 warnings, want 1: %v", len(got), got) + } + for _, bad := range []string{"Move the doc comment directly above", "next statement that can take it", + "create persistent entity Sales.Order"} { + if strings.Contains(got[0], bad) { + t.Errorf("the warning sends the comment to a different element (%q): %s", bad, got[0]) + } + } + for _, want := range []string{"create module role Sales.User", "`--` comment"} { + if !strings.Contains(got[0], want) { + t.Errorf("want %q in %q", want, got[0]) + } + } +}