kenbot/goggles

Pleasant, yet principled Scala optics DSL

goggles's Issues

Support for successive or nested modifications

If we have multiple changes to make to a structure, how can we perform them all without it looking terrible?

val game1 = set"$game.currentLevel.player.health" := 100
val game2 = set"$game1.currentLevel.player.ammo" := 70
val game3 = set"$game2.currentLevel.player.status" = Status.Alive
...

or nested:

set"${set"${set"$game.currentLevel.player.status" = Status.Alive}.currentLevel.player.ammo" := 70}.currentLevel.player.health" := 100

Ew.

But how to do this? A 2d syntax?

val game1 = set"""$game.currentLevel.player.health
                 |                         .ammo
                 |                         .status""" = (100,70,Status.Alive)

That's really weird though. Maybe using the tuple syntax suggested in #10?

set"""$game.currentLevel.player.(health, ammo, status)" := (100, 70, Status.Alive)

This might not be powerful enough though. What do other Lens implementations do? Haskell? Is this needed at all?

Scala.js support

Should be straightforward - almost everything happens in the compiler, anyway.

Strip out Scala.js support

With apologies to @aoiroaoino for going to the effort of adding Scala.js support, I can't bear the extra SBT yak-shaving, and will remove the Scala.JS functionality, with all the extra folder structure & sbt build complexity.

This is one of the reasons I haven't successfully built & released the library in 3 years, and I want to focus on delivering features.

We can always add it again later.

add syntax for composePrism GenPrism

case class Foo(i: Int, bar: Bar)
sealed trait Bar
case class Bazz(b: Boolean) extends Bar
case class Buzz(s: String) extends Bar

val foo = Foo(2, Buzz("Hello"))

set"$foo.bar.as[Buzz].s" := "World"

Internal error: "get" fails on setters, "set" fails on getters

• The "get" DSL produces an internal failure if it finds a setter.
• The "set" DSL produces an unhelpful error message if it finds a getter or fold.

case class Potato(i: Int)
case class Banana(p: Potato)
case class Beetroot(b: Banana)

val getBanana = monocle.Getter[Beetroot, Banana](_.b)
val setBanana = monocle.Setter[Beetroot, Banana](f => br => br.copy(b = f(br.b)))
val foldBanana = new monocle.Fold[Beetroot, Banana] { def foldMap[M: scalaz.Monoid](f: Banana => M)(br: Beetroot): M = f(br.b)}
val br = Beetroot(Banana(Potato(4)))

scala> get"$br.$setBanana.p.i"
<console>:20: error: The macro internally failed to get values from optic Setter.
Please consider filing an issue at https://github.com/kenbot/goggles/issues.
       get"$br.$setBanana.p.i"

scala> set"$br.$getBanana.p.i" := 5
<console>:20: error: value asSetter is not a member of monocle.Getter[Unit,Int]
       set"$br.$getBanana.p.i" := 5
       ^
scala> set"$br.$foldBanana.p.i"
<console>:20: error: value asSetter is not a member of monocle.Fold[Unit,Int]
       set"$br.$foldBanana.p.i"

Shortcut for Iso.id to introduce type in lens mode

Lens mode has to start with an interpolated optic, so that it knows the type to start with. If you want to start with something else, it's possible to put a monocle.Iso.id[MyType] in the leftmost position, which does the trick.

ie: lens"${Iso.id[Blah]}.foo.bar"

However, this has some problems:

• Requires the user to know about the concept of an Iso just for an implementation hack to supply the type to the macro
• Isn't very "you already know how do it", requires detailed knowledge of Monocle's API
• Adds an additional method call(s) at runtime
• Adds a pointless Iso line at the top of the compile error table

One idea is to have a method with an obvious name (which one?) returning Iso.id, that is in scope with an import goggles._. With the right name this might arguably, barely satisfy "you already know how to use it", but wouldn't fix the performance.

Performance

Iso.id cannot itself be changed to override composition to just return the other thing, because the composeXXX methods are final, and should probably stay that way. However, if we make our own type that behaves exactly like Iso, we can make it have zero runtime overhead. While it essentially functions as a terminal object in the lens hierarchy, it might be better called "TypeHint" rather than "IdIso", which is its actual reason for existing. We could easily make the error tables omit or specially handle the "TypeHint" line at the start.

Code quality

A benefit here is that get/set modes could use this approach too, instead of the AppliedObject.const Unit <--> A Iso hack. This could mean more unified code with less special cases, and the generated code would be more recognisable Monocle code, with the object applied at the end. This would also improve their performance, as there would no longer be an runtime-unnecessary Iso at the start.

Naming

This is probably the hardest part of this ticket. There probably isn't a name that a typical user "already knows". Too rare, and it's just yet another obscure library curiosity. Too common, and there will be namespace problems for users.

Some examples:
lens"${choose[Blah]}.foo.bar"
lens"${select[Blah]}.foo.bar"
lens"${selectType[Blah]}.foo.bar"
lens"${typeOf[Blah]}.foo.bar"
lens"${t[Blah]}.foo.bar"
lens"${on[Blah]}.foo.bar"
lens"${from[Blah]}.foo.bar"
lens"${fromType[Blah]}.foo.bar"
lens"${typed[Blah]}.foo.bar"

Publish 1.0 to Maven

Get all that sorted out, and add a Getting Started section at the top of the README with sbt dependency text.

Automatic support for named prisms

In the same way that the .name syntax automatically navigates case-class-like fields with Lenses, it should be able to navigate ADTs in sealed class hierarchies with Prisms. This should be possible with getKnownDirectSubclasses.

But using what syntax? We could use the name of the subclass, where it looks for a field by that name first, then looks for a subclass name:

val myOpt: Option[Int] = Some(4)
set"$myOpt.Some.x"' += 1
// Some(5)

Is that obvious enough to justify "you already know how to use it"? What else could it be?

Publish Scala.js version in the repos

State/Reader support

But what syntax?
getS"..."/setS"..."/getR"..."?

To what extent should we use Monocle's State support?

Test in dotty

This is more for discussion rather than saying "we should do it now", but I want to note that dotty seems to be going in the direction of using more string interpolation (e.g., XML literals will likely be replaced as interpolated strings), and I'd hazard a guess that using VS Code with it (or any other editor that supports the language protocol in the future) would actually show syntax errors within an editor.

Add "Comparison to other approaches" section in README

The README needs a section comparing Goggles' design choices to other Scala approaches, such as QuickLens, Monocle's built-in .lens DSL and Shapeless Lenses.

Indexing is broken for literals

When a literal is interpolated in an index position, the type is overspecialised to the exact literal instance type (Int(1), String("abc"), Char('a') etc), rather than the expected type (Int, String. Char).

I believe the problem lies in the code that parses the implicit monocle.function.Index[S,I,A] instance.

Example:

scala> val x = Map("a" -> 1, "b" -> 2, "c" -> 3)
x: scala.collection.immutable.Map[String,Int] = Map(a -> 1, b -> 2, c -> 3)

scala> get"$x[${"a"}]"
<console>:29: error: No implicit monocle.function.Index[scala.collection.immutable.Map[String,Int], String("a"), _] found to support '[]' indexing

scala> get"$x[${"a": String}]"
res27: Option[Int] = Some(1)

Can named lenses be inlined for performance?

@julien-truffaut mentioned that:

Lenses still cause a performance overhead around 2x comparing to hand written copy (as measured with jmh)

Since Goggles is generating code anyway, could we gain a speedup by directly generating the underlying code for chains of .name optics?

Getters would fuse into a single Getter with direct method calls: foo.bar.baz
Setters would fuse into nested copy calls: foo.copy(bar = foo.bar.copy(baz = <new thing>))
Lenses could fuse both. (Don't forget that lens mode might produce Isos as well, pending #26!)

We would have to examine the produced bytecode to ensure parsimony, and we would have to introduce benchmarks to demonstrate that sufficiently impressive speedups occur in practice.

Since we would still want to provide the illusion of the segments being separate optics (ie in the fancy error message table), some clever software engineering might be required to maintain a modular design within the macro code.

Add example code

We should have some examples of idiomatic Goggles in code.

Unclear CTE using `set"..." ~= { ... }` with a generic case class

I was trying to use goggles to modify a value inside a case class with a type parameter (type parameters should remain the same).

Simplified, code like this:

import goggles._
case class Boxed[+A](get: A)
set"${Boxed(1)}.get" ~= { _ + 1 }

results in following error:

The types of consecutive sections don't match.
 found   : Playground.this.Boxed[Int]
 required: Playground.this.Boxed[Int] 

 Sections │ Types            │ Optics 
──────────┼──────────────────┼────────
 $        │ Boxed[Int]       │        
 .get     │ Boxed[Int]  ⇒  A │ Setter

while I would expect Boxed(2), it would be nice even if compile-time error was less misleading.

Travis CI

It would be nice to have Travis CI set up in Github.

Can types be obtained from the outer expression?

Currently, "lens" expressions have to have an interpolated lens in the first position, because otherwise the types can't be inferred.

If we have, say:

val x: Lens[Monkey, Banana] = lens"..."

does that mean we can allow expressions that start with ?, .name, [i], or *, plugging in what we know about Monkeys and Bananas? Or does this fit into the official discouragement of accessing the surrounding scope?

Can uninferred types of interpolated args be repaired?

Within the macro, type information flows from left to right throughout, allowing type inference to finish the job in the generated code.

However, interpolated values are typechecked before the macro runs. Some interpolated optics that have type parameters (ie monocle.function.At.at) crap out with [Nothing,Nothing], because they don't have the information. Is it possible within the macro to redact the type parameters, inserting the information that we already know?

Tuple syntax inside expressions

Possible syntax extension:

get"$foo.(bar, baz).blah"
// (foo.bar.blah, foo.baz.blah)

get"${List(1,2,3)}.(*, [0])"
// List((1,1), (2,1), (3,1)) 

I suspect that a user could guess the meaning without reading documentation, although the syntax is certainly not as universally recognisable as the existing features. This might greatly increase the internal complexity of the implementation, especially if the (...)s can be recursively nested.

This would significantly increase the complexity of the AST, which would become explicitly recursive, and the parser would need to be expanded. This might pose a problem for the detailed error messages - should each intermediate and sub-expression in the tree be shown in the table as a separate line? I suspect so.

Gracefully handle lack of -Yrangepos

Currently if -Yrangepos is not set, then interpolated args cannot be displayed in the leftmost column of the error table, and the position of the ^ indicator cannot be accurately set.

We should gracefully show something meaningful, such as "$ARG" for the argument names (rather than an empty string), and fixing the ^ at the start of the expression. We could also print a message that suggests that the user add scalacOptions += "-Yrangepos" to build.sbt.

Support for both cats/scalaz

It would be good to support both Tweedledum & Tweedledee, but since Monocle is still wrestling with this dilemma, we should probably wait and see how they resolve it:

optics-dev/Monocle#419

Improve error messages

I want the macro user-errors to look something like:

  Section     | Source      | Target      | Optic type
  ------------+-------------+-------------+--------------------------
  $bunches    |             | List[Bunch] | 
  *           | List[Bunch] | Bunch       | Traversal
  .banana     | Bunch       | Banana      | Lens
  .foo        | Banana      | Int         | Lens
  .$evenPrism | Int         | Int         | Prism, returning Optional
  .foo        | Banana *** ERROR: "Banana" doesn't match "Int"
  .bar

  Perhaps you meant one of these methods in Bunch?
    .brian    : Bunch => String
    .burgle   : Bunch => Int
  
  There is a monocle.Lens[Bunch, Int] in scope called "banana", did you mean 
  to interpolate it?  ie. get"$bunches*.$banana.foo.$evenPrism"

The type & optic information is all there, although the suggestions might need a bit of work.

How could I chain together sets?

val a = set"$to.name" := from.contractName
val b = set"$a.code" := from.code
val c = set"$b.something" := from.somethingName
c

Its often the case that I will need to chain sets together like above, is there is easy syntax for the above?

Extra suggestions for error table

Implement suggestions to be show to the user for MacroUserErrors.
For instance:

  Perhaps you meant one of these methods in Bunch?
    .brian    : Bunch => String
    .burgle   : Bunch => Int
  
  There is a monocle.Lens[Bunch, Int] in scope called "banana", did you mean 
  to interpolate it?  ie. get"$bunches*.$banana.foo.$evenPrism"

Here for a failed name, we are suggesting similar names that would have worked. Matching no-arg method names by first letter is probably sophisticated enough.

Also, a common user error is to name an optic in scope, accidentally leaving out the "$" that would allow it to be interpolated. We can typecheck the symbol, and if it corresponds to a monocle optic with the correct types, we can list it as a suggestion.

Can't access fields on varargs case class

If a case class field is marked as varargs, then "get" and "set" expressions cannot recognise the expected copy method.

Once #3 is resolved, "get" shouldn't be a problem at least.

scala> case class Foo(ns: Int*)
defined class Foo

scala> get"$res1.ns"
<console>:26: error: Can't update 'ns', because no 'copy' method found on Foo
       get"$res1.ns"
       ^

scala> set"$res1.ns" := Nil
<console>:26: error: Can't update 'ns', because no 'copy' method found on Foo
       set"$res1.ns" := Nil
       ^

Support for unapplied get/set?

A common Monocle idiom, currently unsupported by Goggles, is composing lens set/modify expressions before the object is applied. This returns an endofunction, with pleasing compositional properties. For instance:

val x: Item => Item = itemQtyLens.modify( _ + 1) andThen itemPriceLens.set(4)

It also aligns with the common FP intuition of "compose first, many times; execute last, once".

Is there some way to support this idiom in get/set modes without making everything weird and complicated? One way might be to allow a from[MyType] type hint (as per #31) in the left-most position instead of the source object, which would return an endofunction rather than the usual result. This might prove a confusing special case though.

Some terminals can't display fancy unicode error messages

On my poor Windows 10 default, the unicode borders of the fancy error tables turn into question marks:

 Sections  ? Types                         ? Optics
???????????????????????????????????????????????????????????????????????
 $myBasket ? ShoppingBasket                ?
 .items    ? ShoppingBasket  ?  List[Item] ? Getter
 *         ? List[Item]      ?  Item       ? Traversal, returning Fold
 name      ? Item            ?  ???        ?

       get"$myBasket.items*.name"
                            ^

I wonder if there's some way to detect this, and switch to ASCII hyphens & pipes - or just use them and forget the fancy stuff.

Intellij support

I don't know if this is possible, but it would be nice to get rid of the red squigglies in Intellij.

It would have to know about the static type selected by the whitebox macro, so as to support the appropriate += := ~=` operators for set commands.

It would be good to have click-through for the .name fields, and auto-complete from known zero-arg fields and Monocle optic instances. Some sort of nice colouring too.

Assignment operator sugar for collections

Currently, we have some mildly convenient syntax sugar for updating numeric values:

set"$foo.int" += 3
set"$foo.int" *= 3
set"$foo.int" -= 3

Which translate to

set"$foo.int" ~= (_ + 3)
set"$foo.int" ~= (_ * 3)
set"$foo.int" ~= (_ - 3)

Despite the modest gain, it is appropriate because the operators are so instantly recognisable -- it is not a surprising feature.

It may be similarly justifiable to provide overloads for common collection operators:

set"$foo.list" ::= element
set"$foo.list :::= list
set"$foo.coll" ++= coll
set"$foo.coll" :+= postElement
set"$foo.coll" +:= preElement

These can be annoyingly fiddly with ~= explicit functions.

How should it be implemented? The monocle Cons functionality could work with ::, but none of the others would. If we used the standard Scala CanBuildFrom implicit stuff, then it would have to be consistent, rather than one thing using a Monocle typeclass.

Can it work as expected with polymorphic STAB optics?

Needs more experimentation to decide whether it is possible, or worth it.

lens mode .name should generate Isos where possible

Using the .name syntax, get and set modes generate the most general possible optics, Getters and Setters respectively. This means that more code can participate, such as no-arg methods with no copy method. Because we already know what we want to do with it (ie "get", "set"), there is no need for more capable optics to be selected.

Conversely, lens mode produces an optic; we don't know how the user will use it. Therefore, we should choose maximally capable, specific optics, rather than constrained generic ones. Currently, Lenses will be generated.

In lens mode, where a target value is known to be isomorphic to the source object (ie a one-argument case class), then an Iso should be generated instead of a Lens.

Show interpolated code as-is in error messages

Currently, show() prints out fully resolved names for interpolated code, which makes error messages awkward. It would be better to show the exact text the user typed in. Can this be done with the macro/reflection api?

Haskell style commas, ranges for indices

Would this be a sensible extension to the index behaviour?

get"$kittens[0, 2, 3..10]"

Allowing index interpolation in to any position. The trouble is the ".." requires some concept of an enumeration - is there a standard typeclass for this in the std lib? Does it yield a Traversal over all those values, or an Optional of a tuple containing the values? What Would Haskell Do?

.name syntax should generate minimal optics structure

Currently, .name generates a monocle.Lens for a full case-class field.

For "get":
It should work for any no-arg method, and generate a monocle.Getter.

For "set":
It should work for anything with a copy method with a parameter of the appropriate name, where there is a named parameter with the same name. It should generate a monocle.Setter, returning whatever the copy method returns.

For "lens":
It should continue to generate a full monocle.Lens.

Polymorphic assignment

Case classes naturally support polymorphic update:

case class Foo[A](foo: A)
val f: Foo[Int] = Foo(3)
>> f: Foo[Int]

f.copy(_.toString)
>> res2: Foo[String]

This should work in Goggles too, as expected, using PSetters and PLenses.

set"$f.foo" ~= (_.toString)

Support for Scala Native

Monocle is adding support: optics-dev/Monocle@5d1e285

We should too.

Compile errors: ^ should point to the correct spot

Currently, the caret just sits at the start of the expression:

scala> get"foo"
<console>:12: error: value get is not a member of StringContext
       get"foo"
       ^

It should get set to the position inside the expression where the failure occurred. This will have something to do with the Position class.

katakurba

Composing Getter & Optional returns lists

get"$myObj.$lens.$optional" returns an Option, but get"$myObj.$getter.$optional" returns a List. This is because no "Fold1" exists, and a "Fold" is performed instead. This is surprising behaviour.

We can simulate the expected behaviour by tracking Folds that definitely only have 0-1 elements, and generating a .headOption on the end afterwards.

Something like:

case class Fold(notMulti: Boolean) extends OpticType

See optics-dev/Monocle#484

Replace Specs2 with uTest

Specs2 doesn't have a great ScalaJS story at the moment. We are not using any fancy features with our tests at the moment, we just need asserts and ScalaCheck.

If we move our tests to uTest instead, we should be able to run them cross platform, so ScalaJS is tested too.

Error table with single entry looks bad

scala> lens"$myBasket.items"
<console>:18: error: Interpolated value $myBasket must be an optic; one of monocle.{Fold, Getter, Setter, Traversal, Optional, Prism, Lens, Iso}; found: goggles.Fixture.ShoppingBasket

 Sections  │ Types │ Optics
───────────┼───────┼────────
 $myBasket │ ⇒     │

       lens"$myBasket.items"
       ^

This looks terrible and isn't helpful. If there is only one entry, then the table should not be displayed.

Cross compile for 2.11

It would be good to support 2.11 as well. Hopefully this is easy, but the macro/reflection stuff may prove stubborn...

String literal indexes

Strings seem a common enough index type that a literal syntax could be useful, ie for JSON or XML documents. Single quotes will be the most convenient. I don't think there is any value in supporting double quotes as well.

More helpful error message for missing Each/Possible/Index implicits

Currently import goggles._ is not "batteries included", meaning that the *, ? and [] operators won't do anything until implicit m.f.Each, Possible and Index instances are imported or created.

Either:
a) import goggles._ should include default instance imports
b) The error message should be very helpful when the missing implicit is for a Monocle-supported instance, specifically suggesting the required Monocle import.

Including a bunch of instances by default is convenient, but seems like a bad idea; most libraries (including Monocle) now separate instance or syntax imports from their core concepts. Otherwise the user will be poorly equipped to reason about the consequence when default implicit instances swordfight with new ones.

On the other hand, really helpful error messages is what Goggles is all about; the user might be mildly inconvenienced by having to import something, but they will be fully informed.