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

Formatting is not preserved in imports renamed with => #127

Closed
guilgaly opened this issue Nov 16, 2020 · 8 comments · Fixed by #129 or #130
Closed

Formatting is not preserved in imports renamed with => #127

guilgaly opened this issue Nov 16, 2020 · 8 comments · Fixed by #129 or #130
Labels
bug Something isn't working
Milestone

Comments

@guilgaly
Copy link

guilgaly commented Nov 16, 2020

If I have the following imports (formatted with spaces.inImportCurlyBraces = true in scalafmt, which adds spaces within the curly braces), and if they are already correctly organized:

import bar.{ Bar => Baz }
import foo.{ Foo1, Foo2 }

then organize-imports preserves the formatting of the second line, but not the first line (the spaces within the curly braces are removed):

import bar.{Bar => Baz}
import foo.{ Foo1, Foo2 }

I have only noticed this in lines where an import is renamed using the => syntax.

This is not a problem in my opinion for formatting code (we just run organize-imports first, and then scalafmt, which makes sense anyway) ; but it's an issue when checking already formatted code with the -check option, since it detects a difference.

I tested with the following configuration:

OrganizeImports {
  coalesceToWildcardImportThreshold = 50
  expandRelative = false
  groupExplicitlyImportedImplicitsSeparately = false
  groupedImports = Merge
  groups = ["re:javax?\\.", "scala.", "*", "my.app.package"]
  importSelectorsOrder = SymbolsFirst
  importsOrder = SymbolsFirst
  removeUnused = true
}
@liancheng liancheng added the bug Something isn't working label Nov 16, 2020
@liancheng
Copy link
Owner

Hey @guilgaly, thanks for reporting! I tried the following diff but failed to reproduce this issue:

diff --git a/input/src/main/scala/fix/FormatPreserving.scala b/input/src/main/scala/fix/FormatPreserving.scala
new file mode 100644
index 0000000..fe8de14
--- /dev/null
+++ b/input/src/main/scala/fix/FormatPreserving.scala
@@ -0,0 +1,19 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports {
+  coalesceToWildcardImportThreshold = 50
+  expandRelative = false
+  groupExplicitlyImportedImplicitsSeparately = false
+  groupedImports = Merge
+  groups = ["re:javax?\\.", "scala.", "*", "my.app.package"]
+  importSelectorsOrder = SymbolsFirst
+  importsOrder = SymbolsFirst
+  removeUnused = true
+}
+ */
+
+package fix
+
+import java.lang.{ Long => JLong }
+
+object FormatPreserving
diff --git a/output/src/main/scala/fix/FormatPreserving.scala b/output/src/main/scala/fix/FormatPreserving.scala
new file mode 100644
index 0000000..687b140
--- /dev/null
+++ b/output/src/main/scala/fix/FormatPreserving.scala
@@ -0,0 +1,5 @@
+package fix
+
+import java.lang.{ Long => JLong }
+
+object FormatPreserving

If your project is open-source, could you please let me know where the repo is? If not, could you please help to provide a repro?

@guilgaly
Copy link
Author

Sadly it's not an open source project, and I'm not sure what might cause the differece, but I'm going to try to come up with a repro.

@guilgaly
Copy link
Author

I have minimized one example from my project here: https://github.com/guilgaly/repro-organize-imports

Running sbt scalafixAll shows the change on line 10 of the file MyController.scala when I test it.

@liancheng
Copy link
Owner

Thanks, @guilgaly! I reproduced this issue with the following diff:

diff --git a/input/src/main/scala/fix/FormatPreserving.scala b/input/src/main/scala/fix/FormatPreserving.scala
new file mode 100644
index 0000000..56f7fc1
--- /dev/null
+++ b/input/src/main/scala/fix/FormatPreserving.scala
@@ -0,0 +1,12 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports.groupedImports = Merge
+ */
+
+package fix
+
+import fix.MergeImports.FormatPreserving.g1.{ a, b }
+import fix.MergeImports.FormatPreserving.g2._
+import fix.MergeImports.FormatPreserving.g2.{ d => D }
+
+object FormatPreserving
diff --git a/output/src/main/scala/fix/FormatPreserving.scala b/output/src/main/scala/fix/FormatPreserving.scala
new file mode 100644
index 0000000..15f69fc
--- /dev/null
+++ b/output/src/main/scala/fix/FormatPreserving.scala
@@ -0,0 +1,7 @@
+package fix
+
+import fix.MergeImports.FormatPreserving.g1.{ a, b }
+import fix.MergeImports.FormatPreserving.g2._
+import fix.MergeImports.FormatPreserving.g2.{ d => D }
+
+object FormatPreserving
diff --git a/shared/src/main/scala/fix/MergeImports.scala b/shared/src/main/scala/fix/MergeImports.scala
index 5aad18b..16d26fa 100644
--- a/shared/src/main/scala/fix/MergeImports.scala
+++ b/shared/src/main/scala/fix/MergeImports.scala
@@ -45,4 +45,16 @@ object MergeImports {
     object b
     object c
   }
+
+  object FormatPreserving {
+    object g1 {
+      object a
+      object b
+    }
+
+    object g2 {
+      object c
+      object d
+    }
+  }
 }

Test failure output:

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

===========RuleSuite 1s
=> Obtained
===========


package fix

import fix.MergeImports.FormatPreserving.g1.{ a, b }
import fix.MergeImports.FormatPreserving.g2._
import fix.MergeImports.FormatPreserving.g2.{d => D}

object FormatPreserving


=======
=> Diff
=======
--- obtained
+++ expected
@@ -4,3 +4,3 @@
 import fix.MergeImports.FormatPreserving.g2._
-import fix.MergeImports.FormatPreserving.g2.{d => D}
+import fix.MergeImports.FormatPreserving.g2.{ d => D }

@liancheng
Copy link
Owner

@guilgaly, PR #128 and #129 should have fixed this issue. Please give the 0.4.3+11-29aba3eb-SNAPSHOT snapshot release a try and let me know whether it works for you.

@liancheng liancheng linked a pull request Nov 17, 2020 that will close this issue
@liancheng liancheng added this to the v0.4.4 milestone Nov 17, 2020
@liancheng
Copy link
Owner

@guilgaly, at least it works for your repro repo :)

@liancheng liancheng linked a pull request Nov 17, 2020 that will close this issue
@guilgaly
Copy link
Author

I can confirm that it also works on my real project. 👍

BTW, I had already tested this tool with a colleague, over the past few weeks, on another project (with different formatting rules, so it didn't show the same issue) and we've been super happy with it. 🙂

@liancheng
Copy link
Owner

Thanks for the feedback, I'm closing this one and will prepare for the 0.4.4 release to include this fix.

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
2 participants