Skip to content
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

Reference counting for union types not working #18

Open
edkelly303 opened this issue Jan 4, 2018 · 2 comments
Open

Reference counting for union types not working #18

edkelly303 opened this issue Jan 4, 2018 · 2 comments
Assignees
Milestone

Comments

@edkelly303
Copy link

Hi Matt,

I love this plugin even more than elm-test-runner. I'm running version 0.2.0, and I've just noticed one issue with counting external references for union types whose constructors are exposed.

I have a module called Types.elm, declared as follows:
module Types exposing (Model, Entity, EntityClass(..), Sprite, View(..))

And I import these types into my Main.elm as follows:
import Types exposing (Model, Entity, EntityClass(..), Sprite, View(..))

When I look in the Types.elm file, the reference counts for Model, Entity and Sprite are fine - but for the union types EntityClass and View, it says they have 0 external references.

image

However, they definitely do have external references: here is an example from Main.elm:

image

@mbuscemi
Copy link
Owner

mbuscemi commented Jan 5, 2018

It is important to keep in mind the difference between function references, type references, and value constructor references (these are a kind of function, but please read on to see why I'm separating them out).

A function reference, in your example, would be renderBackground or renderLives. Some functions from Elm core include toString and div, for example. When these symbols are used inside function body definitions they count as references of those functions.

A function like renderLives will probably have a type signature in the module where it is declared. It might look something like this: renderLives : Model -> Html message. This signature will count for two type references: Model and Html.

In your example, in order to qualify as an external reference of View, a type signature would have to include View as one of its parts.

Title, LoseLife, and GameOver are type constructors. These are special functions that produce a View type of value. Invoking these functions does not constitute a type reference for View, but rather a function reference for Title, LoseLife, or GameOver respectively.

Elm Lens currently does not support reference counting for type constructors, only the types. This is not due to any particular technical difficulty, but rather a UI/UX problem. One of my simplest implementations of this (which never made it into any commits) consisted of a line of markup above every single type constructor. This was ugly and made the code harder to read. Definitely the opposite of my goal with Elm Lens. Additionally, there is the problem of multiple type constructors on the same line, which, while not possible with elm-format, is still valid Elm code.

So, it is up to me to come up with a good UI solution for this problem. The existing situation is at least as unacceptable: types that are legitimately exposed in order to expose their referenced constructors are incorrectly labeled with a warning.

I will think on potential UI solutions and follow up here.

@edkelly303
Copy link
Author

edkelly303 commented Jan 5, 2018

That makes perfect sense - thank you for the explanation.

Now that you have added the Reference Panel, perhaps you could make the reference count in the contextual markup just count up all references (regardless of whether they are references to types or to functions/type constructors within the type). Then in the Reference Panel you could go into more detail to distinguish what kind of references they are?

It seems to me that this distinction is potentially useful in the Reference Panel, because when you're looking through your code, you might want to know specifically how many references there are to View, versus how many for each of Title, LoseLife or GameOver, and where those references are in your codebase.

But in the main window, the Elm Lens contextual markup is mainly useful to answer the simpler question of "do I really need to expose this type/function or not" (or at least, that's what I've been using it for...) So the more detailed information about how many times each of the type constructors is referenced individually is perhaps not so important here?

@mbuscemi mbuscemi added this to the 0.5 milestone Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants