Skip to content

Fix off-by-one in source regions emitted to JSON and Debug.todo#2359

Merged
evancz merged 1 commit into
elm:mainfrom
anvmn:fix-0-based-source-regions
Jul 13, 2026
Merged

Fix off-by-one in source regions emitted to JSON and Debug.todo#2359
evancz merged 1 commit into
elm:mainfrom
anvmn:fix-0-based-source-regions

Conversation

@anvmn

@anvmn anvmn commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #2358.

In 0.19.2 Reporting.Annotation.Position became a packed Word64#, and its own comment states the new contract:

-- The initial value is (0,0) so conversion is needed to get
-- "editor coordinates" which start at (1,1)

Reporting.Render.Code performs that conversion (show (row + 1)), which is why the terminal error report is still correct. The two places that emit machine-readable coordinates call A.toRowCol raw and never convert:

  • Reporting.Error.encodeRegion — used by elm make --report=json, which editors and elm-language-server consume
  • Generate.JavaScript.Expression.regionToJsExpr — used for Debug.todo / Debug.todoCase

So every JSON diagnostic is one line up and one column left, and a Debug.todo on line 17 crashes with TODO in module `Main` on line 16.

The change

Add a toEditorRowCol helper next to toRowCol and use it at the two emit sites. Reporting.Render.Code uses toRow / toCol directly and already does its own conversion, so it is untouched.

Note: after this change toRowCol has no remaining callers — both of its consumers wanted editor coordinates. Making toRowCol itself 1-based would be a smaller diff; I kept a separately named function so the (0,0)-based accessor stays available and the conversion is explicit at the point of use. Happy to switch to whichever you prefer.

Verification

Built with GHC 9.8.4 (cabal build exe:elm, so under the package's -Wall -Werror): 137/137 modules, links cleanly.

SSCCE from #2358Debug.todo on line 17, column 5; type error on line 6, columns 5–17:

compiler Debug.todo region runtime message --report=json region
0.19.1 (17,5)–(17,15) on line 17 (6,5)–(6,17)
0.19.2 (16,4)–(16,14) on line 16 (5,4)–(5,16)
0.19.2 + this patch (17,5)–(17,15) on line 17 (6,5)–(6,17)

The patched compiler matches 0.19.1 exactly.

Also checked:

  • The human-readable report is unchanged — still 6| "not an int" with the caret correctly aligned, i.e. not double-incremented.
  • On a real 548-module application, all 12 Debug.todo regions emitted into the bundle become identical to the 0.19.1 output (Δline = Δcolumn = 0 for every one), where before the patch every one was off by exactly (-1, -1).

In 0.19.2 `Position` became a packed `Word64#`, and its own comment notes
that "The initial value is (0,0) so conversion is needed to get 'editor
coordinates' which start at (1,1)".

`Reporting.Render.Code` does that conversion, which is why the terminal
error report is still correct. The two places that emit machine-readable
coordinates call `toRowCol` raw and do not convert:

  * `Reporting.Error.encodeRegion`                    -- elm make --report=json
  * `Generate.JavaScript.Expression.regionToJsExpr`   -- Debug.todo / todoCase

So `--report=json`, which editors and elm-language-server consume, and the
runtime `TODO in module `M` on line N` message are both one line and one
column too low. A `Debug.todo` on line 17, column 5 emits
`{start: {line: 16, column: 4}, ...}` and crashes with "on line 16".

Add a `toEditorRowCol` helper and use it at both emit sites. A uniform +1
on row and column reproduces the 0.19.1 output byte for byte.

Fixes elm#2358

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Thanks for suggesting these code changes. To set expectations:

  • Pull requests are reviewed in batches, so it can take some time to get a response.
  • Smaller pull requests are easier to review. To fix nine typos, nine specific issues will always go faster than one big one. Learn why here.
  • Reviewers may not know as much as you about certain situations, so add links to supporting evidence for important claims, especially regarding standards for CSS, HTTP, URI, etc.

Finally, please be patient with the core team. They are trying their best with limited resources.

@evancz
evancz merged commit 1bd5b36 into elm:main Jul 13, 2026
1 check passed
@evancz

evancz commented Jul 13, 2026

Copy link
Copy Markdown
Member

Thank you! I like the approach of having a separate naming convention for "editor rows" and "editor columns".

Sad I did not catch this in the pre-release, but there should be a 0.19.3 relatively soon.

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.

0.19.2: source regions in --report=json and Debug.todo are 0-based (off by one)

2 participants