Respect Configuration in scalafixConfigSettings

[etspaceman]

Hello -

Upgrading to sbt-scalafix 0.9.19 caused one of my projects to fail. When poking around, I noticed that the new scalafixOnCompile configuration setup did not respect the configuration that was passed into the scalafixConfigSettings function.

The scripted test got around this by doing inConfig(IntegrationTest)(scalafixConfigSettings(IntegrationTest)). You should be able to just do scalafixConfigSettings(IntegrationTest).

This PR updates the scalafixConfigSettings function to respect the passed Configuration value for these items.

[bjaglin]

Thanks for the PR!

Which version were you upgrading from, and what started to fail? The inConfig(IntegrationTest)(scalafixConfigSettings(IntegrationTest)) syntax has been documented for a long time, see: https://github.com/scalacenter/scalafix/blame/6584af204fd2a5ef0c261fdc97ad599962a8ffa0/docs/users/installation.md#L138, so I am wondering what can have regressed in 0.9.19?

That's not to say we shouldn't try to simplify and get rid of the config repetition. However, using inConfig() is pretty standard for all sbt plugins, the oddity here is that it's passed as an argument of scalafixConfigSettings. I am AFK at the moment, but I think it is worth investigating whether we could remove that argument. As far as I remember, the scope passed in inConfig() is only picked up within the settings block, not on value lookups in vals or defs that block references.

[etspaceman]

Hey @bjaglin,

Which version were you upgrading from, and what started to fail?

0.9.17. I basically had the following code snippet:

.settings(scalafixConfigSettings(IntegrationTest):_*)

This actually worked for a long time. Scalafix ran properly on my project's integration tests. I must have missed that item in the docs when going through my setup originally, and I was happily oblivious. 😅

I am wondering what can have regressed in 0.9.19?

Upgrading to 0.9.19 introduced the scalafixOnCompile option which added the compile section of the settings above. The compile step is undefined unless properly scoped. You can reproduce this in the above PR's tests by reverting the plugin's change:

[info] [error] Reference to undefined setting:
[info] [error]
[info] [error]   rewrite / compile from rewrite / compile ((scalafix.sbt.ScalafixPlugin.autoImport.scalafixConfigSettings) ScalafixPlugin.scala:100)
[info] [error]      Did you mean rewrite / Compile / compile ?

I am AFK at the moment, but I think it is worth investigating whether we could remove that argument

Potentially! That is indeed the original source of my confusion here, because I assumed that passing in IntegrationTest would properly scope that function call. Having it in the signature makes this quite confusing otherwise.

That said, I would also be happy with this implementation. I am biased of course. 😄

[bjaglin]

inConfig seems to return the same type as scalafixConfigSettings, is there a reason why you moved down all the settings and used the spread operator? it would be nicer if we could keep them inline (for the diff, but mostly for users!)

[bjaglin]

Sorry for the silence, summer break here. Since the change is backward-compatible and I have found at least one project using this syntax (akka-grpc), I am fine with it - I just have a remaining cosmetic comment.

@olafurpg any objection?