Merge pull request scalacenter#1276 from bjaglin/abstract-import

ExplicitResultTypes path-dependent types import bugfix & tweaks
SethTisue · Nov 10, 2020 · 69c0369 · 69c0369
2 parents 199b1d5 + 51a14aa
commit 69c0369
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 5 deletions.
diff --git a/scalafix-rules/src/main/scala/scala/meta/internal/pc/ScalafixGlobal.scala b/scalafix-rules/src/main/scala/scala/meta/internal/pc/ScalafixGlobal.scala
@@ -238,7 +238,21 @@ class ScalafixGlobal(
             // "scala.Seq[T]" even when it's needed.
             loop(ThisType(sym.owner), name)
           } else if (sym.hasPackageFlag || sym.isPackageObjectOrClass) {
-            if (history.tryShortenName(name)) NoPrefix
+            val dotSyntaxFriendlyName = name.map { name0 =>
+              if (name0.symbol.isStatic) name0
+              else {
+                // Use the prefix rather than the real owner to maximize the
+                // chances of shortening the reference: when `name` is directly
+                // nested in a non-statically addressable type (class or trait),
+                // its original owner is that type (requiring a type projection
+                // to reference it) while the prefix is its concrete owner value
+                // (for which the dot syntax works).
+                // https://docs.scala-lang.org/tour/inner-classes.html
+                // https://danielwestheide.com/blog/the-neophytes-guide-to-scala-part-13-path-dependent-types/
+                ShortName(name0.symbol.cloneSymbol(sym))
+              }
+            }
+            if (history.tryShortenName(dotSyntaxFriendlyName)) NoPrefix
             else tpe
           } else {
             if (history.isSymbolInScope(sym, pre)) SingleType(NoPrefix, sym)
@@ -396,8 +410,19 @@ class ScalafixGlobal(
             Iterator(lookupSymbol(name), lookupSymbol(name.otherName))
           results.flatten.filter(_ != LookupNotFound).toList match {
             case Nil =>
-              missingImports(name) = short
-              true
+              // Missing imports must be addressable via the dot operator
+              // syntax (as type projection is not allowed in imports).
+              // https://lptk.github.io/programming/2019/09/13/type-projection.html
+              if (
+                sym.isStaticMember || // Java static
+                sym.owner.ownerChain.forall { s =>
+                  // ensure the symbol can be referenced in a static manner, without any instance
+                  s.isPackageClass || s.isPackageObjectClass || s.isModule
+                }
+              ) {
+                missingImports(name) = short
+                true
+              } else false
             case lookup =>
               lookup.forall(_.symbol.isKindaTheSameAs(sym))
           }

diff --git a/...ests/input/src/main/scala/test/explicitResultTypes/ExplicitResultTypesPathDependent.scala b/...ests/input/src/main/scala/test/explicitResultTypes/ExplicitResultTypesPathDependent.scala
@@ -1,8 +1,33 @@
 /*
 rules = ExplicitResultTypes
+ExplicitResultTypes.skipSimpleDefinitions = ["Lit"]
  */
 package test.explicitResultTypes
 
+// like https://github.com/tpolecat/doobie/blob/c2e044/modules/core/src/main/scala/doobie/free/Aliases.scala#L10
+trait Trait {
+  type T1 // like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/free/Aliases.scala#L14
+  object Nested {
+    type T2
+  }
+}
+
+class Clazz {
+  type T3
+}
+
+// like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/hi/package.scala#L25
+package object PackageObject extends Trait
+
+package pkg {
+  abstract class AbstractClazz {
+    trait T4
+  }
+  object Obj extends Clazz {
+    object NestedObj extends AbstractClazz
+  }
+}
+
 object ExplicitResultTypesPathDependent {
   class Path {
     class B { class C }
@@ -16,4 +41,17 @@ object ExplicitResultTypesPathDependent {
     def bar: Self
   }
   implicit def foo[T] = null.asInstanceOf[Foo[T]].bar
+
+  // like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/util/query.scala#L163
+  def t1: PackageObject.T1 = ???
+  val t1Ref = t1
+
+  def t2: PackageObject.Nested.T2 = ???
+  val t2Ref = t2
+
+  def t3: pkg.Obj.T3 = ???
+  val t3Ref = t3
+
+  def t4: pkg.Obj.NestedObj.T4 = ???
+  val t4Ref = t4
 }
diff --git a/scalafix-tests/input/src/main/scala/tests/ExplicitResultTypesImports.scala b/scalafix-tests/input/src/main/scala/tests/ExplicitResultTypesImports.scala
@@ -16,7 +16,6 @@ object ExplicitResultTypesImports {
 
   val timezone = null.asInstanceOf[java.util.TimeZone]
 
-  // TODO: Is this desirable behavior?
   val inner = null.asInstanceOf[scala.collection.Searching.SearchResult]
 
   final val javaEnum = java.util.Locale.Category.DISPLAY

diff --git a/...sts/output/src/main/scala/test/explicitResultTypes/ExplicitResultTypesPathDependent.scala b/...sts/output/src/main/scala/test/explicitResultTypes/ExplicitResultTypesPathDependent.scala
@@ -1,6 +1,32 @@
 
 package test.explicitResultTypes
 
+import test.explicitResultTypes.PackageObject.{ Nested, T1 }
+import test.explicitResultTypes.pkg.Obj
+// like https://github.com/tpolecat/doobie/blob/c2e044/modules/core/src/main/scala/doobie/free/Aliases.scala#L10
+trait Trait {
+  type T1 // like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/free/Aliases.scala#L14
+  object Nested {
+    type T2
+  }
+}
+
+class Clazz {
+  type T3
+}
+
+// like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/hi/package.scala#L25
+package object PackageObject extends Trait
+
+package pkg {
+  abstract class AbstractClazz {
+    trait T4
+  }
+  object Obj extends Clazz {
+    object NestedObj extends AbstractClazz
+  }
+}
+
 object ExplicitResultTypesPathDependent {
   class Path {
     class B { class C }
@@ -14,4 +40,17 @@ object ExplicitResultTypesPathDependent {
     def bar: Self
   }
   implicit def foo[T]: Foo[T]#Self = null.asInstanceOf[Foo[T]].bar
+
+  // like https://github.com/tpolecat/doobie/blob/c2e0445/modules/core/src/main/scala/doobie/util/query.scala#L163
+  def t1: PackageObject.T1 = ???
+  val t1Ref: T1 = t1
+
+  def t2: PackageObject.Nested.T2 = ???
+  val t2Ref: Nested.T2 = t2
+
+  def t3: pkg.Obj.T3 = ???
+  val t3Ref: Obj.T3 = t3
+
+  def t4: pkg.Obj.NestedObj.T4 = ???
+  val t4Ref: Obj.NestedObj.T4 = t4
 }
diff --git a/scalafix-tests/output/src/main/scala/tests/ExplicitResultTypesImports.scala b/scalafix-tests/output/src/main/scala/tests/ExplicitResultTypesImports.scala
@@ -18,7 +18,6 @@ object ExplicitResultTypesImports {
 
   val timezone: ju.TimeZone = null.asInstanceOf[java.util.TimeZone]
 
-  // TODO: Is this desirable behavior?
   val inner: Searching.SearchResult = null.asInstanceOf[scala.collection.Searching.SearchResult]
 
   final val javaEnum: Category = java.util.Locale.Category.DISPLAY