liancheng / scalafix-organize-imports

A CI-friendly Scalafix semantic rule for organizing imports
MIT License
192 stars 22 forks source link

Add ability to specify a "root" package #312

Open mdedetrich opened 1 year ago

mdedetrich commented 1 year ago

Some projects that happen to have a very long fully qualified project name and they use nested imports to shorten boilerplate. A good example of this is pekko, where we have a very long base package name (i.e. org.apache.pekko) and in order to avoid having to repeat org.apache.pekko multiple times (both in imports, scaladoc documentation and when we have to explicitly refer to a type).

https://github.com/apache/incubator-pekko/blob/1612950702d4dc8107f336442c7617cc67e22eda/remote/src/main/scala/org/apache/pekko/remote/FailureDetectorRegistry.scala is a good example demonstrating this, we first have a "root" import

import org.apache.pekko

this then follows a list of import nested under pekko, i.e.

import pekko.ConfigurationException
import pekko.actor.{ ActorContext, ActorSystem, ExtendedActorSystem }
import pekko.event.EventStream

and also in that file there is documentation that refers to pekko

  /**
   * Loads and instantiates a given [[FailureDetector]] implementation. The class to be loaded must have a constructor
   * that accepts a [[com.typesafe.config.Config]] and an [[pekko.event.EventStream]] parameter. Will throw ConfigurationException
   * if the implementation cannot be loaded.
   *
   * @param fqcn Fully qualified class name of the implementation to be loaded.
   * @param config Configuration that will be passed to the implementation
   * @param system ActorSystem to be used for loading the implementation
   * @return A configured instance of the given [[FailureDetector]] implementation
   */

And here is an example of using the root package when just referring to its type directly https://github.com/apache/incubator-pekko/blob/2df2e0294f763a8f7c41d31830b3f28059e0bf01/remote/src/main/scala/org/apache/pekko/remote/serialization/ProtobufSerializer.scala#L61

One important to thing to note is that there is an exception case, i.e. if you only ever have one single reference to the entire the root package. https://github.com/apache/incubator-pekko/blob/4ac0f00a477873965ee7d52e16faefb1de91fe3a/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/PropsSpec.scala#L19 is a good example of this, i.e. because there is only ever one reference to a type/class within org.apache.pekko we do

import org.apache.pekko.actor.testkit.typed.scaladsl.LogCapturing

rather than

import org.apache.pekko
import pekko.actor.testkit.typed.scaladsl.LogCapturing

Since in only this specific case its worse (i.e. more boilerplate). https://github.com/apache/incubator-pekko/blob/480f5163984999563ba769d92cb97a2af6fe61ef/bench-jmh/src/main/scala/org/apache/pekko/BenchRunner.scala#L22 is another example with an import thats not at the top of the file.

The are also other interest corner cases, i.e. lets assume that we have 2 root packages i.e.

root-packages = [
  "org.apache.pekko",
  "org.apache.kafka"
]

And ontop of that there happens to be a package named kafka under pekko, i.e. org.apache.pekko.kafka. The correct way to handle this is that if we are referring to kafka that that should be under org.apache.kafka this should just be kafka however we are referring to kafka under pekko than that should be pekko.kafka.

There is an upstream scalafix issue about the concept of a "root" package at https://github.com/scalacenter/scalafix/issues/1792

liancheng commented 1 year ago

Hey @mdedetrich, thanks for the feature request. OrganizeImports is now a Scalafix built-in rule since Scalafix v0.11.0, and I'm archiving this repository. Could you please create an issue in the https://github.com/scalacenter/scalafix repo for this one?