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

force usage of shaded TPrint #1546

Merged
merged 1 commit into from
Feb 14, 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
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ lazy val core = project
// metaconfig 0.10.0 shaded pprint but rules built with an old
// scalafix-core must have the original package in the classpath to link
// https://github.com/scalameta/metaconfig/pull/154/files#r794005161
pprint
pprint % Runtime
)
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,23 @@ public static List<URL> scalafixCliJars(
String scalafixVersion,
String scalaVersion
) throws ScalafixException {
Dependency scalafixCli = Dependency.parse(
List<Dependency> dependencies = new ArrayList<Dependency>();
dependencies.add(
Dependency.parse(
"ch.epfl.scala:::scalafix-cli:" + scalafixVersion,
ScalaVersion.of(scalaVersion)
).withConfiguration("runtime")
);
return fetch(repositories, Collections.singletonList(scalafixCli), ResolutionParams.create());
// Coursier does not seem to fetch runtime dependencies transitively, despite what Maven dictates
Copy link
Collaborator Author

@bjaglin bjaglin Feb 14, 2022

Choose a reason for hiding this comment

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

I will follow up in coursier/coursier with a repro, since the scalafix CLIs installed by Coursier will most likely be unable to load old rules (just like the test was showing before this workaround was added) since they won't load pprint.

Running community rules with the CLI is probably not common anyway since it requires setting up the tool classpath yourself, so I think it's OK to break this until it's fixed in cs install/launch.

https://github.com/scalacenter/scalafix/runs/5184084825?check_suite_focus=true

[info] java.util.ServiceConfigurationError: scalafix.v1.Rule: Provider fix.SortImports could not be instantiated
[info] at java.util.ServiceLoader.fail(ServiceLoader.java:232)
[info] at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
[info] at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
[info] at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
[info] at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
[info] at scala.collection.convert.JavaCollectionWrappers$JIteratorWrapper.next(JavaCollectionWrappers.scala:38)
[info] at scala.collection.immutable.List.prependedAll(List.scala:156)
[info] at scala.collection.IterableOnceOps.toList(IterableOnce.scala:1251)
[info] at scala.collection.IterableOnceOps.toList$(IterableOnce.scala:1251)
[info] at scala.collection.AbstractIterator.toList(Iterator.scala:1293)
[info] ...
[info] Cause: java.lang.NoClassDefFoundError: pprint/TPrintColors
[info] at fix.SortImports.(SortImports.scala:30)

Copy link
Collaborator Author

@bjaglin bjaglin Feb 15, 2022

Choose a reason for hiding this comment

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

Actually, it works as expected when using sonatype:

$ cs resolve -r sonatype:snapshots -r central --no-default ch.epfl.scala:scalafix-cli_2.13.8:0.9.34+25-b98e220d-SNAPSHOT:runtime | grep com.lihaoyi:pprint
com.lihaoyi:pprint_2.13:0.6.6:runtime

So the there shouldn't be any problem with CLI installed by coursier. There is however something fishy with ivy2local: coursier/coursier#2370. So the code added there is probably needed only for tests to pass, but it does not hurt anyway.

// https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-scope
// so to be able to retrieve the runtime dependencies of scalafix-core, we need an explicit reference
dependencies.add(
Dependency.parse(
"ch.epfl.scala::scalafix-core:" + scalafixVersion,
ScalaVersion.of(scalaVersion)
).withConfiguration("runtime")
);
return fetch(repositories, dependencies, ResolutionParams.create());
}

public static List<URL> toolClasspath(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package scalafix.internal.rule

import java.util.regex.Pattern

import pprint.TPrint
import scalafix.config.CustomMessage

class TPrintImplicits {
implicit val tprintPattern: TPrint[List[CustomMessage[Pattern]]] =
TPrint.literal("List[Regex]")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package scalafix.internal.rule

import java.util.regex.Pattern

import metaconfig.pprint._
import scalafix.config.CustomMessage

class TPrintImplicits {
implicit val tprintPattern: TPrint[List[CustomMessage[Pattern]]] =
Copy link
Collaborator Author

@bjaglin bjaglin Feb 14, 2022

Choose a reason for hiding this comment

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

This implicit has actually no effect (neither before or after this PR) as the signature of the param it was targeting changed in https://github.com/scalacenter/scalafix/pull/907/files#diff-fa445c2f97a1bfb229caa4cd2fdc97cb9c243ddac155698a132bcbdb22970910R91. Following-up in #1547.

new TPrint[List[CustomMessage[Pattern]]] {
def render(implicit tpc: TPrintColors): fansi.Str =
fansi.Str("List[Regex]")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import metaconfig.annotation.ExampleValue
import metaconfig.annotation.Hidden
import metaconfig.generic
import metaconfig.generic.Surface
import pprint.TPrint
import scalafix.config.CustomMessage
import scalafix.config.Regex

Expand Down Expand Up @@ -108,8 +107,6 @@ case class DisableSyntaxConfig(
}

object DisableSyntaxConfig {
implicit val tprintPattern: TPrint[List[CustomMessage[Pattern]]] =
TPrint.literal("List[Regex]")
val default: DisableSyntaxConfig = DisableSyntaxConfig()
implicit val surface: Surface[DisableSyntaxConfig] =
generic.deriveSurface[DisableSyntaxConfig]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ final class TestkitPath(
val testPath: RelativePath,
val semanticdbPath: RelativePath
) {
override def toString: String = {
val map = Map(
"input" -> input,
"testPath" -> testPath,
"semanticdbPath" -> semanticdbPath
)
pprint.PPrinter.BlackWhite.tokenize(map).mkString
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only pprint.TPrint has been shaded into metaconfig. I felt like other usages of pprint here and below were not worth adding an explicit compile-time dependency to pprint

}
def testName: String = testPath.toURI(isDirectory = false).toString
def toInput: Input =
Input.VirtualFile(testName, FileIO.slurp(input, StandardCharsets.UTF_8))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ final class TestkitProperties(
throw new NoSuchElementException(path.toString())
}
}
override def toString: String = {
val map = Map(
"inputSourceDirectories" -> inputSourceDirectories,
"outputSourceDirectories" -> outputSourceDirectories,
"inputClasspath" -> inputClasspath.syntax,
"sourceroot" -> sourceroot,
"scalaVersion" -> scalaVersion,
"scalacOptions" -> scalacOptions
)
pprint.PPrinter.BlackWhite.tokenize(map).mkString
}
}

object TestkitProperties {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class PrettyTypeFuzzSuite extends BasePrettyTypeSuite {
checkPath(file)
} catch {
case scala.meta.internal.classpath.MissingSymbolException(e) =>
pprint.log(file)
println(file)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class MavenFuzzSuite extends AnyFunSuite with DiffAssertions {
finally stream.close()
result += out
} else {
pprint.log(errors)
println(errors)
}
}
}
Expand Down Expand Up @@ -154,7 +154,7 @@ class MavenFuzzSuite extends AnyFunSuite with DiffAssertions {
exec("git", "init")
exec("git", "add", ".")
exec("git", "commit", "-m", "first-commit")
pprint.log(paths.length)
println(paths.length)
val args = scalafix
.newArguments()
.withSourceroot(tmp)
Expand All @@ -163,19 +163,19 @@ class MavenFuzzSuite extends AnyFunSuite with DiffAssertions {
.withClasspath(classfiles.asJava)
.withMode(ScalafixMainMode.CHECK)
val exit = args.run()
pprint.log(exit)
println(exit)
// exec("git", "diff")
FileIO.listAllFilesRecursively(AbsolutePath(tmp)).foreach { path =>
if (path.toNIO.getFileName.toString.endsWith(".scala")) {
val text = FileIO.slurp(path, StandardCharsets.UTF_8)
val errors = compileErrors(g, text, path.toString())
if (errors.nonEmpty) {
pprint.log(path)
pprint.log(errors)
println(path)
println(errors)
}
}
}
pprint.log(tmp)
println(tmp)
}
}
check("ExplicitResultTypes")
Expand Down