From b3e3fbf7e04793fa6625719f2b247d48805c10ce Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 25 Mar 2020 17:32:18 -0400 Subject: [PATCH] apiStatus annotation This implements `apiStatus ` annotation as a generic form of `deprecated` annotation. `apiStatus` takes `category` and `defaultAction` parameters, corresponding to configurable warning's category and action. One of the usage is to trigger compiler error from the library when a method is invoked to display migration message. Another usage would be to denote bincompat status of the API as warning. This is a resend of #7790 based on the configurable warnings. Ref #8373 / https://twitter.com/not_xuwei_k/status/1240354073297268737 --- build.sbt | 1 + src/compiler/scala/tools/nsc/Reporting.scala | 103 +++++++++++++----- .../scala/tools/nsc/settings/Warnings.scala | 2 +- .../tools/nsc/typechecker/RefChecks.scala | 3 + src/library/scala/annotation/apiStatus.scala | 53 +++++++++ .../scala/reflect/internal/Definitions.scala | 1 + .../scala/reflect/internal/Symbols.scala | 5 + .../reflect/runtime/JavaUniverseForce.scala | 1 + test/files/neg/report-deprecated-error.check | 8 ++ test/files/neg/report-deprecated-error.scala | 31 ++++++ 10 files changed, 180 insertions(+), 28 deletions(-) create mode 100644 src/library/scala/annotation/apiStatus.scala create mode 100644 test/files/neg/report-deprecated-error.check create mode 100644 test/files/neg/report-deprecated-error.scala diff --git a/build.sbt b/build.sbt index b890da75e978..72dd803c9214 100644 --- a/build.sbt +++ b/build.sbt @@ -189,6 +189,7 @@ val mimaFilterSettings = Seq { ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.runtime.JavaUniverse#PerRunReporting.deprecationWarning"), ProblemFilters.exclude[MissingClassProblem]("scala.annotation.nowarn$"), ProblemFilters.exclude[MissingClassProblem]("scala.annotation.nowarn"), + ProblemFilters.exclude[MissingClassProblem]("scala.annotation.apiStatus*"), ProblemFilters.exclude[DirectMissingMethodProblem]("scala.reflect.runtime.Settings#*.clearSetByUser"), ////////////////////////////////////////////////////////////////////////////// Vector backward compatiblity diff --git a/src/compiler/scala/tools/nsc/Reporting.scala b/src/compiler/scala/tools/nsc/Reporting.scala index f84f76a7c21e..341706c08610 100644 --- a/src/compiler/scala/tools/nsc/Reporting.scala +++ b/src/compiler/scala/tools/nsc/Reporting.scala @@ -14,6 +14,7 @@ package scala package tools package nsc +import java.util.Locale.ENGLISH import java.util.regex.PatternSyntaxException import scala.collection.mutable @@ -104,12 +105,14 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio sm.getOrElseUpdate(category, mutable.LinkedHashMap.empty) } - private def issueWarning(warning: Message): Unit = { + private def issueWarning(warning: Message): Unit = issueWarning(warning, wconf.action(warning)) + + private def issueWarning(warning: Message, action: Action): Unit = { def verbose = warning match { case Message.Deprecation(_, msg, site, origin, version) => s"[${warning.category.name} @ $site | origin=$origin | version=${version.filterString}] $msg" case Message.Plain(_, msg, category, site) => s"[${category.name} @ $site] $msg" } - wconf.action(warning) match { + action match { case Action.Error => reporter.error(warning.pos, warning.msg) case Action.Warning => reporter.warning(warning.pos, warning.msg) case Action.WarningVerbose => reporter.warning(warning.pos, verbose) @@ -122,10 +125,12 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio } } - private def checkSuppressedAndIssue(warning: Message): Unit = { + private def checkSuppressedAndIssue(warning: Message): Unit = checkSuppressedAndIssue(warning, wconf.action(warning)) + + private def checkSuppressedAndIssue(warning: Message, action: Action): Unit = { if (suppressionsComplete) { if (!isSuppressed(warning)) - issueWarning(warning) + issueWarning(warning, action) } else suspendedMessages += warning } @@ -251,6 +256,31 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio def warning(pos: Position, msg: String, category: WarningCategory, site: Symbol): Unit = warning(pos, msg, category, siteName(site)) + // someone is using @apiStatus API + def handleApiStatus(pos: Position, sym: Symbol, site: Symbol): Unit = { + val category0 = sym.apiStatusCategory.getOrElse("unspecified") + val category = WarningCategory.parse(category0) + val message = sym.apiStatusMessage.getOrElse("") + val msg = category match { + case WarningCategory.Deprecation => + val origin = s"$sym${sym.locationString}" + val since = Version.fromString(sym.apiStatusVersion.getOrElse("")) + Message.Deprecation(pos, message, siteName(site), origin, since) + case _ => + val since = sym.apiStatusVersion match { + case Some(v) => s" ($v)" + case _ => "" + } + Message.Plain(pos, s"$message$since", category, siteName(site)) + } + val defaultAction0 = sym.apiStatusDefaultAction.getOrElse("warning") + val defaultAction = Action.parse(defaultAction0) match { + case Right(a) => a + case Left(_) => Action.Warning + } + checkSuppressedAndIssue(msg, wconf.actionOpt(msg).getOrElse(defaultAction)) + } + // used by Global.deprecationWarnings, which is used by sbt def deprecationWarnings: List[(Position, String)] = summaryMap(Action.WarningSummary, WarningCategory.Deprecation).toList.map(p => (p._1, p._2.msg)) def uncheckedWarnings: List[(Position, String)] = summaryMap(Action.WarningSummary, WarningCategory.Unchecked).toList.map(p => (p._1, p._2.msg)) @@ -302,12 +332,13 @@ object Reporting { } sealed trait WarningCategory { - lazy val name: String = { + def name: String = name0 + private[this] lazy val name0: String = { val objectName = this.getClass.getName.split('$').last WarningCategory.insertDash.replaceAllIn(objectName, "-") .stripPrefix("-") .stripSuffix("-") - .toLowerCase + .toLowerCase(ENGLISH) } def includes(o: WarningCategory): Boolean = this eq o @@ -317,8 +348,8 @@ object Reporting { object WarningCategory { private val insertDash = "(?=[A-Z][a-z])".r - var all: mutable.Map[String, WarningCategory] = mutable.Map.empty - private def add(c: WarningCategory): Unit = all += ((c.name, c)) + val builtIn: mutable.Map[String, WarningCategory] = mutable.Map.empty + private def add(c: WarningCategory): Unit = builtIn += ((c.name, c)) object Deprecation extends WarningCategory; add(Deprecation) @@ -330,6 +361,8 @@ object Reporting { object JavaSource extends WarningCategory; add(JavaSource) + object Unspecified extends WarningCategory; add(Unspecified) + sealed trait Other extends WarningCategory { override def summaryCategory: WarningCategory = Other } object Other extends Other { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Other] }; add(Other) object OtherShadowing extends Other; add(OtherShadowing) @@ -391,6 +424,20 @@ object Reporting { object FeaturePostfixOps extends Feature; add(FeaturePostfixOps) object FeatureReflectiveCalls extends Feature; add(FeatureReflectiveCalls) object FeatureMacros extends Feature; add(FeatureMacros) + + case class Custom private (override val name: String) extends WarningCategory { + override def includes(o: WarningCategory): Boolean = this == o + } + def custom(name: String): Custom = { + val n = WarningCategory.insertDash.replaceAllIn(name, "-") + .stripPrefix("-") + .stripSuffix("-") + .toLowerCase(ENGLISH) + Custom(n) + } + + def parse(category: String): WarningCategory = + WarningCategory.builtIn.getOrElse(category, WarningCategory.custom(category)) } sealed trait Version { @@ -514,17 +561,29 @@ object Reporting { object InfoSummary extends Action object InfoVerbose extends Action object Silent extends Action + + def parse(s: String): Either[String, Action] = s match { + case "error" | "e" => Right(Error) + case "warning" | "w" => Right(Warning) + case "warning-summary" | "ws" => Right(WarningSummary) + case "warning-verbose" | "wv" => Right(WarningVerbose) + case "info" | "i" => Right(Info) + case "info-summary" | "is" => Right(InfoSummary) + case "info-verbose" | "iv" => Right(InfoVerbose) + case "silent" | "s" => Right(Silent) + case _ => Left(s"unknonw action: `$s`") + } } final case class WConf(filters: List[(List[MessageFilter], Action)]) { - def action(message: Message): Action = filters.find(_._1.forall(_.matches(message))) match { - case Some((_, action)) => action - case _ => Action.Warning + def action(message: Message): Action = actionOpt(message).getOrElse(Action.Warning) + + def actionOpt(message: Message): Option[Action] = filters.find(_._1.forall(_.matches(message))) map { + case (_, action) => action } } object WConf { - import Action._ import MessageFilter._ private def regex(s: String) = @@ -538,8 +597,7 @@ object Reporting { regex(s.substring(4)).map(MessagePattern) } else if (s.startsWith("cat=")) { val cs = s.substring(4) - val c = WarningCategory.all.get(cs).map(Category) - c.toRight(s"Unknown category: `$cs`") + Right(Category(WarningCategory.parse(cs))) } else if (s.startsWith("site=")) { regex(s.substring(5)).map(SitePattern) } else if (s.startsWith("origin=")) { @@ -577,18 +635,6 @@ object Reporting { } def parse(setting: List[String], rootDir: String): Either[List[String], WConf] = { - def parseAction(s: String): Either[List[String], Action] = s match { - case "error" | "e" => Right(Error) - case "warning" | "w" => Right(Warning) - case "warning-summary" | "ws" => Right(WarningSummary) - case "warning-verbose" | "wv" => Right(WarningVerbose) - case "info" | "i" => Right(Info) - case "info-summary" | "is" => Right(InfoSummary) - case "info-verbose" | "iv" => Right(InfoVerbose) - case "silent" | "s" => Right(Silent) - case _ => Left(List(s"unknonw action: `$s`")) - } - if (setting.isEmpty) Right(WConf(Nil)) else { val parsedConfs: List[Either[List[String], (List[MessageFilter], Action)]] = setting.map(conf => { @@ -596,7 +642,10 @@ object Reporting { val (ms, fs) = parts.view.init.map(parseFilter(_, rootDir)).toList.partitionMap(identity) if (ms.nonEmpty) Left(ms) else if (fs.isEmpty) Left(List("no filters or no action defined")) - else parseAction(parts.last).map((fs, _)) + else Action.parse(parts.last) match { + case Right(a) => Right((fs, a)) + case Left(s) => Left(List(s)) + } }) val (ms, fs) = parsedConfs.partitionMap(identity) if (ms.nonEmpty) Left(ms.flatten) diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 7e97529fd22f..eebf004176a9 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -88,7 +88,7 @@ trait Warnings { | - silence certain deprecations: -Wconf:origin=some\\.lib\\..*&since>2.2:s | |Full list of message categories: - |${WarningCategory.all.keys.groupBy(_.split('-').head).toList.sortBy(_._1).map(_._2.toList.sorted.mkString(", ")).mkString(" - ", "\n - ", "")} + |${WarningCategory.builtIn.keys.groupBy(_.split('-').head).toList.sortBy(_._1).map(_._2.toList.sorted.mkString(", ")).mkString(" - ", "\n - ", "")} | |To suppress warnings locally, use the `scala.annotation.nowarn` annotation. | diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index bfab345d0d51..50e83e6c111c 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1244,6 +1244,9 @@ abstract class RefChecks extends Transform { // warnings after the first, but I think it'd be better if we didn't have to // arbitrarily choose one as more important than the other. private def checkUndesiredProperties(sym: Symbol, pos: Position): Unit = { + if (sym.isApiStatus && !currentOwner.ownerChain.exists(x => x.isApiStatus)) + currentRun.reporting.handleApiStatus(pos, sym, currentOwner) + // If symbol is deprecated, and the point of reference is not enclosed // in either a deprecated member or a scala bridge method, issue a warning. if (sym.isDeprecated && !currentOwner.ownerChain.exists(x => x.isDeprecated)) diff --git a/src/library/scala/annotation/apiStatus.scala b/src/library/scala/annotation/apiStatus.scala new file mode 100644 index 000000000000..dcad4a327749 --- /dev/null +++ b/src/library/scala/annotation/apiStatus.scala @@ -0,0 +1,53 @@ +/* + * Scala (https://www.scala-lang.org) + * + * Copyright EPFL and Lightbend, Inc. + * + * Licensed under Apache License 2.0 + * (http://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package scala.annotation + +import scala.annotation.meta._ + +/** + * An annotation to denote the API status. + * + * @param message the advisory to print during compilation + * @param category a string identifying the categorization of the restriction + * @param since a string identifying the first version in which the status is applied + * @param defaultAction the default severity of the restriction when the annotee is referenced + * @since 2.13.2 + * @see [[scala.annotation.apiStatus.Action]] + * @see [[scala.annotation.apiStatus.Category]] + */ +@getter @setter @beanGetter @beanSetter @companionClass @companionMethod +class apiStatus( + message: String, + category: String, + since: String = "", + defaultAction: String = apiStatus.Action.Warning) extends scala.annotation.StaticAnnotation + +object apiStatus { + object Action { + final val Error = "error" + final val Warning = "warning" + final val WarningSummary = "warning-summary" + final val WarningVerbose = "warning-verbose" + final val Info = "info" + final val InfoSummary = "info-summary" + final val InfoVerbose = "info-verbose" + final val Silent = "silent" + } + + object Category { + final val ForRemoval = "for-removal" + final val InternalOnly = "internal-only" + final val ApiMayChange = "api-may-change" + final val Mistake = "mistake" + } +} diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 48f789c6c811..f2bc4a58f510 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -1278,6 +1278,7 @@ trait Definitions extends api.StandardDefinitions { lazy val DeprecatedNameAttr = requiredClass[scala.deprecatedName] lazy val DeprecatedInheritanceAttr = requiredClass[scala.deprecatedInheritance] lazy val DeprecatedOverridingAttr = requiredClass[scala.deprecatedOverriding] + lazy val ApiStatusAttr = getClassIfDefined("scala.annotation.apiStatus") lazy val NativeAttr = requiredClass[scala.native] lazy val ScalaInlineClass = requiredClass[scala.inline] lazy val ScalaNoInlineClass = requiredClass[scala.noinline] diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 7029440cd4c3..98fd4656fdc5 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -913,6 +913,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => = getAnnotation(DeprecatedOverridingAttr) flatMap (_ stringArg 0) def deprecatedOverridingVersion = getAnnotation(DeprecatedOverridingAttr) flatMap (_ stringArg 1) + def isApiStatus = hasAnnotation(ApiStatusAttr) + def apiStatusMessage = getAnnotation(ApiStatusAttr) flatMap (_ stringArg 0) + def apiStatusCategory = getAnnotation(ApiStatusAttr) flatMap (_ stringArg 1) + def apiStatusVersion = getAnnotation(ApiStatusAttr) flatMap (_ stringArg 2) + def apiStatusDefaultAction = getAnnotation(ApiStatusAttr) flatMap (_ stringArg 3) // !!! when annotation arguments are not literal strings, but any sort of // assembly of strings, there is a fair chance they will turn up here not as diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 20b82f0a6e8a..f1702fc5e174 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -422,6 +422,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => definitions.DeprecatedNameAttr definitions.DeprecatedInheritanceAttr definitions.DeprecatedOverridingAttr + definitions.ApiStatusAttr definitions.NativeAttr definitions.ScalaInlineClass definitions.ScalaNoInlineClass diff --git a/test/files/neg/report-deprecated-error.check b/test/files/neg/report-deprecated-error.check new file mode 100644 index 000000000000..e96b5da75263 --- /dev/null +++ b/test/files/neg/report-deprecated-error.check @@ -0,0 +1,8 @@ +report-deprecated-error.scala:26: error: method <<= is removed; use := syntax instead (foo-lib 1.0) + <<=() + ^ +report-deprecated-error.scala:28: warning: should DSL is incubating, and future compatibility is not guaranteed (foo-lib 1.0) + "bar" should { + ^ +1 warning +1 error diff --git a/test/files/neg/report-deprecated-error.scala b/test/files/neg/report-deprecated-error.scala new file mode 100644 index 000000000000..ba0a45f6dc74 --- /dev/null +++ b/test/files/neg/report-deprecated-error.scala @@ -0,0 +1,31 @@ +import scala.annotation.apiStatus + +package foo { + object syntax { + @apiStatus( + "method <<= is removed; use := syntax instead", + category = apiStatus.Category.ForRemoval, + since = "foo-lib 1.0", + defaultAction = apiStatus.Action.Error, + ) + def <<=() = ??? + + @apiStatus( + "should DSL is incubating, and future compatibility is not guaranteed", + category = apiStatus.Category.ApiMayChange, + since = "foo-lib 1.0", + ) + implicit class ShouldDSL(s: String) { + def should(o: String): Unit = () + } + } +} + +object Test1 { + import foo.syntax._ + <<=() + + "bar" should { + "something" + } +}