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

Add shortcuts for at, index #978

Merged
merged 7 commits into from
Dec 6, 2020
Merged

Conversation

sapizhak
Copy link
Contributor

@sapizhak sapizhak commented Dec 5, 2020

@julien-truffaut
Copy link
Member

Thanks @sapizhak Could you also add a test to check at works with tuples? My guess it that we need to make I contravariant (i.e. def at[-I, A1])

@sapizhak
Copy link
Contributor Author

sapizhak commented Dec 6, 2020

Great idea, I have added some tests to cover all the At and Index summoned by at and index functions (probably every instance is called implicitly now in tests, so if a new instance is added, it would be fine to be called in tests too).

As for contravariant position for I.. We cannot set variance annotations for functions in their type parameters, because variance is a quality of classes, whereas functions declared as def only implement FunctionN trait after desugaring.

Apart from that, Function1 ... FunctionN type parameters are already variance annotated: their arguments are contravariant and result type is invariant. An implicit argument scenario would extend a trait where arguments (including an implicit one) are contravariant as well:

trait ImplicitFunction2[-A, -AA, +B] {
    def apply(a: A)(implicit i: AA): B
  }

So an I should already be contravariant! :))

@julien-truffaut
Copy link
Member

julien-truffaut commented Dec 6, 2020

As for contravariant position for I.. We cannot set variance annotations for functions in their type parameters, because variance is a quality of classes, whereas functions declared as def only implement FunctionN trait after desugaring.

You are completely right, good point.

Great idea, I have added some tests to cover all the At and Index summoned by at and index functions (probably every instance is called implicitly now in tests, so if a new instance is added, it would be fine to be called in tests too).

Cheers, that's a lot of work but at least we are covered :)
Maybe it will be enough to test all the instances for one optic, e.g. Lens but we can always refactor later on.

@julien-truffaut julien-truffaut merged commit 63e08d9 into optics-dev:master Dec 6, 2020
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 this pull request may close these issues.

Add shortcut for index
2 participants