From 664fcba5b0a08ceb9b700d9c17cc43feea0681d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Thu, 17 Aug 2017 12:26:04 +0200 Subject: [PATCH] Emit deprecation warnings when referencing a rewrite by old name. This change allows rewrites to change their names without breaking compatibility. Users using the old name will receive deprecation warnings and have opportunity to switch to the new name. --- .../src/main/scala/scalafix/cli/Cli.scala | 6 +---- .../main/scala/scalafix/cli/CliRunner.scala | 3 ++- .../internal/cli/ScalafixOptions.scala | 4 ++++ .../main/scala/scalafix/config/package.scala | 2 +- .../internal/config/LazySemanticCtx.scala | 24 +++++++++++++++++++ .../config/ScalafixMetaconfigReaders.scala | 13 ++++++---- .../scalafix/internal/config/package.scala | 8 ------- .../scala/scalafix/rewrite/RewriteName.scala | 14 ++++++++++- .../main/scala/scalafix/util/Deprecated.scala | 4 ++++ .../scalafix/reflect/ScalafixReflect.scala | 4 ++-- .../scala/scalafix/testkit/DiffTest.scala | 6 +++-- 11 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 scalafix-core/shared/src/main/scala/scalafix/internal/config/LazySemanticCtx.scala create mode 100644 scalafix-core/shared/src/main/scala/scalafix/util/Deprecated.scala diff --git a/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala b/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala index 4059e14f4..a8ae1b5f4 100644 --- a/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala +++ b/scalafix-cli/src/main/scala/scalafix/cli/Cli.scala @@ -225,11 +225,7 @@ object ScalafixRewriteNames { val result = cliCommand match { case CliCommand.PrintAndExit(msg, exit) => if (exit.isOk) commonOptions.out.println(msg) - else { - ScalafixReporter.default - .copy(outStream = commonOptions.err) - .error(msg) - } + else commonOptions.reporter.error(msg) exit case CliCommand.RunScalafix(runner) => val exit = runner.run() diff --git a/scalafix-cli/src/main/scala/scalafix/cli/CliRunner.scala b/scalafix-cli/src/main/scala/scalafix/cli/CliRunner.scala index b3302197c..fc83a3133 100644 --- a/scalafix-cli/src/main/scala/scalafix/cli/CliRunner.scala +++ b/scalafix-cli/src/main/scala/scalafix/cli/CliRunner.scala @@ -333,7 +333,8 @@ object CliRunner { if (kind.isSyntactic) None else computeAndCacheDatabase() } - private val lazySemanticCtx: LazySemanticCtx = resolveDatabase + private val lazySemanticCtx: LazySemanticCtx = + new LazySemanticCtx(resolveDatabase, common.reporter) // expands a single file into a list of files. def expand(matcher: FilterMatcher)(path: AbsolutePath): Seq[FixFile] = { diff --git a/scalafix-cli/src/main/scala/scalafix/internal/cli/ScalafixOptions.scala b/scalafix-cli/src/main/scala/scalafix/internal/cli/ScalafixOptions.scala index 4cb52fcf3..7841d5793 100644 --- a/scalafix-cli/src/main/scala/scalafix/internal/cli/ScalafixOptions.scala +++ b/scalafix-cli/src/main/scala/scalafix/internal/cli/ScalafixOptions.scala @@ -5,6 +5,8 @@ import java.io.InputStream import java.io.PrintStream import scala.meta._ import scala.meta.io.AbsolutePath +import scalafix.internal.config.PrintStreamReporter +import scalafix.internal.config.ScalafixReporter import scalafix.internal.rewrite.ProcedureSyntax import scalafix.rewrite.ScalafixRewrites import caseapp._ @@ -16,6 +18,8 @@ case class CommonOptions( @Hidden err: PrintStream = System.err, @Hidden stackVerbosity: Int = 20 ) { + lazy val reporter: PrintStreamReporter = + ScalafixReporter.default.copy(outStream = out) def workingPath = AbsolutePath(workingDirectory) def workingDirectoryFile = new File(workingDirectory) } diff --git a/scalafix-core/shared/src/main/scala/scalafix/config/package.scala b/scalafix-core/shared/src/main/scala/scalafix/config/package.scala index bf9c411c2..f278cef21 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/config/package.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/config/package.scala @@ -18,7 +18,7 @@ package object config { configuration: String, decoder: ConfDecoder[Rewrite] ): Configured[(Rewrite, internal.config.ScalafixConfig)] = - fromInput(Input.String(configuration), _ => None, Nil, decoder) + fromInput(Input.String(configuration), LazySemanticCtx.empty, Nil, decoder) /** Load configuration from an input. * diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/config/LazySemanticCtx.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/config/LazySemanticCtx.scala new file mode 100644 index 000000000..37e081801 --- /dev/null +++ b/scalafix-core/shared/src/main/scala/scalafix/internal/config/LazySemanticCtx.scala @@ -0,0 +1,24 @@ +package scalafix.internal.config + +import scalafix.SemanticCtx + +// The challenge when loading a rewrite is that 1) if it's semantic it needs a +// semanticCtx constructor argument and 2) we don't know upfront if it's semantic. +// For example, to know if a classloaded rewrites is semantic or syntactic +// we have to test against it's Class[_]. For default rewrites, the interface +// to detect if a rewrite is semantic is different. +// LazySemanticCtx allows us to delay the computation of a semanticCtx right up until +// the moment we instantiate the rewrite. +//type LazySemanticCtx = RewriteKind => Option[SemanticCtx] +class LazySemanticCtx( + f: RewriteKind => Option[SemanticCtx], + val reporter: ScalafixReporter) + extends Function[RewriteKind, Option[SemanticCtx]] { + override def apply(v1: RewriteKind): Option[SemanticCtx] = f(v1) +} + +object LazySemanticCtx { + lazy val empty = new LazySemanticCtx(_ => None, ScalafixReporter.default) + def apply(f: RewriteKind => Option[SemanticCtx]): LazySemanticCtx = + new LazySemanticCtx(f, ScalafixReporter.default) +} diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/config/ScalafixMetaconfigReaders.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/config/ScalafixMetaconfigReaders.scala index 9ed4726d9..a0f4eed7c 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/internal/config/ScalafixMetaconfigReaders.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/internal/config/ScalafixMetaconfigReaders.scala @@ -102,15 +102,20 @@ trait ScalafixMetaconfigReaders { ScalafixRewrites.syntaxName2rewrite ++ semanticCtx.fold(Map.empty[String, Rewrite])( ScalafixRewrites.name2rewrite) - ReaderUtil.fromMap(names).read(conf) + val result = ReaderUtil.fromMap(names).read(conf) + result match { + case Ok(rewrite) => + rewrite.rewriteName + .reportDeprecationWarning(value, getSemanticCtx.reporter) + case _ => + } + result } private lazy val semanticRewriteClass = classOf[SemanticRewrite] def classloadRewrite( semanticCtx: LazySemanticCtx): Class[_] => Seq[SemanticCtx] = { cls => - val semanticRewrite = - cls.getClassLoader.loadClass("scalafix.rewrite.SemanticRewrite") val kind = if (semanticRewriteClass.isAssignableFrom(cls)) RewriteKind.Semantic else RewriteKind.Syntactic @@ -146,7 +151,7 @@ trait ScalafixMetaconfigReaders { } def baseSyntacticRewriteDecoder: ConfDecoder[Rewrite] = - baseRewriteDecoders(_ => None) + baseRewriteDecoders(LazySemanticCtx.empty) def baseRewriteDecoders(semanticCtx: LazySemanticCtx): ConfDecoder[Rewrite] = { MetaconfigPendingUpstream.orElse( defaultRewriteDecoder(semanticCtx), diff --git a/scalafix-core/shared/src/main/scala/scalafix/internal/config/package.scala b/scalafix-core/shared/src/main/scala/scalafix/internal/config/package.scala index af212fc09..ad884fbdc 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/internal/config/package.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/internal/config/package.scala @@ -6,12 +6,4 @@ import scalafix.SemanticCtx package object config extends ScalafixMetaconfigReaders { type MetaParser = Parse[_ <: Tree] - // The challenge when loading a rewrite is that 1) if it's semantic it needs a - // semanticCtx constructor argument and 2) we don't know upfront if it's semantic. - // For example, to know if a classloaded rewrites is semantic or syntactic - // we have to test against it's Class[_]. For default rewrites, the interface - // to detect if a rewrite is semantic is different. - // LazySemanticCtx allows us to delay the computation of a semanticCtx right up until - // the moment we instantiate the rewrite. - type LazySemanticCtx = RewriteKind => Option[SemanticCtx] } diff --git a/scalafix-core/shared/src/main/scala/scalafix/rewrite/RewriteName.scala b/scalafix-core/shared/src/main/scala/scalafix/rewrite/RewriteName.scala index 09f364bee..d8a39754f 100644 --- a/scalafix-core/shared/src/main/scala/scalafix/rewrite/RewriteName.scala +++ b/scalafix-core/shared/src/main/scala/scalafix/rewrite/RewriteName.scala @@ -1,9 +1,11 @@ package scalafix.rewrite +import scalafix.internal.config.ScalafixReporter + /** A thin wrapper around a string name and optional deprecation warning. */ final case class RewriteIdentifier( value: String, - deprecated: Option[scala.deprecated] + deprecated: Option[scalafix.util.Deprecated] ) { override def toString: String = value } @@ -22,6 +24,16 @@ final case class RewriteName(identifiers: List[RewriteIdentifier]) { def +(other: RewriteName): RewriteName = new RewriteName((identifiers :: other.identifiers :: Nil).flatten) override def toString: String = name + def reportDeprecationWarning(name: String, reporter: ScalafixReporter): Unit = { + identifiers.foreach { ident => + if (ident.value == name) { + ident.deprecated.foreach { d => + reporter.warn( + s"Name $name is deprecated. ${d.message} (since ${d.since})") + } + } + } + } } object RewriteName { diff --git a/scalafix-core/shared/src/main/scala/scalafix/util/Deprecated.scala b/scalafix-core/shared/src/main/scala/scalafix/util/Deprecated.scala new file mode 100644 index 000000000..90bd2b083 --- /dev/null +++ b/scalafix-core/shared/src/main/scala/scalafix/util/Deprecated.scala @@ -0,0 +1,4 @@ +package scalafix.util + +/** Identical to scala.deprecated except it's a case class. */ +final case class Deprecated(message: String, since: String) diff --git a/scalafix-reflect/src/main/scala/scalafix/reflect/ScalafixReflect.scala b/scalafix-reflect/src/main/scala/scalafix/reflect/ScalafixReflect.scala index 32be25ed6..01aa220fb 100644 --- a/scalafix-reflect/src/main/scala/scalafix/reflect/ScalafixReflect.scala +++ b/scalafix-reflect/src/main/scala/scalafix/reflect/ScalafixReflect.scala @@ -8,10 +8,10 @@ import metaconfig.ConfDecoder object ScalafixReflect { def syntactic: ConfDecoder[Rewrite] = - fromLazySemanticCtx(_ => None) + fromLazySemanticCtx(LazySemanticCtx.empty) def semantic(semanticCtx: SemanticCtx): ConfDecoder[Rewrite] = - fromLazySemanticCtx(_ => Some(semanticCtx)) + fromLazySemanticCtx(LazySemanticCtx(_ => Some(semanticCtx))) def fromLazySemanticCtx(semanticCtx: LazySemanticCtx): ConfDecoder[Rewrite] = rewriteConfDecoder( diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/DiffTest.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/DiffTest.scala index 9dfd9dd8e..b431f08b7 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/DiffTest.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/DiffTest.scala @@ -3,6 +3,7 @@ package scalafix.testkit import scala.meta._ import scalafix.SemanticCtx import scalafix.Rewrite +import scalafix.internal.config.LazySemanticCtx import scalafix.internal.config.ScalafixConfig import scalafix.reflect.ScalafixReflect import org.scalatest.exceptions.TestFailedException @@ -32,11 +33,12 @@ object DiffTest { .collectFirst { case Token.Comment(comment) => val decoder = - ScalafixReflect.fromLazySemanticCtx(_ => Some(semanticCtx)) + ScalafixReflect.fromLazySemanticCtx( + LazySemanticCtx(_ => Some(semanticCtx))) ScalafixConfig .fromInput( Input.VirtualFile(label, stripPrefix(comment)), - _ => Some(semanticCtx))(decoder) + LazySemanticCtx(_ => Some(semanticCtx)))(decoder) .get } .getOrElse(throw new TestFailedException(