search

elm / compiler

Compiler for Elm, a functional language for reliable webapps.
https://elm-lang.org/
BSD 3-Clause "New" or "Revised" License
7.48k stars 659 forks source link

Error trying to import module not in the exposed-modules list of a package could be improved. #2253

Closed houseofmercy-github closed 2 years ago

houseofmercy-github commented 2 years ago

Quick Summary:

For example https://ellie-app.com/hcbgFYzMTQYa1 reports the error

You are trying to import a `Markdown.HtmlRenderer` module:

11| import Markdown.HtmlRenderer
           ^^^^^^^^^^^^^^^^^^^^^
I checked the "dependencies" and "source-directories" listed in your elm.json,
but I cannot find it!

The real problem seems to be that dillonkearns/elm-markdown does not include Markdown.HtmlRenderer in its exposed-modules so other modules may not import it.

Suggestion

It would be more informative if the error were to explicitly mention this. E.g.

You are trying to import a `Markdown.HtmlRenderer` module:

11| import Markdown.HtmlRenderer
           ^^^^^^^^^^^^^^^^^^^^^

This module is not in the "exposed-modules" section of that package's elm.json file.

The Modules section of the elm guide could also mention the https://github.com/elm/compiler/blob/master/docs/elm.json/package.md#exposed-modules setting explicitly.

SSCCE

https://ellie-app.com/hcbgFYzMTQYa1

which simply adds 

import Markdown.HtmlRenderer

to https://ellie-app.com/d7R3b9FsHfCa1

Additional Details

What I was trying to do at the time:
https://old.reddit.com/r/elm/comments/tw36nv/easy_questions_beginners_thread_week_of_20220404/i3x2lrj

What I discovered a few days later
https://old.reddit.com/r/elm/comments/tw36nv/easy_questions_beginners_thread_week_of_20220404/i4g5msp

github-actions[bot] commented 2 years ago

Thanks for reporting this! To set expectations:

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

jfmengels commented 2 years ago

@houseofmercy-github A problem here is that there is no relation between a module's name and the package it comes from. While it is somewhat obvious (to a human) that Markdown.HtmlRenderer could come from dillonkearns/elm-markdown, it isn't for the compiler.

And it might not actually be obvious. Maybe you also depend on elm-explorations/markdown in which case it's a lot less obvious. Maybe the package jfmengels/something contains a (hidden) Markdown.HtmlRenderer module. Or maybe it's part of your application, but the file was recently removed, or you forgot to run your code generation.

The only thing that connects the two is the package's source-directories field. So if a module is not listed in any of the packages' source directories, then the compiler has to assume it is in your application.

Technically, it could find that information from looking at the different files in the dillonkearns/elm-markdown package's source code, but that would probably incur a performance cost. I also don't know if it's possible for multiple packages to have non-exposed modules named the same, in which case there needs to be a different error message in that case as well.

houseofmercy-github commented 2 years ago

@jfmengels Thank you for the explaination. If there is no easy way for the compiler to look for non-exposed modules of an imported package then a precise suggestion might not be worth the performance cost.

Perhaps the error message in this case could simply include a generic hint that importing a module not listed in the package's exposed-modules isn't allowed and refer the user to your explaination.

If the operations involved are more expensive then perhaps a separate "lint" utility that could make more assumptions about the user's code would be a better place for it. Are you aware of anything like that? I'm new to Elm but would be happy to try and make a PR that could do that.

jfmengels commented 2 years ago

If you look for a linter, you should probably check out elm-review. But it assumes that the code has already been validated by the compiler, so it's probably not the best fit.

I also think that this is the kind of issue that once you've encountered once, you'll know about it. Beginners won't think about using a separate tool (they probably won't even discover it), so I think that this should either be addressed by the compiler or not addressed at all

Perhaps the error message in this case could simply include a generic hint that importing a module not listed in the package's exposed-modules isn't allowed and refer the user to your explaination.

I agree that the compiler could give a slightly better answer, though I'm at a loss as to what details to add because there needs to be a trade-off between a short helpful explanation and a long detailed one. I don't think we want to confuse beginners with all the details I mentioned, especially since the most likely case for this error to happen is that the user forgot to create a file or renamed a file without changing the import (which the current error doesn't address super well actually).

In your particular use-case, I think you got confused because you looked at the source code, whereas you should have ideally looked at the package's documentation (I mean in an ideal world, not saying you did anything bad at all, on the contrary). If that didn't give you the information you wanted, then I'd report that there and maybe open a PR to improve the documentation there. That sounds like an easier way to start contributing :) (and I know that Dillon welcomes the help ;) )

houseofmercy-github commented 2 years ago

Thank you. I will take a look at elm-review. This is definitely one of those things I'll know to watch out for in the future. I probably should have started with something less ambitious then helping custom html handlers in elm-markdown report better errors but I was so impressed by it otherwise that I felt I had to give it try.

I think some form of your description would make sense in the elm guide since it talks quite a bit about modules but doesn't mention the exposed-modules section of elm.json or how that relates to a package's supported interface. An example there explaining the mistake I made would have saved me some time. I will see if I can make a review to add some text to the guide and maybe a link to elm.json/package.md#exposed-modules page.