From 1ee6bf8bfe2d102456c90ea4be3007cbb1ccf27a Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Fri, 27 Sep 2024 21:28:23 +0200 Subject: [PATCH] on version mismatch, protect PC dynamic loading behind a flag --- project/ScalafixBuild.scala | 5 ++- .../internal/rule/ExplicitResultTypes.scala | 45 ++++++++++++------- .../rule/ExplicitResultTypesConfig.scala | 6 +++ .../scala/scalafix/tests/rule/RuleSuite.scala | 4 +- ...a3CompilerArtifactsOnVersionMismatch.scala | 15 +++++++ .../rules/ExplicitResultTypesSuite.scala | 32 +++++++++++++ ...a3CompilerArtifactsOnVersionMismatch.scala | 5 +++ 7 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 scalafix-tests/input/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala create mode 100644 scalafix-tests/output/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala diff --git a/project/ScalafixBuild.scala b/project/ScalafixBuild.scala index 673f7dc74..0755ac900 100644 --- a/project/ScalafixBuild.scala +++ b/project/ScalafixBuild.scala @@ -58,7 +58,10 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys { val xsource3 = TargetAxis(sv, xsource3 = true) (prevVersions :+ xsource3).map((sv, _)) - } :+ (scala3Next, TargetAxis(scala213)) + } ++ Seq( + (scala3Next, TargetAxis(scala213)), + (scala3Next, TargetAxis(scala3LTS)) + ) lazy val publishLocalTransitive = taskKey[Unit]("Run publishLocal on this project and its dependencies") diff --git a/scalafix-rules/src/main/scala-3/scalafix/internal/rule/ExplicitResultTypes.scala b/scalafix-rules/src/main/scala-3/scalafix/internal/rule/ExplicitResultTypes.scala index 97b7e8dde..f22922096 100644 --- a/scalafix-rules/src/main/scala-3/scalafix/internal/rule/ExplicitResultTypes.scala +++ b/scalafix-rules/src/main/scala-3/scalafix/internal/rule/ExplicitResultTypes.scala @@ -40,25 +40,36 @@ final class ExplicitResultTypes( "To fix this problem, either remove ExplicitResultTypes from .scalafix.conf or upgrade the compiler in your build." ) } else { - config.conf // Support deprecated explicitReturnTypes config - .getOrElse("explicitReturnTypes", "ExplicitResultTypes")( - ExplicitResultTypesConfig.default - ) - .map(c => - new ExplicitResultTypes( - c, - Option { - if ( - stripPatchVersion(config.scalaVersion) == - stripPatchVersion(compilerScalaVersion) - ) - PresentationCompilerTypeInferrer - .static(config, new ScalaPresentationCompiler()) + config.conf.getOrElse("ExplicitResultTypes")(this.config).andThen { + conf => + val majorMinorCompilerScalaVersion = + stripPatchVersion(compilerScalaVersion) + + val matchingMinors = + majorMinorScalaVersion == majorMinorCompilerScalaVersion + + if ( + !matchingMinors && !conf.fetchScala3CompilerArtifactsOnVersionMismatch + ) { + Configured.error( + s"The ExplicitResultTypes rule was compiled with a different Scala 3 minor ($majorMinorCompilerScalaVersion) " + + s"than the target sources ($majorMinorScalaVersion). To fix this problem, either enable " + + "ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch in .scalafix.conf or check " + + s"if Scalafix can be loaded with $majorMinorScalaVersion." + ) + } else { + val pcTypeInferrer = + if (matchingMinors) + PresentationCompilerTypeInferrer.static( + config, + new ScalaPresentationCompiler() + ) else PresentationCompilerTypeInferrer.dynamic(config) - } - ) - ) + + Configured.Ok(new ExplicitResultTypes(conf, Some(pcTypeInferrer))) + } + } } } diff --git a/scalafix-rules/src/main/scala/scalafix/internal/rule/ExplicitResultTypesConfig.scala b/scalafix-rules/src/main/scala/scalafix/internal/rule/ExplicitResultTypesConfig.scala index 03b4c5aa6..6e72c4e4a 100644 --- a/scalafix-rules/src/main/scala/scalafix/internal/rule/ExplicitResultTypesConfig.scala +++ b/scalafix-rules/src/main/scala/scalafix/internal/rule/ExplicitResultTypesConfig.scala @@ -39,6 +39,12 @@ case class ExplicitResultTypesConfig( rewriteStructuralTypesToNamedSubclass: Boolean = true, @Description("If true, adds result types only to implicit definitions.") onlyImplicits: Boolean = false, + @Description( + "If true and the Scala 3 version Scalafix was compiled with differs from the target files' one," + + "attempts to resolve and download compiler artifacts dynamically via Coursier. " + + "Disabled by default as this introduces a performance overhead and might not work." + ) + fetchScala3CompilerArtifactsOnVersionMismatch: Boolean = false, @Hidden() symbolReplacements: Map[String, String] = Map.empty ) diff --git a/scalafix-tests/expect/src/test/scala/scalafix/tests/rule/RuleSuite.scala b/scalafix-tests/expect/src/test/scala/scalafix/tests/rule/RuleSuite.scala index 1cd74c005..b6f837ca7 100644 --- a/scalafix-tests/expect/src/test/scala/scalafix/tests/rule/RuleSuite.scala +++ b/scalafix-tests/expect/src/test/scala/scalafix/tests/rule/RuleSuite.scala @@ -39,7 +39,9 @@ class RuleSuite extends AbstractSemanticRuleSuite with AnyFunSuiteLike { val versionMismatch = stripPatch(RulesBuildInfo.scalaVersion) != stripPatch(props.scalaVersion) val explicitResultTypesTest = - diffTest.path.input.toNIO.toString.contains("explicitResultTypes") + diffTest.path.input.toNIO.toString.contains( + "explicitResultTypes" + java.io.File.separator // don't skip tests with a suffix + ) // ExplicitResultTypes can only run against sources compiled with the sam // binary version as the one used to compile the rule diff --git a/scalafix-tests/input/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala b/scalafix-tests/input/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala new file mode 100644 index 000000000..a79d8b417 --- /dev/null +++ b/scalafix-tests/input/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala @@ -0,0 +1,15 @@ +/* +rules = ExplicitResultTypes +ExplicitResultTypes.skipSimpleDefinitions = true +ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch = true + +// RuleSuite.scala does run this test for expect3.xTarget3.y with 3.x != 3.y +// because it is in a different folder than "explicitResultTypes" (the suffix +// matters). Take that into account if you move this file. + +*/ +package test.explicitResultTypesRunAcrossMinors + +object FetchScala3CompilerArtifactsOnVersionMismatch { + def foo = 1 +} diff --git a/scalafix-tests/integration/src/test/scala-3/scalafix/tests/rules/ExplicitResultTypesSuite.scala b/scalafix-tests/integration/src/test/scala-3/scalafix/tests/rules/ExplicitResultTypesSuite.scala index 0dda137bd..81b40142d 100644 --- a/scalafix-tests/integration/src/test/scala-3/scalafix/tests/rules/ExplicitResultTypesSuite.scala +++ b/scalafix-tests/integration/src/test/scala-3/scalafix/tests/rules/ExplicitResultTypesSuite.scala @@ -2,7 +2,9 @@ package scalafix.tests.rules import scala.meta.io.AbsolutePath +import metaconfig.Conf import metaconfig.Configured +import metaconfig.typesafeconfig.* import org.scalatest.funsuite.AnyFunSuite import scalafix.internal.rule.ExplicitResultTypes import scalafix.v1.Configuration @@ -34,4 +36,34 @@ class ExplicitResultTypesSuite extends AnyFunSuite { assert(rule.withConfiguration(earlyScala3) == Configured.error(expected)) } + test("Other Scala 3 minor versions are not supported by default") { + val neitherLTSNorNext = + config.withScalaVersion("3.5.0") + + val v = + buildinfo.RulesBuildInfo.scalaVersion.split('.').take(2).mkString(".") + + val expected = + s"The ExplicitResultTypes rule was compiled with a different Scala 3 minor ($v) than the target sources (3.5). " + + "To fix this problem, either enable ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch in .scalafix.conf or check if Scalafix can be loaded with 3.5." + + assert( + rule.withConfiguration(neitherLTSNorNext) == Configured.error(expected) + ) + } + + test( + "fetchScala3CompilerArtifactsOnVersionMismatch enables dynamic loading" + ) { + val neitherLTSNorNext = + config.withScalaVersion("3.5.0") + + val conf = Conf + .parseString( + "ExplicitResultTypes.fetchScala3CompilerArtifactsOnVersionMismatch = true" + ) + .get + assert(rule.withConfiguration(neitherLTSNorNext.withConf(conf)).isOk) + } + } diff --git a/scalafix-tests/output/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala b/scalafix-tests/output/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala new file mode 100644 index 000000000..91de38dbd --- /dev/null +++ b/scalafix-tests/output/src/main/scala-3/test/explicitResultTypesRunAcrossMinors/FetchScala3CompilerArtifactsOnVersionMismatch.scala @@ -0,0 +1,5 @@ +package test.explicitResultTypesRunAcrossMinors + +object FetchScala3CompilerArtifactsOnVersionMismatch { + def foo: Int = 1 +}