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 more information to reflection guide #2005

Merged
merged 18 commits into from May 27, 2021

Conversation

vincenzobaz
Copy link
Contributor

This PR adds some information that I wished had been documented when I started to use the API.

Most of the elements were discussed with @liufengyun and @MaximeKjaer

Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Vincenzo! I found a few typos or style remarks.

_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
@@ -75,6 +98,47 @@ def g[T: Type](using Quotes) =
...
```

## Symbols
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A thing worth checking is whether the flavor of Markdown used for the docs supports heading IDs. In my experience, adding is a good way of future-proofing internal links, in case a title changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs.scala-lang uses kramdown. I ran jekyll serve and the internal link seems to work

vincenzobaz and others added 3 commits May 11, 2021 17:05
Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
Co-authored-by: Maxime Kjaer <maxime.kjaer@gmail.com>
Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition


For example [`TypeBounds`](https://dotty.epfl.ch/api/scala/quoted/Quotes$reflectModule.html#TypeBounds-0), a subtype of `TypeRepr`, represents a type tree of the form `T >: L <: U`: a type `T` which is a super type of `L`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For example [`TypeBounds`](https://dotty.epfl.ch/api/scala/quoted/Quotes$reflectModule.html#TypeBounds-0), a subtype of `TypeRepr`, represents a type tree of the form `T >: L <: U`: a type `T` which is a super type of `L`
For example [`TypeBounds`](https://dotty.epfl.ch/api/scala/quoted/Quotes$reflectModule.html#TypeBounds-0), a subtype of `TypeRepr`, represents a type of the form `T >: L <: U`: a type `T` which is a super type of `L`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be confusing to call TypeRepr when we have TypeTree around

_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
is not used, then the `tree` of the symbol will not be available. Symbols originated from
Java code do not have an associated `tree`.

## Suggestion and anti-patterns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those should be moved to https://docs.scala-lang.org/scala3/guides/macros/best-practices.html. There we can expand a bit each point and add some examples if possible.

We could leave a link to that page here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved a few points to best-practices.md and provided small examples

vincenzobaz and others added 6 commits May 12, 2021 11:43
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
Co-authored-by: Nicolas Stucki <nicolas.stucki@gmail.com>
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved

- `tpe.typeSymbol` returns the symbol of the type represented by `TypeRepr`. The recommended way to obtain a `Symbol` given a `Type[T]` is `TypeRepr.of[T].typeSymbol`
- For a singleton type, `tpe.termSymbol` returns the symbol of the underlying object or value.
- `tpe.memberType(symbol)` returns the `TypeRepr` of the provided symbol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe give an example? (That could be added later in a separate PR)

Something that would be useful IMHO would be to show a concrete example of what this would expand to (e.g., how do I construct a tree for the type expression Foo#Bar (for some type Foo that has a type member Bar).

_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/best-practices.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
_overviews/scala3-macros/tutorial/reflection.md Outdated Show resolved Hide resolved
@@ -40,14 +40,14 @@ This will import all the types and modules (with extension methods) of the API.

## How to navigate the API

The full imported API can be found in the [API documentation for `scala.quoted.Quotes.reflectModule`][reflection doc].
The full API can be found in the [API documentation for `scala.quoted.Quotes.reflectModule`][reflection doc].
Unfortunately, at this stage, this automatically generated documentation is not very easy to navigate.

The most important element on the page is the hierarchy tree which provides a synthetic overview of the subtyping relationships of
the types in the API. For each type `Foo` in the tree:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not be fully clear that Foo is a metavariable, we could also use something like <TYPE>Methods. But if you think that is not necessary, then I am also ok with how it is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this can also be addressed in a separate PR. To not block any longer, I am going to accept and merge it as is.

@b-studios b-studios merged commit ac7796b into scala:main May 27, 2021
@vincenzobaz vincenzobaz deleted the reflection-guide branch May 27, 2021 13:14
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.

None yet

7 participants