Remove the intellijCompatible option (scalacenter#48)

README.adoc

@@ -765,119 +765,6 @@ import scala.concurrent.duration
 ----
 --
 
-=== `intellijCompatible`
-
-==== Description
-
-When set to `true`, try to be compatible with the IntelliJ IDEA Scala import optimizer when possible.
-
-In some edge cases, the IntelliJ IDEA Scala import optimizer may produce broken code. For example, it does not handle the explicitly imported implicits case properly (see the <<groups, `groups`>> option). In order to guarantee correctness, `OrganizeImports` may behave differently from IntelliJ in these cases.
-
-However, to some users, having `OrganizeImports` fighting against IntelliJ can be annoying during development. Considering that those edge cases are rarely seen, the `intellijCompatible` option provides a choice to sacrifice edge case correctness for IntelliJ compatibility.
-
-==== Value type
-
-Boolean
-
-==== Default value
-
-`false`
-
-==== Example
-
-IntelliJ compatible mode turned off::
-+
---
-In the following example, we have two explicitly imported implicit names:
-
-[source,scala]
-----
-import scala.language.postfixOps
-import scala.concurrent.ExecutionContext.Implicits.global
-----
-
-By default, when the IntelliJ compatible mode is turned off, to avoid the aforementioned correctness issue (see the <<groups, `groups`>> option), they are moved into the last order-preserving group together with all the relative imports, if any.
-
-Configuration:
-
-[source,hocon]
-----
-OrganizeImports {
-  groups = ["scala.", "*"]
-  intellijCompatible = false
-}
-----
-
-Before:
-
-[source,scala]
-----
-import scala.collection.mutable
-import mutable.ArrayBuffer
-import scala.collection.JavaConverters._
-import scala.language.postfixOps
-import org.apache.spark.SparkContext
-import scala.concurrent.ExecutionContext.Implicits.global
-----
-
-After:
-
-[source,scala]
-----
-import scala.collection.mutable
-import scala.collection.JavaConverters._
-
-import org.apache.spark.SparkContext
-
-import mutable.ArrayBuffer
-import scala.language.postfixOps
-import scala.concurrent.ExecutionContext.Implicits.global
-----
---
-
-IntelliJ compatible mode turned on::
-+
---
-After turning on the IntelliJ compatible mode, explicitly imported implicits names are grouped in the same way as other imports (only relative imports are moved to the last group).
-
-Configuration:
-
-[source,hocon]
-----
-OrganizeImports {
-  groups = ["scala.", "*"]
-  intellijCompatible = true
-}
-----
-
-Before:
-
-[source,scala]
-----
-import scala.collection.mutable
-import mutable.ArrayBuffer
-import scala.collection.JavaConverters._
-import scala.language.postfixOps
-import org.apache.spark.SparkContext
-import scala.concurrent.ExecutionContext.Implicits.global
-----
-
-After:
-
-[source,scala]
-----
-import scala.collection.mutable
-import scala.collection.JavaConverters._
-import scala.concurrent.ExecutionContext.Implicits.global
-import scala.language.postfixOps
-
-import org.apache.spark.SparkContext
-
-import mutable.ArrayBuffer
-----
---
-
-[[remove-unused]]
 === `removeUnused`
 
 ==== Description

input/src/main/scala/fix/ExplicitlyImportedImplicitsLanguageFeature.scala

@@ -0,0 +1,19 @@
+/*
+rules = [OrganizeImports]
+OrganizeImports.importsOrder = Ascii
+ */
+package fix
+
+import scala.concurrent.ExecutionContext
+import fix.Implicits.b._
+import scala.languageFeature.postfixOps
+import ExecutionContext.Implicits.global
+import scala.languageFeature.implicitConversions
+import fix.Implicits.a.{i, s}
+
+object ExplicitlyImportedImplicitsLanguageFeature {
+  def f1()(implicit i: Int) = ???
+  def f2()(implicit s: String) = ???
+  f1()
+  f2()
+}

output/src/main/scala/fix/ExplicitlyImportedImplicitsLanguageFeature.scala

@@ -0,0 +1,18 @@
+package fix
+
+import scala.concurrent.ExecutionContext
+import scala.languageFeature.implicitConversions
+import scala.languageFeature.postfixOps
+
+import fix.Implicits.b._
+
+import ExecutionContext.Implicits.global
+import fix.Implicits.a.i
+import fix.Implicits.a.s
+
+object ExplicitlyImportedImplicitsLanguageFeature {
+  def f1()(implicit i: Int) = ???
+  def f2()(implicit s: String) = ???
+  f1()
+  f2()
+}

rules/src/main/scala/fix/OrganizeImports.scala

@@ -146,25 +146,44 @@ class OrganizeImports(config: OrganizeImportsConfig) extends SemanticRule("Organ
   private def partitionImplicits(
     importers: Seq[Importer]
   )(implicit doc: SemanticDocument): (Seq[Importer], Seq[Importer]) = {
-    if (config.intellijCompatible) {
-      (Nil, importers)
-    } else {
-      val (implicits, implicitPositions) = importers.flatMap {
-        case importer @ Importer(_, importees) =>
-          importees
-            .filter(_.is[Importee.Name])
-            .filter(_.symbol.info.exists(_.isImplicit))
-            .map(i => importer.copy(importees = i :: Nil) -> i.pos)
-      }.unzip
-
-      val noImplicits = importers.flatMap {
-        filterImportees(_) { importee =>
-          !implicitPositions.contains(importee.pos)
-        }.toSeq
-      }
-
-      (implicits, noImplicits)
+    val (implicits, implicitPositions) = importers.flatMap {
+      case importer @ Importer(_, importees) =>
+        importees
+          .filter(_.is[Importee.Name])
+          .filter(_.symbol.info.exists(_.isImplicit))
+          // Explicitly imported `scala.languageFeature` implicits are special cased and treated as
+          // normal imports due to the following reasons:
+          //
+          // 1. The IntelliJ IDEA Scala import optimizer does not handle the explicitly imported
+          //    implicit names case (see [issue #30][1]). Moving `scala.languageFeature` to the last
+          //    order-preserving import group produces a different result from IntelliJ, which can
+          //    be annoying for users who use both IntelliJ and `OrganizeImports`.
+          //
+          //    [1]: https://github.com/liancheng/scalafix-organize-imports/issues/30
+          //
+          // 2. Importing `scala.languageFeature` values is almost the only commonly seen cases
+          //    where a Scala developer imports an implicit by name explicitly. Yet, there's
+          //    practically zero chance that an implicit of the same type can be imported to cause a
+          //    conflict, unless the user is intentionally torturing the Scala compiler. Not
+          //    treating them as implicits and group them as other normal imports minimizes the
+          //    chance of behaving differently from IntelliJ without.
+          //
+          // 3. The `scala.languageFeature` values are defined as `object`s in Scala 2.12 but
+          //    changed to implicit lazy vals in Scala 2.13. Treating them as implicits means that
+          //    `OrganizeImports` may produce different results when `OrganizeImports` users upgrade
+          //    their code base from Scala 2.12 to 2.13. This introduces an annoying and unnecessary
+          //    thing to be taken care of.
+          .filter(_.symbol.owner.normalized != "scala.languageFeature.")
+          .map(i => importer.copy(importees = i :: Nil) -> i.pos)
+    }.unzip
+
+    val noImplicits = importers.flatMap {
+      filterImportees(_) { importee =>
+        !implicitPositions.contains(importee.pos)
+      }.toSeq
     }
+
+    (implicits, noImplicits)
   }
 
   private def expandRelative(importer: Importer)(implicit doc: SemanticDocument): Importer = {
@@ -350,7 +369,7 @@ object OrganizeImports {
     List(symbols, lowerCases, upperCases) flatMap (_ sortBy (_.syntax))
   }
 
-  private def mergeImporters(importers: Seq[Importer]): Seq[Importer] = {
+  private def mergeImporters(importers: Seq[Importer]): Seq[Importer] =
     importers.groupBy(_.ref.syntax).values.toSeq.flatMap {
       case importer :: Nil =>
         // If this group has only one importer, returns it as is to preserve the original source
@@ -472,7 +491,6 @@ object OrganizeImports {
 
         importeesList filter (_.nonEmpty) map (Importer(ref, _))
     }
-  }
 
   private def explodeImportees(importers: Seq[Importer]): Seq[Importer] =
     importers.flatMap {

rules/src/main/scala/fix/OrganizeImportsConfig.scala

@@ -56,7 +56,6 @@ final case class OrganizeImportsConfig(
   ),
   importSelectorsOrder: ImportSelectorsOrder = ImportSelectorsOrder.Ascii,
   importsOrder: ImportsOrder = ImportsOrder.Ascii,
-  intellijCompatible: Boolean = false,
   removeUnused: Boolean = true
 )