Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline pprint to avoid binary incompatibilities #154

Merged
merged 1 commit into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,33 @@ lazy val sharedSettings = List[Setting[_]](
skip.in(publish) := true
disablePlugins(MimaPlugin)

lazy val pprint = crossProject(JVMPlatform, JSPlatform, NativePlatform)
.in(file("metaconfig-pprint"))
.settings(
sharedSettings,
moduleName := "metaconfig-pprint",
libraryDependencies += "com.lihaoyi" %%% "fansi" % "0.3.0",
libraryDependencies ++= {
if (scalaVersion.value.startsWith("2."))
List(
"org.scala-lang" % "scala-reflect" % scalaVersion.value,
"org.scala-lang" % "scala-compiler" % scalaVersion.value
)
else Nil
}
)
.nativeSettings(
crossScalaVersions -= scala3
)

lazy val core = crossProject(JVMPlatform, JSPlatform, NativePlatform)
.in(file("metaconfig-core"))
.settings(
sharedSettings,
moduleName := "metaconfig-core",
libraryDependencies ++= List(
"org.typelevel" %%% "paiges-core" % "0.4.2",
"org.scala-lang.modules" %%% "scala-collection-compat" % "2.5.0",
"com.lihaoyi" %%% "pprint" % "0.7.1"
Copy link
Member

@bjaglin bjaglin Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To address scalacenter/scalafix#1522 (comment), a version prior to com-lihaoyi/PPrint#72 should remain pinned in the classpath so that code compiled against old versions of metaconfig (<=0.9.15) can still run with upcoming versions (as a macro generates code that relies on it). If we do that code compiled against 0.9.16 will start breaking but considering it's very recent, I'd say it's acceptable if we tag a 0.9.17 soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the example with https://github.com/NeQuissimus/sort-imports/blob/4e85753a52756106f5e21c2a23b721413f2571c3/rules/src/main/scala/fix/SortImports.scala#L19-L21 to illustrate:

$ javap -p -c -cp /tmp/sort-imports_2.13-0.5.4.jar fix.SortImportsConfig\$ | grep pprint
      42: getstatic     #66                 // Field pprint/TPrint$.MODULE$:Lpprint/TPrint$;
      50: invokevirtual #91                 // Method pprint/TPrint$.lambda:(Lscala/Function1;)Lpprint/TPrint;
      53: getstatic     #94                 // Field pprint/TPrintColors$BlackWhite$.MODULE$:Lpprint/TPrintColors$BlackWhite$;
      56: invokeinterface #99,  2           // InterfaceMethod pprint/TPrint.render:(Lpprint/TPrintColors;)Ljava/lang/String;
      97: getstatic     #66                 // Field pprint/TPrint$.MODULE$:Lpprint/TPrint$;
     105: invokevirtual #91                 // Method pprint/TPrint$.lambda:(Lscala/Function1;)Lpprint/TPrint;
     108: getstatic     #94                 // Field pprint/TPrintColors$BlackWhite$.MODULE$:Lpprint/TPrintColors$BlackWhite$;
     111: invokeinterface #99,  2           // InterfaceMethod pprint/TPrint.render:(Lpprint/TPrintColors;)Ljava/lang/String;
  public static final java.lang.String $anonfun$surface$2(pprint.TPrintColors);
      10: invokevirtual #195                // Method pprint/TPrintColors.typeColor:()Lfansi/Attrs;
  public static final java.lang.String $anonfun$surface$1(pprint.TPrintColors);
      10: invokevirtual #195                // Method pprint/TPrintColors.typeColor:()Lfansi/Attrs;
      37: getstatic     #66                 // Field pprint/TPrint$.MODULE$:Lpprint/TPrint$;
      40: getstatic     #66                 // Field pprint/TPrint$.MODULE$:Lpprint/TPrint$;
      48: invokevirtual #91                 // Method pprint/TPrint$.lambda:(Lscala/Function1;)Lpprint/TPrint;
      51: invokevirtual #238                // Method pprint/TPrint$.implicitly:(Lpprint/TPrint;)Lpprint/TPrint;
      55: invokeinterface #99,  2           // InterfaceMethod pprint/TPrint.render:(Lpprint/TPrintColors;)Ljava/lang/String;
  public static final java.lang.String $anonfun$surface$3(pprint.TPrintColors);
      10: invokevirtual #195                // Method pprint/TPrintColors.typeColor:()Lfansi/Attrs;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To address scalacenter/scalafix#1522 (comment), a version prior to com-lihaoyi/PPrint#72 should remain pinned in the classpath

I am afraid there is no actual way to do that.

I inlined pprint as metaconfig.pprint to avoid any issues in the future of conflicting versions, not sure this will help in this situation then, but it's something we need to do. We can't inline pprint as just pprint since that doesn't solve the issue when pprint would be on the classpath.

Is there any way actually to migrate to newer/inlined pprint and at the same allowing scalafix compiled rules to still work?
Or keeping a separate 0.9.x branch is the only way? I don't think we would be able to keep a separate branch really :/

Will it work if older pprint is kept on the classpath of scalafix? Since the generated macros would use that and the metaconfig.print classes would not clash? To be honest I am a bit lost and I am afraid that there is nothing we can do aside from recompiling all those rules.

Since, there is not that much happening on metaconfig side I would say it's safe to just keep at the current version of metaconfig until we bump scalafix version to an incompatible one. What do you think? I am totally open to suggestions, the pprint incompatibility cause a whole lot of unexpected problems 😓

Copy link
Contributor Author

@tgodzik tgodzik Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming to think of it maybe Since the generated macros would use that and the metaconfig.print classes would not clash? would work? Since there is no actual clash between the current metaconfig pprint and the old one? Are we able to test that out with the current snapshot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming to think of it maybe Since the generated macros would use that and the metaconfig.print classes would not clash? would work? Since there is no actual clash between the current metaconfig pprint and the old one?

That's my understanding/suggestion yes - leaving the real pprint in the dependencies of metaconfig for a while (even if effectively not used at compile time), just to satisfy linking when used with client code compiled in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we instead add the old pprint as a dependency to scalafix? Or would be better to keep it in metaconfig?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we instead add the old pprint as a dependency to scalafix? Or would be better to keep it in metaconfig?

That would work indeed 👍 If scalafix is the only user of metaconfig really affected by the breaking change introduced in metaconfig 0.9.16, I am happy to do that!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be only a scalafix problem as far I know, since metaconfig is transitive dependency for those rules. I will do a next release though 0.10.0 just in case since effecitively we change the binary compatibility.

Copy link
Member

@bjaglin bjaglin Jan 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do a next release though 0.10.0 just in case since effecitively we change the binary compatibility.

👍 for a minor bump over a patch one. 0.9.16 was breaking too though, but I guess tagging a 0.9.17 is a hassle. In any case, fine for me, thanks for looking into this!

"org.scala-lang.modules" %%% "scala-collection-compat" % "2.5.0"
)
)
.settings(
Expand All @@ -115,6 +133,7 @@ lazy val core = crossProject(JVMPlatform, JSPlatform, NativePlatform)
.nativeSettings(
crossScalaVersions -= scala3
)
.dependsOn(pprint)

lazy val coreJVM = core.jvm
lazy val coreJS = core.js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class Macros(val c: blackbox.Context) {
}
val tprint = c.internal.typeRef(
NoPrefix,
weakTypeOf[pprint.TPrint[_]].typeSymbol,
weakTypeOf[metaconfig.pprint.TPrint[_]].typeSymbol,
paramTpe :: Nil
)
val tpeString = c.inferImplicitValue(tprint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ private[generic] def deriveSurfaceImpl[T: Type](using q: Quotes) =
val tpeString =
fieldType.asType match
case '[t] =>
val renderer = Expr.summon[pprint.TPrint[t]].get
val renderer = Expr.summon[metaconfig.pprint.TPrint[t]].get
'{ $renderer.render.render }

val fieldExpr = '{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,299 @@
/* MIT License
Copyright (c) 2019 Li Haoyi
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE. */

package metaconfig.pprint

import language.experimental.macros
import reflect.macros.blackbox.Context

trait TPrintLowPri {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only inlined TPrint part, but maybe we should inline it all? This way we could drop the dependency on pprint in mdoc too and this has been causing issues.

implicit def default[T]: TPrint[T] = macro TPrintLowPri.typePrintImpl[T]
}
object TPrintLowPri {
sealed trait WrapType
object WrapType {
case object NoWrap extends WrapType
case object Infix extends WrapType
case object Tuple extends WrapType
}
def typePrintImpl[T: c.WeakTypeTag](c: Context): c.Expr[TPrint[T]] = {
// Used to provide "empty string" values in quasiquotes

import c.universe._
val tpe = weakTypeOf[T]
val rendered = typePrintImplRec(c)(tpe, rightMost = true).render
val res = c.Expr[TPrint[T]](
q"_root_.metaconfig.pprint.TPrint.recolor(_root_.fansi.Str($rendered))"
)
res
}

val functionTypes: Set[String] =
Range.inclusive(0, 22).map(i => s"scala.Function$i").toSet
val tupleTypes: Set[String] =
Range.inclusive(0, 22).map(i => s"scala.Tuple$i").toSet

def typePrintImplRec[T](
c: Context
)(tpe: c.Type, rightMost: Boolean): fansi.Str = {
typePrintImplRec0(c)(tpe, rightMost)._1
}
def typePrintImplRec0[T](
c: Context
)(tpe: c.Type, rightMost: Boolean): (fansi.Str, WrapType) = {
import c.universe._
def printSymString(s: Symbol) =
if (s.name.decodedName.toString.startsWith("_$")) "_"
else s.name.decodedName.toString.stripSuffix(".type")

def literalColor(s: fansi.Str): fansi.Str = fansi.Color.Green(s)
def printSym(s: Symbol): fansi.Str = literalColor(printSymString(s))

def printSymFull(s: Symbol): fansi.Str = {
if (lookup(s)) printSym(s)
else printSymFull(s.owner) ++ "." ++ printSym(s)
}

/**
* Looks up a symbol in the enclosing scope and returns
* whether it exists in scope by the same name
*/
def lookup(s: Symbol) = {
val cas = c.asInstanceOf[reflect.macros.runtime.Context]
val g = cas.global
val gName = s.name.asInstanceOf[g.Name]
val lookedUps =
for (n <- Stream(gName.toTermName, gName.toTypeName)) yield {
cas.callsiteTyper.context
.lookupSymbol(n, _ => true)
.symbol
}

if (!s.isType) {
// Try to de-reference `val` references
lookedUps.exists(x => x == s || x.tpe.termSymbol == s.asTerm)
} else {
// Try to resolve aliases for types
lookedUps.exists(x =>
x == s || x.tpe.typeSymbol == s.asInstanceOf[g.Symbol].tpe.typeSymbol
)
}
}

def prefixFor(pre: Type, sym: Symbol): fansi.Str = {
// Depending on what the prefix is, you may use `#`, `.`
// or even need to wrap the prefix in parentheses
val sep = pre match {
case x if x.toString.endsWith(".type") =>
typePrintImplRec(c)(pre, false) ++ "."
case x: TypeRef => literalColor(typePrintImplRec(c)(pre, true)) ++ "#"
case x: SingleType =>
literalColor(typePrintImplRec(c)(pre, false)) ++ "."
case x: ThisType => literalColor(typePrintImplRec(c)(pre, false)) ++ "."
case x => fansi.Str("(") ++ typePrintImplRec(c)(pre, true) ++ ")#"
}

val prefix = if (!lookup(sym)) sep else fansi.Str("")
prefix ++ printSym(sym)
}

def printArgSyms(args: List[Symbol]): fansi.Str = {
def added =
args
.map { x =>
val TypeBounds(lo, hi) = x.info
printSym(x) ++ printBounds(lo, hi)
}
.reduceLeft[fansi.Str]((l, r) => l ++ ", " ++ r)

if (args == Nil) fansi.Str("") else fansi.Str("[") ++ added ++ "]"
}
def printArgs(args: List[Type]): fansi.Str = {
def added =
args
.map(typePrintImplRec(c)(_, true))
.reduceLeft[fansi.Str]((l, r) => l ++ ", " ++ r)

if (args == Nil) fansi.Str("") else fansi.Str("[") ++ added ++ "]"
}

def printBounds(lo: Type, hi: Type) = {
val loTree =
if (lo =:= typeOf[Nothing]) fansi.Str("")
else fansi.Str(" >: ") ++ typePrintImplRec(c)(lo, true)
val hiTree =
if (hi =:= typeOf[Any]) fansi.Str("")
else fansi.Str(" <: ") ++ typePrintImplRec(c)(hi, true)
loTree ++ hiTree
}

def showRefinement(quantified: List[Symbol]) = {
def stmts =
for {
t <- quantified
suffix <- t.info match {
case PolyType(typeParams, resultType) =>
val paramTree = printArgSyms(
t.asInstanceOf[TypeSymbol].typeParams
)
val resultBounds =
if (resultType =:= typeOf[Any]) fansi.Str("")
else fansi.Str(" <: ") ++ typePrintImplRec(c)(resultType, true)

Some(paramTree ++ resultBounds)
case TypeBounds(lo, hi)
if t.toString.contains("$") && lo =:= typeOf[Nothing] && hi =:= typeOf[
Any
] =>
None
case TypeBounds(lo, hi) =>
Some(printBounds(lo, hi))
}
} yield {
if (t.toString.endsWith(".type")) {
val TypeBounds(lo, hi) = t.info
val RefinedType(parents, defs) = hi
val filtered = internal.refinedType(
parents.filter(x => !(x =:= typeOf[scala.Singleton])),
defs
)

fansi.Str("val ") ++ literalColor(
t.name.toString.stripSuffix(".type")
) ++
": " ++ typePrintImplRec(c)(filtered, true)
} else {
fansi.Str("type ") ++ printSym(t) ++ suffix
}
}
if (stmts.length == 0) None
else Some(stmts.reduceLeft((l, r) => l + "; " + r))
}

tpe match {
case TypeBounds(lo, hi) =>
val res = printBounds(lo, hi)
(fansi.Str("_") ++ res, WrapType.NoWrap)
case ThisType(sym) =>
(
printSymFull(sym) + (if (sym.isPackage || sym.isModuleClass) ""
else ".this.type"),
WrapType.NoWrap
)

case SingleType(NoPrefix, sym) =>
(printSym(sym) ++ (if (rightMost) ".type" else ""), WrapType.NoWrap)
case SingleType(pre, sym) =>
(
prefixFor(pre, sym) ++ (if (rightMost) ".type" else ""),
WrapType.NoWrap
)
// Special-case operator two-parameter types as infix
case TypeRef(pre, sym, List(left, right))
if lookup(sym) && sym.name.encodedName.toString != sym.name.decodedName.toString =>
(
typePrintImplRec(c)(left, true) ++ " " ++ printSym(sym) ++ " " ++ typePrintImplRec(
c
)(right, true),
WrapType.Infix
)

case TypeRef(pre, sym, args) if functionTypes.contains(sym.fullName) =>
args match {
case Seq(r) =>
(
fansi.Str("() => ") ++ typePrintImplRec(c)(r, true),
WrapType.Infix
)

case many =>
val (left, leftWrap) = typePrintImplRec0(c)(many.head, true)

if (many.size == 2 && leftWrap == WrapType.NoWrap) {
(
left ++ " => " ++ typePrintImplRec(c)(many(1), true),
WrapType.Infix
)
} else
(
fansi.Str("(") ++
fansi.Str.join(
(left +: many.init.tail.map(typePrintImplRec(c)(_, true))),
sep = ", "
) ++
") => " ++ typePrintImplRec(c)(many.last, true),
WrapType.Infix
)
}
case TypeRef(pre, sym, args) if tupleTypes.contains(sym.fullName) =>
(
fansi.Str("(") ++
fansi.Str
.join(args.map(typePrintImplRec(c)(_, true)), sep = ", ") ++
")",
WrapType.Tuple
)

case TypeRef(NoPrefix, sym, args) =>
(printSym(sym) ++ printArgs(args), WrapType.NoWrap)
case TypeRef(pre, sym, args) =>
if (sym.fullName == "scala.<byname>")
(
fansi.Str("=> ") ++ typePrintImplRec(c)(args(0), true),
WrapType.Infix
)
else (prefixFor(pre, sym) ++ printArgs(args), WrapType.NoWrap)
case et @ ExistentialType(quantified, underlying) =>
(
showRefinement(quantified) match {
case None => typePrintImplRec(c)(underlying, true)
case Some(block) =>
typePrintImplRec(c)(underlying, true) ++ " forSome { " ++ block ++ " }"
},
WrapType.NoWrap
)
case AnnotatedType(annots, tp) =>
val mapped = annots
.map(x => " @" + typePrintImplRec(c)(x.tpe, true))
.reduceLeft((x, y) => x + y)

(
typePrintImplRec(c)(tp, true) + mapped,
WrapType.NoWrap
)
case RefinedType(parents, defs) =>
val pre =
if (parents.forall(_ =:= typeOf[AnyRef])) ""
else
parents
.map(typePrintImplRec(c)(_, true))
.reduceLeft[fansi.Str]((l, r) => l ++ " with " ++ r)
(
pre + (if (defs.isEmpty) "" else "{" ++ defs.mkString(";") ++ "}"),
WrapType.NoWrap
)
case ConstantType(value) =>
(fansi.Str(value.value.toString()), WrapType.NoWrap)
}
}

}
Loading