diff --git a/README.md b/README.md index 08bd241..c749696 100644 --- a/README.md +++ b/README.md @@ -3,34 +3,32 @@ This project contains scalafix rules I create when migrating [akka](https://github.com/akka/akka/) to dotty. -#### FinalObject -Remove redundant `final` modifier for objects: -```scala -final object Abc -``` - #### ConstructorProcedureSyntax Remove constructor procedure syntax: `def this(..) {..}` => `def this(..) = {..}` -This rule complement to the built-in [ProcedureSyntax](https://github.com/scalacenter/scalafix/blob/master/scalafix-rules/src/main/scala/scalafix/internal/rule/ProcedureSyntax.scala) rule - -#### ExplicitNonNullaryApply -Compare to [fix.scala213.ExplicitNonNullaryApply](https://github.com/scala/scala-rewrites/blob/1cea92d/rewrites/src/main/scala/fix/scala213/ExplicitNonNullaryApply.scala) - from scala-rewrites project: - - Don't need scala.meta.internal.pc.ScalafixGlobal to hook into scala-compiler. - - So this rule also add `()` to methods that is defined in java and be overridden in scala, eg the calls in [ExplicitNonNullaryApplyJavaPending](scalafix/input/src/main/scala/fix/scala213/ExplicitNonNullaryApplyJavaPending.scala) test. - - When I try scala-rewrites' ExplicitNonNullaryApply for akka sources, it crashed! - - Don't need to publishLocal before use. [scala/scala-rewrites#32](https://github.com/scala/scala-rewrites/issues/32) - - [Just add](https://github.com/ohze/akka/blob/dotty/wip/.scalafix.conf) `"github:ohze/scalafix-rules/ExplicitNonNullaryApply"` to `.scalafix.conf` +This rule complement to the built-in [ProcedureSyntax](https://github.com/scalacenter/scalafix/blob/master/scalafix-rules/src/main/scala/scalafix/internal/rule/ProcedureSyntax.scala) rule. #### Any2StringAdd - ~~Fix https://github.com/scala/scala-rewrites/issues/18~~ (now fixed) - Plus some improvements +#### ParensAroundLambda +Fix: parentheses are required around the parameter of a lambda +```scala +Seq(1).map { i: Int => // rewrite to: Seq(1).map { (i: Int) => + i + 1 +} +Seq(1).map { i => i + 1 } // keep +``` + +#### FinalObject +Remove redundant `final` modifier for objects: +```scala +final object Abc +``` + #### NullaryOverride -Consistent nullary overriding, eg: +Consistent nullary overriding ```scala trait A { def i: Int @@ -42,40 +40,72 @@ trait B { } ``` -#### ParensAroundLambda -Fix: parentheses are required around the parameter of a lambda -```scala -Seq(1).map { i: Int => // rewrite to: Seq(1).map { (i: Int) => - i + 1 -} -Seq(1).map { i => i + 1 } // keep -``` +#### ExplicitNonNullaryApply +Compare to [fix.scala213.ExplicitNonNullaryApply](https://github.com/scala/scala-rewrites/blob/1cea92d/rewrites/src/main/scala/fix/scala213/ExplicitNonNullaryApply.scala) + from scala-rewrites project: ++ Pros + - When I try scala-rewrites' ExplicitNonNullaryApply for akka sources, it crashed! + - This rule run faster than scala-rewrites' one. + - Don't need to publishLocal to use. [scala/scala-rewrites#32](https://github.com/scala/scala-rewrites/issues/32) + + [Just add](https://github.com/ohze/akka/blob/dotty/wip/.scalafix.conf) `"github:ohze/scalafix-rules/ExplicitNonNullaryApply"` to `.scalafix.conf` ++ Cons + - This rule also add `()` to methods that are defined in java and be overridden in scala, + eg the calls in [ExplicitNonNullaryApplyJavaPending](scalafix/input/src/main/scala/fix/scala213/ExplicitNonNullaryApplyJavaPending.scala) test. + Those `()`s are not required by dotty. ++ Technical note: This rule don't need `scala.meta.internal.pc.ScalafixGlobal` to hook into scala-compiler. #### ExplicitImplicitTypes -To explicitly add type to `implicit def/val/var`s (required by dotty) you need: -1. Use the built-in ExplicitResultTypes rule with config: +Before scalacenter/scalafix#1180 is fixed, to explicitly add type to `implicit def/val/var`s (required by dotty) you need: ++ Use the built-in [ExplicitResultTypes](https://scalacenter.github.io/scalafix/docs/rules/ExplicitResultTypes.html) rule with config: ```hocon rules = [ ExplicitResultTypes ] ExplicitResultTypes { memberVisibility = [] # only rewrite implicit members - skipSimpleDefinitions = [] } ``` => Add type to implicit members of `class`es, `trait`s +```scala +trait T { + // rewrite to `implicit val s: Seq[String] = Nil` + implicit val s = Seq.empty[String] + // rewrite to `implicit def i: Int = 1` + implicit def i = 1 +} +``` -2. And this rule ++ *And* use this rule ```hocon rules = [ "github:ohze/scalafix-rules/ExplicitImplicitTypes" ] -# maybe need +# Optinal ExplicitImplicitTypes.symbolReplacements { "scala/concurrent/ExecutionContextExecutor#" = "scala/concurrent/ExecutionContext#" } ``` -=> Add type to implicit local `def/val/var`s +=> Add type to implicit local `def/val/var`s that its parent is a trait/class +```scala +import scala.concurrent.ExecutionContextExecutor + +trait T +trait A { + def f() = { + class C { + // rewrite to `implicit val i: Int = 1` + implicit val i = 1 + } + ??? + } + def someEc(): ExecutionContextExecutor + def g() = new T { + // rewrite to `implicit def ec: ExecutionContext = someEc()` + implicit def ec = someEc() + } +} +``` ## Usage + Eg, for ExplicitNonNullaryApply rule: diff --git a/scalafix/input/src/main/scala/fix/ExplicitImplicitTypesTest.scala b/scalafix/input/src/main/scala/fix/ExplicitImplicitTypesTest.scala index 7bcbf3a..d76178b 100644 --- a/scalafix/input/src/main/scala/fix/ExplicitImplicitTypesTest.scala +++ b/scalafix/input/src/main/scala/fix/ExplicitImplicitTypesTest.scala @@ -13,7 +13,7 @@ abstract class ExplicitImplicitTypesTest { def ec: ExecutionContextExecutor } trait T - + def f(e: E) = new T { private implicit def ec = e.ec final implicit val s1 = Seq(1) diff --git a/scalafix/input/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala b/scalafix/input/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala new file mode 100644 index 0000000..52b53d6 --- /dev/null +++ b/scalafix/input/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala @@ -0,0 +1,19 @@ +/* +rule = ExplicitImplicitTypes +*/ +package test.explicitResultTypes + +class SkipLocalImplicits { + trait T + def f(): T = new T { + implicit val i = 1 + } + def g(): Unit = { + class C { + implicit val i = 2 + } + } + def h(): Unit = { + implicit val i = 3 + } +} diff --git a/scalafix/output/src/main/scala/fix/ExplicitImplicitTypesTest.scala b/scalafix/output/src/main/scala/fix/ExplicitImplicitTypesTest.scala index 9d9b2f6..2e281cc 100644 --- a/scalafix/output/src/main/scala/fix/ExplicitImplicitTypesTest.scala +++ b/scalafix/output/src/main/scala/fix/ExplicitImplicitTypesTest.scala @@ -8,15 +8,15 @@ abstract class ExplicitImplicitTypesTest { def ec: ExecutionContextExecutor } trait T - + def f(e: E) = new T { private implicit def ec: ExecutionContext = e.ec final implicit val s1: Seq[Int] = Seq(1) implicit var nil_ : Seq[AnJavaInterface] = Nil implicit var i: Int = 10 // keep def g() = { - implicit val l: Long = 1L - implicit var none: Option[String] = None + implicit val l = 1L + implicit var none = Option.empty[String] } } } diff --git a/scalafix/output/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala b/scalafix/output/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala new file mode 100644 index 0000000..4eb7be1 --- /dev/null +++ b/scalafix/output/src/main/scala/test/explicitResultTypes/SkipLocalImplicits.scala @@ -0,0 +1,16 @@ +package test.explicitResultTypes + +class SkipLocalImplicits { + trait T + def f(): T = new T { + implicit val i: Int = 1 + } + def g(): Unit = { + class C { + implicit val i: Int = 2 + } + } + def h(): Unit = { + implicit val i = 3 + } +} diff --git a/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypes.scala b/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypes.scala index c593603..491ac06 100644 --- a/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypes.scala +++ b/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypes.scala @@ -57,19 +57,52 @@ final class ExplicitImplicitTypes(global: LazyValue[ScalafixGlobal]) extends Sem lazy val types = new CompilerTypePrinter(global.value) doc.tree.collect { case t @ Defn.Val(mods, Pat.Var(name) :: Nil, None, body) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => // t.hasMod(mod"implicit") + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) case t @ Defn.Var(mods, Pat.Var(name) :: Nil, None, Some(body)) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) case t @ Defn.Def(mods, name, _, _, None, body) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) }.asPatch } + // Don't explicitly annotate vals when the right-hand body is a single call + // to `implicitly`. Prevents ambiguous implicit. Not annotating in such cases, + // this a common trick employed implicit-heavy code to workaround SI-2712. + // Context: https://gitter.im/typelevel/cats?at=584573151eb3d648695b4a50 + private def isImplicitly(term: Term): Boolean = term match { + case Term.ApplyType(Term.Name("implicitly"), _) => true + case _ => false + } + + def isRuleCandidate( + defn: Defn, + nm: Name, + mods: Iterable[Mod], + body: Term + )(implicit ctx: SemanticDocument): Boolean = { + + def isFinalLiteralVal: Boolean = + defn.is[Defn.Val] && + mods.exists(_.is[Mod.Final]) && + body.is[Lit] + + def isImplicit: Boolean = + mods.exists(_.is[Mod.Implicit]) && !isImplicitly(body) + + def hasParentWihTemplate: Boolean = + defn.parent.exists(_.is[Template]) + + isImplicit && + !isFinalLiteralVal && + nm.symbol.isLocal && // use ExplicitResultTypes for non-local implicits + hasParentWihTemplate // use this Rule for local implicits in Template only + } + def fixDefinition(defn: Defn, body: Term, name: Term.Name, types: TypePrinter)( implicit ctx: SemanticDocument ): Patch = { @@ -135,11 +168,14 @@ class CompilerTypePrinter(g: ScalafixGlobal)( defn: m.Defn, space: String ): Option[v1.Patch] = { - if (sym.isGlobal) return None // pls use rule ExplicitResultTypes - val gpos = unit.position(pos.start) - val t = GlobalProxy.typedTreeAt(g, gpos) - val inverseSemanticdbSymbol = t.symbol + val tpe = GlobalProxy.typedTreeAt(g, gpos) + val inverseSemanticdbSymbol = + if (sym.isLocal) tpe.symbol + else + g.inverseSemanticdbSymbols(sym.value) + .find(s => g.semanticdbSymbol(s) == sym.value) + .getOrElse(g.NoSymbol) val hasNothing = inverseSemanticdbSymbol.info.exists { case g.definitions.NothingTpe => true case _ => false @@ -307,6 +343,7 @@ class CompilerTypePrinter(g: ScalafixGlobal)( private def isPossibleSyntheticParent(tpe: Type): Boolean = { definitions.isPossibleSyntheticParent(tpe.typeSymbol) || + CompilerCompat.serializableClass(g).toSet[Symbol](tpe.typeSymbol) || definitions.AnyRefTpe == tpe || definitions.ObjectTpe == tpe } @@ -329,9 +366,19 @@ class CompilerTypePrinter(g: ScalafixGlobal)( case t: MethodType => loop(t.resultType) case RefinedType(parents, _) => // Remove redundant `Product with Serializable`, if possible. - val strippedParents = parents.filterNot { tpe => - definitions.isPossibleSyntheticParent(tpe.typeSymbol) - } + val productRootClass = definitions.ProductClass.seq + .toSet[Symbol] + definitions.ProductRootClass + val serializableClass = + CompilerCompat.serializableClass(g).toSet[Symbol] + val parentSymbols = parents.map(_.typeSymbol).toSet + val strippedParents = + if (parentSymbols + .intersect(productRootClass) + .nonEmpty && parentSymbols + .intersect(serializableClass) + .nonEmpty) { + parents.filterNot(isPossibleSyntheticParent) + } else parents val newParents = if (strippedParents.nonEmpty) strippedParents else parents @@ -395,6 +442,12 @@ class CompilerTypePrinter(g: ScalafixGlobal)( g.rootMirror.RootPackage ) } + +object CompilerCompat { + // support scala 2.13 only + def serializableClass(g: ScalafixGlobal): Set[g.ClassSymbol] = + Set(g.definitions.SerializableClass) +} // end inlining fix/impl/CompilerTypePrinter.scala // begin inlining fix/impl/PatchEmptyBody.scala diff --git a/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypesImpl.scala b/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypesImpl.scala index 02f4ac4..ed4e896 100644 --- a/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypesImpl.scala +++ b/scalafix/rules/src/main/scala/fix/ExplicitImplicitTypesImpl.scala @@ -57,19 +57,52 @@ final class ExplicitImplicitTypesImpl(global: LazyValue[ScalafixGlobal]) extends lazy val types = new CompilerTypePrinter(global.value) doc.tree.collect { case t @ Defn.Val(mods, Pat.Var(name) :: Nil, None, body) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => // t.hasMod(mod"implicit") + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) case t @ Defn.Var(mods, Pat.Var(name) :: Nil, None, Some(body)) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) case t @ Defn.Def(mods, name, _, _, None, body) - if mods.exists(_.isInstanceOf[Mod.Implicit]) => + if isRuleCandidate(t, name, mods, body) => fixDefinition(t, body, name, types) }.asPatch } + // Don't explicitly annotate vals when the right-hand body is a single call + // to `implicitly`. Prevents ambiguous implicit. Not annotating in such cases, + // this a common trick employed implicit-heavy code to workaround SI-2712. + // Context: https://gitter.im/typelevel/cats?at=584573151eb3d648695b4a50 + private def isImplicitly(term: Term): Boolean = term match { + case Term.ApplyType(Term.Name("implicitly"), _) => true + case _ => false + } + + def isRuleCandidate( + defn: Defn, + nm: Name, + mods: Iterable[Mod], + body: Term + )(implicit ctx: SemanticDocument): Boolean = { + + def isFinalLiteralVal: Boolean = + defn.is[Defn.Val] && + mods.exists(_.is[Mod.Final]) && + body.is[Lit] + + def isImplicit: Boolean = + mods.exists(_.is[Mod.Implicit]) && !isImplicitly(body) + + def hasParentWihTemplate: Boolean = + defn.parent.exists(_.is[Template]) + + isImplicit && + !isFinalLiteralVal && + nm.symbol.isLocal && // use ExplicitResultTypes for non-local implicits + hasParentWihTemplate // use this Rule for local implicits in Template only + } + def fixDefinition(defn: Defn, body: Term, name: Term.Name, types: TypePrinter)( implicit ctx: SemanticDocument ): Patch = { diff --git a/scalafix/rules/src/main/scala/fix/impl/CompilerTypePrinter.scala b/scalafix/rules/src/main/scala/fix/impl/CompilerTypePrinter.scala index ce73b55..edba4b8 100644 --- a/scalafix/rules/src/main/scala/fix/impl/CompilerTypePrinter.scala +++ b/scalafix/rules/src/main/scala/fix/impl/CompilerTypePrinter.scala @@ -40,11 +40,14 @@ class CompilerTypePrinter(g: ScalafixGlobal)( defn: m.Defn, space: String ): Option[v1.Patch] = { - if (sym.isGlobal) return None // pls use rule ExplicitResultTypes - val gpos = unit.position(pos.start) - val t = GlobalProxy.typedTreeAt(g, gpos) - val inverseSemanticdbSymbol = t.symbol + val tpe = GlobalProxy.typedTreeAt(g, gpos) + val inverseSemanticdbSymbol = + if (sym.isLocal) tpe.symbol + else + g.inverseSemanticdbSymbols(sym.value) + .find(s => g.semanticdbSymbol(s) == sym.value) + .getOrElse(g.NoSymbol) val hasNothing = inverseSemanticdbSymbol.info.exists { case g.definitions.NothingTpe => true case _ => false @@ -212,6 +215,7 @@ class CompilerTypePrinter(g: ScalafixGlobal)( private def isPossibleSyntheticParent(tpe: Type): Boolean = { definitions.isPossibleSyntheticParent(tpe.typeSymbol) || + CompilerCompat.serializableClass(g).toSet[Symbol](tpe.typeSymbol) || definitions.AnyRefTpe == tpe || definitions.ObjectTpe == tpe } @@ -234,9 +238,19 @@ class CompilerTypePrinter(g: ScalafixGlobal)( case t: MethodType => loop(t.resultType) case RefinedType(parents, _) => // Remove redundant `Product with Serializable`, if possible. - val strippedParents = parents.filterNot { tpe => - definitions.isPossibleSyntheticParent(tpe.typeSymbol) - } + val productRootClass = definitions.ProductClass.seq + .toSet[Symbol] + definitions.ProductRootClass + val serializableClass = + CompilerCompat.serializableClass(g).toSet[Symbol] + val parentSymbols = parents.map(_.typeSymbol).toSet + val strippedParents = + if (parentSymbols + .intersect(productRootClass) + .nonEmpty && parentSymbols + .intersect(serializableClass) + .nonEmpty) { + parents.filterNot(isPossibleSyntheticParent) + } else parents val newParents = if (strippedParents.nonEmpty) strippedParents else parents @@ -300,3 +314,9 @@ class CompilerTypePrinter(g: ScalafixGlobal)( g.rootMirror.RootPackage ) } + +object CompilerCompat { + // support scala 2.13 only + def serializableClass(g: ScalafixGlobal): Set[g.ClassSymbol] = + Set(g.definitions.SerializableClass) +}