Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

characters ` are missing after reorganize import #101

Closed
JR-Utily opened this issue Jul 21, 2020 · 8 comments
Closed

characters ` are missing after reorganize import #101

JR-Utily opened this issue Jul 21, 2020 · 8 comments
Labels
bug Something isn't working
Milestone

Comments

@JR-Utily
Copy link

Hello !

In my company, we have a macro package in which we write our macros.
As macro is a reserved keyword, the package is name macro with backquote `

When we import something from this package, we write for example:

import com.company.`macro`.Dummy

but when this is reorganized, the ` are removed, and I have just:

import com.company.macro.Dummy

which is not a valid syntax and leads to compilation error

@liancheng
Copy link
Owner

Hey @JR-Utily, thanks for reporting! Which version of OrganizeImports are you using? I think this issue has been addressed a while ago and here are some test cases:

@liancheng
Copy link
Owner

liancheng commented Jul 21, 2020

@JR-Utily, actually, I reproduced this issue with the most recent master (f5ee0a8) with the following diff:

diff --git a/input/src/main/scala/fix/ExpandRelativeQuotedIdent.scala b/input/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
index d91fe7f..59ed1ee 100644
--- a/input/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
+++ b/input/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
@@ -5,6 +5,7 @@ OrganizeImports.expandRelative = true
 package fix
 
 import QuotedIdent.`a.b`
+import QuotedIdent.`macro`
 import `a.b`.c
 import `a.b`.`{ d }`
 
diff --git a/output/src/main/scala/fix/ExpandRelativeQuotedIdent.scala b/output/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
index 24c79cb..633c3f3 100644
--- a/output/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
+++ b/output/src/main/scala/fix/ExpandRelativeQuotedIdent.scala
@@ -3,5 +3,6 @@ package fix
 import fix.QuotedIdent.`a.b`
 import fix.QuotedIdent.`a.b`.`{ d }`
 import fix.QuotedIdent.`a.b`.c
+import fix.QuotedIdent.`macro`
 
 object ExpandRelativeQuotedIdent
diff --git a/shared/src/main/scala/fix/QuotedIdent.scala b/shared/src/main/scala/fix/QuotedIdent.scala
index 010e8e2..121d61f 100644
--- a/shared/src/main/scala/fix/QuotedIdent.scala
+++ b/shared/src/main/scala/fix/QuotedIdent.scala
@@ -7,4 +7,6 @@ object QuotedIdent {
       object e
     }
   }
+
+  object `macro`
 }

Test failure output:

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

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

package fix

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

object ExpandRelativeQuotedIdent


=======
=> Diff
=======
--- obtained
+++ expected
@@ -5,3 +5,3 @@
 import fix.QuotedIdent.`a.b`.c
-import fix.QuotedIdent.macro
+import fix.QuotedIdent.`macro`

Hmm, I guess it's because macro is a keyword. This might be a bug in the Scalafix/Scalameta side, though. At least this was the case the last time I tried to fix some quoted-identifier issue. Will take a look later.

@liancheng liancheng added the bug Something isn't working label Jul 21, 2020
@liancheng
Copy link
Owner

liancheng commented Jul 21, 2020

Yeah, I believe the problem is in the Scalameta Scala.Names.encode() method, which doesn't handle Scala keywords while quoting identifiers. Could be tricky to fix since it's outside OrganizeImports, but maybe we can hack a little bit...

Update: Per discussion in scalameta/scalameta#2096, it's not really about the Scala.Names.encode() method, but still a Scalameta side issue.

@liancheng
Copy link
Owner

The Scalameta side issue has been fixed in scalameta/scalameta#2098, waiting for new releases of Scalameta and Scalafix.

@JR-Utily
Copy link
Author

Thank you so much for your reactivity !

@liancheng liancheng added this to the v0.4.1 milestone Aug 3, 2020
@liancheng liancheng mentioned this issue Aug 10, 2020
liancheng added a commit that referenced this issue Sep 21, 2020
@liancheng
Copy link
Owner

@JR-Utily, a fix for this issue has been included in the v0.4.1 release.

@JR-Utily
Copy link
Author

Thanks !
Me and my team are very thankful !

@JR-Utily
Copy link
Author

JR-Utily commented Oct 5, 2020

I don't know why, I try to fix from

import com.company.`macro`.{Dummy, DefaultImpl}

to

import com.company.`macro`.Dummy
import com.company.`macro`.DefaultImpl

and it were not ok, it put it again as

import com.company.macro.Dummy
import com.company.macro.DefaultImpl

but after adding back the quotes in the defactorized lines, it keeps them correctly on next reorganize calls, so this is ok for me

but maybe there is still a bug (non-blocking this time)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants