Skip to content

Fix batch of reported issues (#18, #19, #23, #25, #26, #27, #28)#35

Merged
ako merged 10 commits intomendixlabs:mainfrom
engalar:fix/github-issues-batch-2
Mar 26, 2026
Merged

Fix batch of reported issues (#18, #19, #23, #25, #26, #27, #28)#35
ako merged 10 commits intomendixlabs:mainfrom
engalar:fix/github-issues-batch-2

Conversation

@engalar
Copy link
Contributor

@engalar engalar commented Mar 26, 2026

Summary

Fixes for issues reported at engalar/mxcli:

Test plan

  • Regression tests added in bugfix_regression_test.go covering all fixed issues
  • Existing tests pass (go test ./...)
  • Manual validation with Mendix Studio Pro for Date type and XPath fixes

Note: The large diff in mdl/grammar/parser/mdl_parser.go (~13k lines) is auto-generated by ANTLR4 from the grammar change for inline if-then-else expressions.

🤖 Generated with Claude Code

engalar added 10 commits March 26, 2026 14:04
Bug #18: WHILE loop body activities were omitted because emitLoopBody
only looked for flows in the parent flowsByOrigin map. WHILE loops store
their body flows in loop.ObjectCollection.Flows, which was not being
consulted. Also changed END LOOP to END WHILE for WhileLoopCondition
loops.
Bug #25: Constants created with COMMENT 'text' were stored correctly
but DESCRIBE CONSTANT did not emit the COMMENT line. Also fixed the
CREATE CONSTANT handler to store stmt.Comment into the model's
Documentation field.
Bug #27: RETURN values containing qualified enum literals (e.g.,
Module.Enum.Value) were incorrectly prefixed with $, producing
$Module.Enum.Value which causes CE0109 on roundtrip. Enum literals
contain dots and should not be treated as variable names.
The writer was incorrectly mapping Long to DataTypes$IntegerType in BSON,
causing Long constants, microflow parameters, and REST parameters to be
read back as Integer. Fixed in writer_enumeration.go, writer_microflow.go,
writer_rest.go, and parser_rest.go.

Fixes #19
[%CurrentDateTime%], [%CurrentUser%] and other Mendix XPath tokens
are special placeholders, not string literals. Writing them as quoted
strings ('[%CurrentDateTime%]') caused CE0161 XPath parse errors in
Studio Pro.
setAssociationRef only set EntityRef but not AttributeRef, causing the
association path to be lost in the BSON output. Also fixed
extractCustomWidgetAttribute to look up specific property keys
(attributeAssociation, attributeEnumeration) instead of returning
the first AttributeRef found, which could match CaptionAttribute.
Column names were always generated as sequential identifiers (col1,
col2, ...) in DESCRIBE PAGE output. Now derives semantic names from
the column's bound attribute or caption text, falling back to col%d
only when neither is available.
Date attributes are stored as DomainModels$DateTimeAttributeType with
LocalizeDate=false in BSON. Added DateAttributeType (domain model) and
DateType (microflows) structs. Parser now distinguishes Date
(LocalizeDate=false) from DateTime (LocalizeDate=true). Updated all
type switches in executor, formatters, writers, and catalog.

Also fixed Long type mapping in cmd_dbconnection.go (DataTypes$LongType).

Fixes #26
26 unit tests covering:
- #18: loopEndKeyword (WHILE vs LOOP), WHILE loop header formatting
- #19: Long type mapping (convertASTToMicroflowDataType, GetTypeName)
- #25: DESCRIBE CONSTANT COMMENT output, escaping, absence
- #26: Date vs DateTime type mapping and constant formatting
- #27: isQualifiedEnumLiteral, enum RETURN without $ prefix
- #28: inline if-then-else parsing and expressionToString serialization
- #23: deriveColumnName from attribute, caption, fallback, special chars

Issue #20 (XPath tokens) already covered by bugfix_test.go.
The previous check matched any string containing a dot, which could
falsely match attribute paths like "Object.Attribute". Enum literals
always have the form "Module.Enum.Value" (at least 2 dots).
Copy link
Collaborator

@ako ako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

+9605 / -2783 across 35 files, 10 commits. Fixes a batch of reported issues. ~13k lines of the diff is auto-generated ANTLR parser.


Overlap with Main

Fix Status on Main Notes
#18 WHILE loop END WHILE keyword Partially on main CREATE/format already handle WHILE. DESCRIBE emits END LOOP; instead of END WHILE;. Body traversal already works via emitLoopBody. Only the keyword fix is needed.
#19 Long as DataTypes$LongType Already on main — duplicate Will likely cause merge conflicts
#23 DataGrid2 column names Not on main — needed
#25 DESCRIBE CONSTANT COMMENT Not on main — needed
#26 Date vs DateTime distinction Partially on main AST has TypeDate, but no microflows.DateType struct
#27 Enum literal $ prefix in RETURN Not on main — needed
#28 Inline if-then-else expressions Not on main — needed
XPath tokens unquoted Not on main — needed
ComboBox association AttributeRef Partially on main EntityRef set but not AttributeRef
isQualifiedEnumLiteral 2+ dots Not on main — needed

Fix #19 (LongType) is fully duplicate and should be dropped to avoid conflicts.


Bugs

1. HIGH: DateTime roundtrip regression — absent LocalizeDate defaults to Date
sdk/mpr/parser_domainmodel.go — The new parser logic:

localize, _ := raw["LocalizeDate"].(bool)
if localize { return DateTimeAttributeType }
return DateAttributeType  // Date

If LocalizeDate is absent from BSON (all MPR files written by older code, and possibly older Mendix versions), the type assertion yields false, and all existing DateTime attributes are misclassified as Date. Fix: default to DateTime when absent:

localize, ok := raw["LocalizeDate"].(bool)
if !ok || localize { return DateTimeAttributeType }
return DateAttributeType

2. MEDIUM: Same absent-LocalizeDate issue in cmd_diff_local.go
mdl/executor/cmd_diff_local.go — Both LocalizeDate checks default to "Date" when the field is absent (ok=false). Should default to "DateTime".

3. MEDIUM: DateType can never be produced by the microflow parser
sdk/mpr/parser_microflow.go — Both Date and DateTime map to DataTypes$DateTimeType in BSON. The parser always returns DateTimeType and has no way to distinguish. The new DateType struct is created but can never be produced by the parser, so DESCRIBE MICROFLOW will always show DateTime even for Date variables.

4. MEDIUM: Business events writer doesn't set LocalizeDate for Date
sdk/mpr/writer_businessevents.go — Maps both "DateTime" and "Date" to DomainModels$DateTimeAttributeType but creates a simple {$Type, $ID} object without setting LocalizeDate=false for Date. These will roundtrip as DateTime.

5. LOW: isQualifiedEnumLiteral is too broad
cmd_microflows_format_action.gostrings.Count(s, ".") >= 2 matches any string with 2+ dots (e.g., some.java.ClassName). Probably safe in the RETURN value context but could be tightened.


What looks good

  • END WHILE keyword fix (#18) is clean and targeted
  • DataGrid2 column name derivation (#23) is well-implemented with sensible fallback
  • DESCRIBE CONSTANT COMMENT (#25) is straightforward
  • Enum literal $ prefix fix (#27) addresses a real CE0109 roundtrip issue
  • Inline if-then-else (#28) grammar addition is well-tested
  • XPath token unquoting prevents real CE0161 errors
  • Good regression test coverage (26 tests in bugfix_regression_test.go)

Overall: The DateTime roundtrip regression (bug #1) is the main blocker — it would break every existing DateTime attribute when reading an MPR. Fix #19 is duplicate and should be dropped. The remaining fixes are solid and needed.

🤖 Generated with Claude Code

@ako ako merged commit bca4b48 into mendixlabs:main Mar 26, 2026
1 of 2 checks passed
ako added a commit that referenced this pull request Mar 26, 2026
…trip

Reverted:
- DataTypes$LongType → DataTypes$IntegerType (4 files): Mendix type
  cache doesn't have LongType; Long must serialize as IntegerType
- XPath token quoting restored: '[%Token%]' not [%Token%] (CE0161)
- ComboBox setAssociationRef AttributeRef removed: 2-part association
  path causes ArgumentNullException in Mendix AttributeRef.AttributeId

Fixed:
- DateTime roundtrip: absent LocalizeDate defaults to DateTime, not Date
- Business events writer sets LocalizeDate for Date/DateTime distinction
- cmd_diff_local.go: same absent-LocalizeDate fix in both code paths

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants