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

Missing autocompletion for instances of Selectable #14891

Closed
kitlangton opened this issue Apr 8, 2022 · 7 comments · Fixed by #15283
Closed

Missing autocompletion for instances of Selectable #14891

kitlangton opened this issue Apr 8, 2022 · 7 comments · Fixed by #15283

Comments

@kitlangton
Copy link

kitlangton commented Apr 8, 2022

Compiler version

3.1.1

Minimized code

class Structural(map: Map[String, Any]) extends Selectable:
  def selectDynamic(name: String) = map(name)

type Person = Structural { val: name; val age: Int }

val p = new Structural(Map("name" -> "jacob", "age" -> 5)).asInstanceOf[Person]

p.name //I should see name + age in autocomplete

Output

There is no autocomplete for name or age.

Expectation

It would be great if there were autocomplete for name and age 😄

Notes

I've copied this issue from a metals issue after being informed that it may make more sense here (scalameta/metals-feature-requests#238).

My hopes are that autocomplete for Selectable types can help provide a suitable replacement for annotation macros. I'm imagining a Macro that would generate a refined Selectable type. This Selectable instance would be a bag of autocompleteable methods based upon the structure of the given case class. Generating Lenses, for instance, could look like this:

case class Person(name: String, age: Int)

object Person {
  val lenses = Lenses.gen[Person]
}

Person.lenses.name // For this to be tenable, this would need to autocomplete with the type Lens[Person, String] 
@kitlangton kitlangton added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 8, 2022
@odersky
Copy link
Contributor

odersky commented Apr 9, 2022

The problem is that Selectable generates very slow & unportable code since it relies on Java reflection. I am not sure we want to encourage its use and make it easier. Selectable was introduced as a backstop, to support the use cases where reflection was used before (for instance in Chisel). Basing lenses on it looks like a bad idea.

@joroKr21
Copy link
Member

joroKr21 commented Apr 9, 2022

But I thought the new Selectable is generalised and doesn't have to use reflection.
The Person example uses a Map[String, Any] implementation.

@bertlebee
Copy link

@odersky I think that autocomplete support is very important to realise the goals of programmatic structural types. From the docs (emphasis mine)

https://docs.scala-lang.org/scala3/reference/changed-features/structural-types.html#motivation

Structural types help in situations where we would like to support simple dot notation in dynamic contexts without losing the advantages of static typing. They allow developers to use dot notation and configure how fields and methods should be resolved.

Autocomplete is a massive advantage of static typing and is especially important for the success of any public API.

I have (and I'm sure @kitlangton would be) been using inlines/macros to define selectDynamic, so there is no overhead at all, and just as efficient. A mandatory import protects against all the ways that use reflection.

https://docs.scala-lang.org/scala3/reference/changed-features/structural-types.html#using-java-reflection

Structural calls like this tend to be much slower than normal method calls. The mandatory import of reflectiveSelectable serves as a signpost that something inefficient is going on.

The sketch above certainly looks more appealing to use than current lens libraries but would completely fail without autocomplete.

The legitimate use of a language feature shouldn't be discouraged by making the API user experience awkward. The current use of mandatory imports for the reflective cases is sufficient IMO. When it's packaged up in a library, end users wouldn't even know they're using programmatic structural types without digging into the implementation.

@odersky
Copy link
Contributor

odersky commented Apr 10, 2022

OK, I note that there are important use cases. Thanks for pointing them out!

@odersky odersky added itype:enhancement area:ide area:repl and removed itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 10, 2022
@kitlangton
Copy link
Author

kitlangton commented Apr 10, 2022

Thanks for considering this @odersky! And thank you @robmwalsh for laying out a cogent technical appeal 😄. I was benighted of the performance implications either way: didn't realize it was potentially bad—and further didn't realize it could be potentially not bad in certain cases.

Anyhow, I'm merely seeking to regain some of the conveniences of annotation macros under the jurisdiction of our new hygienic and extra-lawful sheriff (Sheriff Dotty 👋🤠). So pardon if my questions violate certain phase consistencies, as I'm grasping monomaniacally for ergonomics and boilerplate elimination. I am more than happy to be shot down by those with the real domain knowledge, such as you two fine gents.

So yeah, to extend my ignorant, wistful wishing by one step—possibly into the domain of the untenable:

It would be extra delightful if that fancy new export keyword could join in the fun:

case class Person(name: String, age: Int)

object Person {
  val lenses = Lenses.gen[Person]
  export lenses._
}

Person.name // Ah, ergonomic apogee! 🌞 

I feel this would fully satisfy the best and most reasonable (IMHO) use-cases for annotation macros, which is to outfit a companion object with some convenience methods derived from its companion type. I assume export has more stringent requirements than would allow for such a thing. But I'd be very excited if the new macro system were to allow for this more constrained extension (compared to the full Pandora's box of hell that I'm sure the traditional annotation macros unleash).

All done! Thanks for reading. :bowtie:

@bertlebee
Copy link

bertlebee commented Apr 10, 2022

@kitlangton I feel like putting me in the same basket as @odersky is generous by a long stretch. I am merely a socially awkward person that knows what I want/like and lack whatever filter is usually required to stop people from debating those with significantly more domain knowledge/power/authority than myself :) Sometimes I look like and idiot but you can learn a lot from getting schooled :p

Thanks to @odersky for considering my appeal!

Being able to export would be great. Gut feel is that it would be achievable, but could be complicated if you want to do more than one export like this as what you're really exporting is syntax sugar for selectDynamic, which would be exported every time you export this way on a Selectable

Edit: "Look like and idiot" was unintentional but proves my point

@jdegoes
Copy link

jdegoes commented Apr 20, 2022

As a user, I would hope and expect that auto-complete is supported for all static types, including structural types, and I can think of many applications for this: for example, adding typing information to Spark workflows via metaprogramming (which can easily be done, but will be of limited use without auto-complete).

Separately, I do believe that export should work uniformly across nominal and structural types. One of the most tangible and practical perks of working with Scala 3 is the enhanced uniformity it brings to the language, and it would be a shame (in my view) to pass up the opportunity to make export work uniformly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants