add scalafixCallback setting key to customize diagnostics reporting

[xuwei-k]

Towards scalacenter/scalafix#951

[bjaglin]

Thanks for the PR. I am curious about your use-case. I assume it's similar to what was discussed in scalacenter/scalafix#951?

[bjaglin]

As I read again my "Can be set in ThisBuild or at project-level" suggestion, I realized I missed an important point in my first review - see inline comment below.

[bjaglin]

Watch out for:

sbt-scalafix/src/main/scala/scalafix/internal/sbt/ScalafixInterface.scala

Lines 131 to 136 in c2777cf

	fromToolClasspathMemo.getOrElseUpdate(
	(
	scalafixBinaryScalaVersion,
	scalafixDependencies,
	scalafixCustomResolvers
	), {

As it stands in the current state of this PR, a client might set different values for scalafixCallback across projects, and see only one being used because of memoization.

We can either

1. add callback as a cache key (using reference equality I guess?)
2. support only one value per build

Suggested change

	callback = scalafixCallback.value
	callback = (ThisBuild / scalafixCallback).value

I would go for (1) for consistency with other configurable attributes of scalafixInterface

[bjaglin]

Reopening this for visibility: given the current state of sbt-scalafix, there is a small limitation to be aware of in the solution (2) you went for. The cache is never flushed, so if a sbt command changed the state of the build and update ThisBuild / scalafixCallback, you will get false positive cache hits.

This is a corner case, but depending on how you end up injecting a per-invocation scalafixCallback to avoid mutability at that level, this could be annoying. If that's the case, we'll have to go for (1), as well as maybe keep the cache in a setting key to avoid leaking memory upon each sbt reload because of a new JVM instance of ScalafixMainCallback.

[xuwei-k]

I assume it's similar to what was discussed in scalacenter/scalafix#951?

Oh, I didn't know the issue. I have same motivation 👍

[bjaglin]

I assume it's similar to what was discussed in scalacenter/scalafix#951?

Oh, I didn't know the issue. I have same motivation 👍

Nice. I just looked at your projects and I assume this PR is effectively a step towards xuwei-k/sbt-warning-diff#6. I'll be watching that - if you manage to do what you want with a custom ScalafixMainCallback, we could upstream it to the main scalafix repo, maybe in a new contrib module and close scalacenter/scalafix#951. Let' see!

[bjaglin]

For the record, I am planning to do a Scalafix release in a few weeks, in sync with the scala 2.x patch releases. Until then, you can try the snapshots.