Skip to content

fix: parse negative annotation coordinates#494

Merged
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/parse-negative-position-annotations
May 2, 2026
Merged

fix: parse negative annotation coordinates#494
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/parse-negative-position-annotations

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented May 2, 2026

Closes #493.

Summary

  • preserves signed integer values in annotation parameters such as @position(-150, -210)
  • keeps the existing literal-only fallback for non-signed numeric annotation parameters
  • adds a synthetic microflow visitor regression test for negative coordinates

Validation

  • go test ./mdl/visitor -run 'NegativeCoordinates|PositionAnnotation' -v
  • make build
  • make lint-go
  • make test

Symptom: `@position(-150, -210)` parsed without errors, but the stored coordinates became `(0, 0)`. Re-executing described microflows then moved activities whose canvas coordinates were negative.

Root cause: annotation parameters allow expressions, so a leading minus is parsed as a unary expression rather than a `NUMBER_LITERAL`. The annotation integer helper only read direct numeric literals and silently fell back to zero for unary-negative values.

Fix: parse the raw annotation parameter text as an integer before falling back to the literal-only path, preserving both positive and negative integer coordinates without evaluating arbitrary expressions.

Tests: added a synthetic microflow visitor test for negative `@position` coordinates; `make build`; `make lint-go`; `make test`.
@ako
Copy link
Copy Markdown
Collaborator

ako commented May 2, 2026

Review: fix: parse negative annotation coordinates

Verdict: Approve

What the PR does

Fixes issue #493: @position(-150, -210) annotations were silently returning 0 for both coordinates. The existing parseAnnotationParamInt only followed the Literal → NUMBER_LITERAL path, which doesn't match negative numbers. A value like -150 is parsed by the grammar through the expression rule (unary minus + integer), so annValue.Literal() returns nil and the function fell through to return 0.

The fix calls strconv.Atoi(strings.TrimSpace(valueCtx.GetText())) first — ANTLR's GetText() concatenates all child token texts, producing "-150" for the expression case — then keeps the existing literal fallback path for any value Atoi can't handle.

No blockers

Minor observation

The TrimSpace call handles leading/trailing whitespace on the raw text, but not intra-token whitespace like - 150 (space between minus and digits). That would cause Atoi to fail and fall through to the literal path, returning 0. In practice annotation params written with - 150 are unlikely, and the expression grammar rule lexes -150 without intermediate whitespace. Worth a brief comment if this code is touched again, but not a blocker.

Positive notes

  • Minimal, focused fix — 3 lines of production code, no grammar change required
  • New test directly reproduces the bug and asserts exact coordinate values
  • The early-return structure is correct: positive integers now also hit the Atoi fast path (same result as the literal path), which is harmless
  • Clean PR, no bundled unrelated changes

@ako ako merged commit daea97d into mendixlabs:main May 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Negative @position coordinates are parsed as zero

3 participants