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

List.member on Dict.keys/Set.toList -> Dict/Set.member #290

Open
morteako opened this issue Feb 24, 2024 · 4 comments
Open

List.member on Dict.keys/Set.toList -> Dict/Set.member #290

morteako opened this issue Feb 24, 2024 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@morteako
Copy link
Contributor

morteako commented Feb 24, 2024

What the rule should do:
Replace List.member on Set.toList/Dict.keys with Set/Dict.member

What problems does it solve:
Simplifying and optimizes unnecessary convertions

Example of things the rule would report:

List.member c (Dict.keys d)
--> Dict.member c d
List.member c (Set.toList d)
--> Set.member c d

Example of things the rule would not report:
Since Set/Dict.member requires comparable, but List.member does not,
applying this replacement to Set/Dict.empty, could lead to a type error.
So therefore we should/could not replace it in that case. (But it should be caught by some other simplification rule)

List.member c (Dict.keys Dict.empty)

The above should be simplified to
False anyways, by the callOnEmptyReturnsCheck

Should this be part of the Simplify rule or should it be a new rule?

Part of Simplify

I am looking for:

  • Feedback
@morteako morteako changed the title List.member on Dict/Set.toList -> Dict/Set.member List.member on Dict.keys/Set.toList -> Dict/Set.member Feb 24, 2024
@lue-bird
Copy link
Collaborator

Nice. So basically we can only apply these if the set/dict is constructed right there (fromList, insert, singleton) and is non-empty.

(And I think you mean List.member c (Dict.toList Dict.empty) instead of List.member c (Dict.member Dict.empty) in the "not reported" example)

@morteako
Copy link
Contributor Author

Ah, yes. That seems (unfortunately) correct.

Those restrictions seems to make this rule pretty niche, so I don't think it is worth to implement it probably.
I think it would be a good rule in the general case, but having the possibility to create type errors is of course really bad.

@jfmengels jfmengels added enhancement New feature or request help wanted Extra attention is needed labels Mar 2, 2024
@jfmengels
Copy link
Owner

jfmengels commented Mar 2, 2024

Sounds like a good idea 👍
Thank you both for your input ☺️

I wonder if there are other conversations (for instance List.isEmpty (Dict.toList dict)) that could be done with the base type's functions, and that we don't already support.

@morteako
Copy link
Contributor Author

morteako commented Mar 3, 2024

@jfmengels

I think these are not supported.

.singleton

Dict.fromList (List.singleton (k,v))
---> Dict.singleton k v

List.length ✅ #293

List.length (Set.toList set)
---> Set.size set

-- also for Dict.keys and Dict.values
List.length (Dict.toList dict)
---> Dict.size dict

List.isEmpty ✅ #293

-- also for Dict.keys and Dict.values
List.isEmpty (Dict.toList dict)
---> Dict.isEmpty dict

List.isEmpty (Set.toList set)
---> Set.isEmpty set

List.member (both of these have the same problems as the one above :()

List.member k (Dict.keys dict)
---> Dict.member k dict

List.member k (Set.toList set)
---> Set.member k set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants