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

force usage of shaded TPrint #1546

merged 1 commit into from
Feb 14, 2022

Conversation

bjaglin
Copy link
Collaborator

@bjaglin bjaglin commented Feb 14, 2022

Follows #1530
Should unblock #1531

Injecting instances of pprint.TPrint to customize the docs no longer has effect as of #1530, which was a silent breaking change. This commit exposes the breaking change so that rules built against the latest scalafix-core either drop usage of TPrint or switch to the shaded version which is used internally by metaconfig.

@bjaglin bjaglin closed this Feb 14, 2022
@bjaglin bjaglin reopened this Feb 14, 2022
@bjaglin bjaglin force-pushed the fix-pprint branch 4 times, most recently from b23114d to e65c659 Compare February 14, 2022 12:31
"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

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.

@bjaglin bjaglin force-pushed the fix-pprint branch 5 times, most recently from de2e77d to 8731c79 Compare February 14, 2022 13:59
Injecting instances of pprint.TPrint to customize the docs no longer
has effect as of ffcd8b9, which was a silent breaking change. This
commit exposes the breaking change so that rules built against the
latest scalafix-core either drop usage of TPrint or switch to the
shaded version which is used internally by metaconfig.
);
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants