Skip to content

Compiler@master sometimes reports module name instead of source file as error location #5635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Minnozz opened this issue Sep 5, 2022 · 11 comments · Fixed by #5662
Closed
Milestone

Comments

@Minnozz
Copy link
Contributor

Minnozz commented Sep 5, 2022

Context: trying out JSX v4

rescript: [59/64] src/SpotDetails-Stallingsnet.cmj
FAILED: src/SpotDetails-Stallingsnet.cmj

  We've found a bug for you!
  SpotDetails

  This has type:
    props<option<Stallingsnet.Prelude.occupationDetailsSnapshot>> => React.element
  Somewhere wanted:
    React.component<
  ({..
    "maybeSnapshot": option<Stallingsnet.Prelude.occupationDetailsSnapshot>,
  } as 'a),
>
      (defined as 'a => Jsx.element)
  
  The incompatible parts:
    props<option<Stallingsnet.Prelude.occupationDetailsSnapshot>> vs 'a

rescript: [60/64] src/Statistics-Stallingsnet.cmj
FAILED: cannot make progress due to previous errors.

The SpotDetails below We've found a bug for you! should be /absolute/path/to/Source.res:line:char-char instead of a module name.

The VSCode extension tries to open a nonexistent directory when you click the Problem:
Screenshot from 2022-09-05 22-00-20
Screenshot from 2022-09-05 22-02-39

This does not happen with all errors; some report a correct location.

@mununki
Copy link
Member

mununki commented Sep 5, 2022

Can you share the code for me to reproduce?

@Minnozz
Copy link
Contributor Author

Minnozz commented Sep 6, 2022

Here:

module Foo = {
  let compare = (nextProps, currentProps) => nextProps["foo"] == currentProps["foo"]
  let memo = React.memoCustomCompareProps(_, compare)

  @react.component
  let make = memo((~foo: string) => React.string(foo))
}
$ rescript build -w
>>>> Start compiling 
rescript: [5/6] src/MissingErrorLocation.cmj
FAILED: src/MissingErrorLocation.cmj

  We've found a bug for you!
  MissingErrorLocation

  This has type: props<string> => React.element
  Somewhere wanted:
    React.component<({.."foo": 'b} as 'a)> (defined as 'a => React.element)
  
  The incompatible parts:
    props<string> vs 'a


@cristianoc
Copy link
Collaborator

@mattdamon108 looks like a source location issue (e.g. loc_none is used)

@cristianoc
Copy link
Collaborator

Source:

module React = {
  type element
  external string: string => element = "%identity"
}

module Foo = {
  @react.component
  let make = (~s) => React.string(s)
}

Debug command:

% ~/GitHub/rescript-compiler/bsc -dtypedtree -bs-loc -bs-jsx 4  src/Cmp.res
[
  structure_item (src/Cmp.res[1,0+0]..src/Cmp.res[4,83+1])
    Tstr_module
    React/1002
      module_expr (src/Cmp.res[1,0+15]..src/Cmp.res[4,83+1])
        Tmod_structure
        [
          structure_item (src/Cmp.res[2,17+2]..src/Cmp.res[2,17+14])
            Tstr_type Nonrec
            [
              type_declaration element/1003 (src/Cmp.res[2,17+2]..src/Cmp.res[2,17+14])
                ptype_params =
                  []
                ptype_cstrs =
                  []
                ptype_kind =
                  Ttype_abstract
                ptype_private = Public
                ptype_manifest =
                  None
            ]
          structure_item (src/Cmp.res[3,32+2]..src/Cmp.res[3,32+50])
            Tstr_primitive
            value_description string/1004 (src/Cmp.res[3,32+2]..src/Cmp.res[3,32+50])
              core_type (src/Cmp.res[3,32+19]..src/Cmp.res[3,32+36])
                Ttyp_arrow
                Nolabel
                core_type (src/Cmp.res[3,32+19]..src/Cmp.res[3,32+25])
                  Ttyp_constr "string/13"
                  []
                core_type (src/Cmp.res[3,32+29]..src/Cmp.res[3,32+36])
                  Ttyp_constr "element/1003"
                  []
              [
                "%identity"
              ]
        ]
  structure_item (src/Cmp.res[6,86+0]..src/Cmp.res[9,157+1])
    Tstr_module
    Foo/1005
      module_expr (src/Cmp.res[6,86+13]..src/Cmp.res[9,157+1])
        module_expr (src/Cmp.res[6,86+13]..src/Cmp.res[9,157+1])
          Tmod_structure
          [
            structure_item (_none_[1,0+-1].._none_[1,0+-1]) ghost
              Tstr_type Nonrec
              [
                type_declaration props/1006 (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                  ptype_params =
                    [
                      core_type (_none_[1,0+-1].._none_[1,0+-1]) ghost
                        Ttyp_var s
                    ]
                  ptype_cstrs =
                    []
                  ptype_kind =
                    Ttype_record
                      [
                        (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                          attribute "ns.optional"
                            []
                          Immutable
                          key/1007                          core_type (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                            Ttyp_poly
                            core_type (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                              Ttyp_constr "option/10"
                              [
                                core_type (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                                  Ttyp_constr "string/13"
                                  []
                              ]
                        (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                          Immutable
                          s/1008                          core_type (_none_[1,0+-1].._none_[1,0+-1]) ghost
                            Ttyp_poly
                            core_type (_none_[1,0+-1].._none_[1,0+-1]) ghost
                              Ttyp_var s
                      ]
                  ptype_private = Public
                  ptype_manifest =
                    None
              ]
            structure_item (src/Cmp.res[7,101+2]..src/Cmp.res[8,120+36])
              Tstr_value Nonrec
              [
                <def>
                    attribute "react.component"
                      []
                  pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                    Tpat_var "make/1009"
                  expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                    Texp_function
                    param/1011                    Nolabel
                    [
                      <case>
                        pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                          Tpat_constraint
                          core_type (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                            Ttyp_constr "props/1006"
                            [
                              core_type (src/Cmp.res[8,120+14]..src/Cmp.res[8,120+16])
                                Ttyp_var s
                            ]
                          pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                            Tpat_record
                            [
                              "s"
                                pattern (src/Cmp.res[8,120+14]..src/Cmp.res[8,120+16])
                                  attribute "ns.namedArgLoc"
                                    []
                                  Tpat_var "s/1010"
                            ]
                        expression (src/Cmp.res[8,120+21]..src/Cmp.res[8,120+36])
                          Texp_apply
                          expression (src/Cmp.res[8,120+21]..src/Cmp.res[8,120+33])
                            Texp_ident "React/1002.string"
                          [
                            <arg>
                              Nolabel
                              expression (src/Cmp.res[8,120+34]..src/Cmp.res[8,120+35])
                                Texp_ident "s/1010"
                          ]
                    ]
              ]
            structure_item (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
              Tstr_value Nonrec
              [
                <def>
                  pattern (src/Cmp.res[8,120+6]..src/Cmp.res[8,120+10])
                    Tpat_var "make/1012"
                  expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                    Texp_let Nonrec
                    [
                      <def>
                        pattern (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                          Tpat_var "Cmp$Foo/1013"
                        expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                          Texp_function
                          props/1014                          Nolabel
                          [
                            <case>
                              pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                Tpat_constraint
                                core_type (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                  Ttyp_constr "props/1006"
                                  [
                                    core_type (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                      Ttyp_any
                                  ]
                                pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                  Tpat_alias "props/1014"
                                  pattern (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                    Tpat_any
                              expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                Texp_apply
                                expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                  Texp_ident "make/1009"
                                [
                                  <arg>
                                    Nolabel
                                    expression (_none_[1,0+-1].._none_[1,0+-1]) ghost
                                      Texp_ident "props/1014"
                                ]
                          ]
                    ]
                    expression (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                      Texp_ident "Cmp$Foo/1013"
              ]
          ]
]

// Generated by ReScript, PLEASE EDIT WITH CARE
'use strict';


var React = {};

function Cmp$Foo(props) {
  return props.s;
}

var Foo = {
  make: Cmp$Foo
};

exports.React = React;
exports.Foo = Foo;
/* No side effect */

@cristianoc
Copy link
Collaborator

So the entire definition of type props uses ghost locations.
@mattdamon108 how about try to use the same location as the attribute @react.component as it's most definitely unused and that's kind of where the type is coming from?

@mununki
Copy link
Member

mununki commented Sep 8, 2022

I didn't grab the time to look into it yet.
Yes, that would be a solution. How about using this loc for type props? It seems not good.

                          core_type (Cmp[1,0+-1]..Cmp[1,0+-1]) ghost
                            Ttyp_constr "props/1006"
                            [
                              core_type (src/Cmp.res[8,120+14]..src/Cmp.res[8,120+16])
                                Ttyp_var s
                            ]

@mununki
Copy link
Member

mununki commented Sep 8, 2022

Yeap, I'll fix it to use the same location of @react.component

structure_item (src/Cmp.res[7,101+2]..src/Cmp.res[8,120+36])

@mununki
Copy link
Member

mununki commented Sep 9, 2022

I did quick fix yesterday to add the same location of @react.component to the type declaration of props, it didn’t give me an error location for the above error. I’ll keep exploring to find where is the actual error point and wire the loc to it.

@cristianoc
Copy link
Collaborator

I did quick fix yesterday to add the same location of @react.component to the type declaration of props, it didn’t give me an error location for the above error. I’ll keep exploring to find where is the actual error point and wire the loc to it.

Perhaps the body of the function?
Worth trying more than one thing at once, in case multiple places need to have better locations.

@mununki
Copy link
Member

mununki commented Sep 9, 2022

Perhaps the body of the function?
Worth trying more than one thing at once, in case multiple places need to have better locations.

Sure thing. I’ll put dummy locs to possible points to explore the better location.

@mununki
Copy link
Member

mununki commented Sep 14, 2022

Fixed rescript-lang/syntax#633

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 a pull request may close this issue.

3 participants