gren-lang / gren-lang/compiler-common

The wrong row number is assigned to "import", "type", "type alias", and "port" in the AST

Open
#25 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
No language data
Stars
1
Forks
1
PR merge metrics
No merged PRs in 30d

Description

These keywords are followed by a name, like:

import Array
type Bar
type alias Foo
port baz

The name's row is assigned to the AST node instead of the keyword's row. Usually these are on the same row, so we haen't noticed, but it is legal to put a newline between the keyword and the name.
Example:

module PosBug exposing (Foo)


import
    Array


type
    alias Foo =
    { x : Int }

Line numbers: import is on line 4, Array on line 5; type is on
line 8, alias Foo on line 9.

Parse it and inspect the start of each declaration (here via gren format --pre-ast, but it is the raw parser output):

Declaration reported Located.start leading keyword is actually on name start
import Array { row: 5, col: 1 } row 4 (import) { row: 5, col: 5 }
type alias Foo { row: 9, col: 1 } row 8 (type) { row: 9, col: 11 }

For example, the import node:

    "imports": [
      {
        "start": {
          "row": 5,
          "col": 1
        },
        "end": {
          "row": 5,
          "col": 10
        },
        "value": {
          "module_": {
            "start": {
              "row": 5,
              "col": 5
            },
            "end": {
              "row": 5,
              "col": 10
            },
            "value": "Array"
          },
          "alias": null,
          "expose": []
        }
      }
    ],

This causes a problem in "gren format" if the user does put that legal newline between the keyword and the name. We cannot produce canonical output.

Root cause

The four conversion sites derive start from the name, not from the keyword
that was already consumed:

  • src/Compiler/Parse/Module.gren:366importLoopParser
    SourcePosition.at { moduleName.start | col = 1 } … (the import token is
    skipped just below, its position never captured).
  • src/Compiler/Parse/Module.gren:497declarationToModuleAlias
    { row = v.name.start.row, col = 1 }
  • src/Compiler/Parse/Module.gren:519declarationToModuleUnion
    { row = v.name.start.row, col = 1 }
  • src/Compiler/Parse/Module.gren:535declarationToModulePort
    { row = v.name.start.row, col = 1 }

For aliases/unions/ports the keyword's position isn't available at the
conversion site at all: Compiler.Parse.Declaration.parser consumes the
type/port keyword (typeParser/portParser) but never records where it
was, and Declaration carries only { docs, value }.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the newline case with gren format --pre-ast, then inspect the four conversion sites in src/Compiler/Parse/Module.gren and Compiler.Parse.Declaration.parser. Trace how keyword positions are consumed and represented in Declaration; done means import, type alias, type, and port nodes report the keyword's row and gren format produces canonical output for the example.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.