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

Scalafix 0.9.26 breaks scalafix-organize-imports unit test #1337

Closed
liancheng opened this issue Feb 28, 2021 · 5 comments
Closed

Scalafix 0.9.26 breaks scalafix-organize-imports unit test #1337

liancheng opened this issue Feb 28, 2021 · 5 comments

Comments

@liancheng
Copy link
Contributor

liancheng commented Feb 28, 2021

###########> Diff       <###########

===========
=> Obtained
===========

package fix

import fix.QuotedIdent.`a.b`.c
import fix.QuotedIdent.`a.b`.{ d }
import fix.QuotedIdent.a.b
import fix.QuotedIdent.macro

object ExpandRelativeQuotedIdent

         
=======
=> Diff
=======
--- obtained
+++ expected
@@ -2,6 +2,6 @@
∙
+import fix.QuotedIdent.`a.b`
+import fix.QuotedIdent.`a.b`.`{ d }`
 import fix.QuotedIdent.`a.b`.c
-import fix.QuotedIdent.`a.b`.{ d }
-import fix.QuotedIdent.a.b
-import fix.QuotedIdent.macro
+import fix.QuotedIdent.`macro`
[info] - fix/ExpandRelativeQuotedIdent.scala *** FAILED ***
[info]   see above (AbstractSemanticRuleSuite.scala:97)

So the identifier `{ d }` in the input is rewritten into { d } without quotation.

More details can be found in this build of liancheng/scalafix-organize-imports#158.

I'm not sure whether the problem is from 0.9.25 or 0.9.26, because the Scala Steward PR for upgrading Scalafix to 0.9.25 failed and I didn't get enough cycles to investigate.

@bjaglin
Copy link
Collaborator

bjaglin commented Feb 28, 2021

I'm not sure whether the problem is from 0.9.25 or 0.9.26, because the Scala Steward PR for upgrading Scalafix to 0.9.25 failed and I didn't get enough cycles to investigate.

Looking at https://github.com/liancheng/scalafix-organize-imports/pull/153/checks?check_run_id=1736149686, we can see that the tests pass, but the coverage task fail (as mentioned in liancheng/scalafix-organize-imports#153 (comment)), so the problem is related to the 0.9.26 upgrade, and most likely to the scalameta bump. I'll have a look.

@bjaglin
Copy link
Collaborator

bjaglin commented Feb 28, 2021

Looks like a regression introduced in scalameta 4.4.9 (first present in scalafix 0.9.26) scalameta/scalameta#2260

@liancheng
Copy link
Contributor Author

I think we need to bump Scalameta version to 4.4.11 or a higher version, which contains scalameta/scalameta#2260, to resolve this one.

@bjaglin
Copy link
Collaborator

bjaglin commented May 14, 2021

I verified via liancheng/scalafix-organize-imports#179 that this is fixed on main. Keeping open for confirmation until a release is cut out next week.

@bjaglin
Copy link
Collaborator

bjaglin commented May 24, 2021

Confirmed via liancheng/scalafix-organize-imports#181

@bjaglin bjaglin closed this as completed May 24, 2021
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

No branches or pull requests

2 participants