-
Notifications
You must be signed in to change notification settings - Fork 186
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
Update scalameta, semanticdb-scalac-core, ... to 4.7.3 #1725
Conversation
e4bc3b0
to
9c0d24f
Compare
@@ -118,7 +118,7 @@ The `replaceTree()` patch is a convenience operator for `removeTokens` and | |||
|
|||
```scala mdoc | |||
doc.tree.collect { | |||
case println @ Term.Apply(Name("println"), _) => | |||
case println @ q"println(..$_)" => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not matter here as println
can only have one list of arguments, but I didn't find a built-inway to match a Term.Apply
with an arbitrary number of elements.
@ val Term.Apply(expr, _) = q"println()"
expr: Term = Term.Name(value = "println")
@ val Term.Apply(expr, _) = q"println"
scala.MatchError: println (of class scala.meta.Term$Name$TermNameImpl)
ammonite.$sess.cmd48$.<clinit>(cmd48.sc:1)
@ val q"$expr(...$exprssnel)" = q"println()"
expr: Term = Term.Name(value = "println")
exprssnel: List[Term.ArgClause] = List(Term.ArgClause(values = List(), mod = None))
@ val q"$expr(...$exprssnel)" = q"println"
expr: Term = Term.Name(value = "println")
exprssnel: List[Term.ArgClause] = List()
@ val q"$expr(..$_)" = q"println"
scala.MatchError: println (of class scala.meta.Term$Name$TermNameImpl)
ammonite.$sess.cmd51$.<clinit>(cmd51.sc:1)
It looks like the -nel
suffix has no effect. Also, having a Term.Name
match the quasiquote syntax for Apply
looks weird 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't 3-dots work for arbitrary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't 3-dots work for arbitrary?
It was too arbitrary. I probably misunderstood the usage of suffixes, but I was expecting q"println(...$exprssnel)"
not to match q"println"
as the captured list is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never saw those suffixes before. probably convention in the examples on that page.
9c0d24f
to
feb0f15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting scalameta 4.7.x upgrade on hold for the moment as:
- Tree extractors (
unapply
) are key to the current Scalafix API. Without them, the UX becomes much worse for rule authors (example). - Code generated by scalameta 4.7.x quasiquotes references internal code, that is not guaranteed to be backward compatible. Rules relying on quasiquotes and built against scalameta 4.7.x would be at risk of not working on a future version of scalafix.
Related discussion ongoing on the scalameta Discord.
@@ -118,7 +118,7 @@ The `replaceTree()` patch is a convenience operator for `removeTokens` and | |||
|
|||
```scala mdoc | |||
doc.tree.collect { | |||
case println @ Term.Apply(Name("println"), _) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scalameta/scalameta#3067 should fix both issues in the upcoming scalameta release 🎉 |
Superseded by #1735. |
Came to the repo to see if this was being worked on, nice to see that it is :-) (I don't like to see the Scalameta version we use in the Scala 2 community build falling too far behind...) |
Is there any chance this or #1735 will be ready to go soon-ish? I just ran into scalameta/metals#4584 in my codebase and see that it's fixed in scalameta 4.7.0 Alternatively, is it safe to override the scalameta dependency version just in my codebase? |
It's not neccessary for Metals, we use our own internal version of Scalameta, we need to update that in Metals. |
Sorry if I was unclear @tgodzik -- I'm not using metals, just scalafix. I just happened to find that bug on the metals repo that perfectly describes the error I ran into |
I'll look at releasing that this week-end |
@mrdziuban until a new release is cut, using the latest SNAPSHOT should fix your issue. If it does not, please open a ticket. |
Updates
from 4.6.0 to 4.7.3.
GitHub Release Notes - Version Diff
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a
.scala-steward.conf
file.Have a fantastic day writing Scala!
Adjust future updates
Add this to your
.scala-steward.conf
file to ignore future updates of this dependency:Or, add this to slow down future updates of this dependency:
labels: library-update, early-semver-minor, semver-spec-minor, version-scheme:semver-spec, commit-count:1