ModuleNameLookupTable.moduleNameAt for List type returns [ "List" ]

[lue-bird]

Describe the bug Review.ModuleNameLookupTable.moduleNameAt returns [ "List" ] for a List type, even though the List module does not expose that type.

SSCCE (Short Self-Contained Correct Example) https://github.com/lue-bird/elm-review-sscce-modulenamelookuptable-at-list-type

Expected behavior I expected this to return either [] or [ "" ].

Additional context I encountered this with https://dark.elm.dmy.fr/packages/lue-bird/elm-review-import-simple/latest/.

If you consider fixing this, I strongly suggest holding off until 3.0.0 since this could break existing rules.

[lydell]

🤯 TIL. The Default Imports listed at https://elm.dmy.fr/packages/elm/core/latest/ have this error in them. I thought it would be valid to copy-paste those to any file (they would just be redundant), but this line causes an error:

import List exposing (List, (::))

The `List` module does not expose `List`:

26| import List exposing ((::), List)
                                ^^^^
I cannot find any super similar exposed names. Maybe it is private?

Ugh, why does the List type has to be such a special case? 🫠

[jfmengels]

Yeah, List not being exposed is such a weird edge-case. I should make a PR to elm/core to just add the definition of List there, although I don't know whether that would break somewhere else. An isue should at least be opened there if it isn't already. (EDIT: there's one open here https://github.com/elm/core/issues/1037 and it would require a new compiler version)

I think the current result is the one that makes most sense.

Returning [] would mean it's a local type, which is confusing. Especially so if the user re-defined a type List = ... which does happen from time to time (I sometimes see it in the #beginners channel on Slack).

Returning [ "" ] would be even weirder. It doesn't necessarily mean anything in particular as far as I can tell.

From what I can see from Review.ImportSimple, the issue (and why you suggest those alternatives) is because you're trying to qualify the references, but in other situations those wouldn't make much sense either.

I think the current solution is "okay". Fixing the issue in elm/core is undoubtedly the best solution. In the meantime, i think this solution is one that makes quite a bit of sense, but does require some edge-case handling depending on the use-case.

[lue-bird]

I agree that this is a problem with elm/core rather than elm-review. Compiler errors seem to imply this origin as well: https://ellie-app.com/qXJC8NtdHb3a1

This call produces:

    List.List a

But the type annotation says it should be:

    Main.List a