Skip to content

Commit

Permalink
Make logging never repeat the same experimental warning
Browse files Browse the repository at this point in the history
  • Loading branch information
MaciejG604 committed Sep 1, 2023
1 parent 15f5d0e commit f9df159
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 31 deletions.
Expand Up @@ -15,19 +15,31 @@ object WarningMessages {
|If you encounter any bugs or have feedback to share, make sure to reach out to the maintenance team at $scalaCliGithubUrl""".stripMargin
def experimentalFeaturesUsed(namesAndFeatureTypes: Seq[(String, FeatureType)]): String = {
val message = namesAndFeatureTypes match {
case Seq((name, featureType)) => s"The $name $featureType is experimental"
case Seq((name, featureType)) => s"The `$name` $featureType is experimental"
case namesAndTypes =>
val nl = System.lineSeparator()
val bulletPointList = namesAndTypes.map((name, fType) => s" - `$name` $fType")
.mkString(nl)
s"""Some utilized features are marked as experimental:
val nl = System.lineSeparator()
val distinctFeatureTypes = namesAndTypes.map(_._2).distinct
val (bulletPointList, featureNameToPrint) = if (distinctFeatureTypes.size == 1)
(
namesAndTypes.map((name, fType) => s" - `$name`")
.mkString(nl),
s"${distinctFeatureTypes.head}s" // plural form
)
else
(
namesAndTypes.map((name, fType) => s" - `$name` $fType")
.mkString(nl),
"features"
)

s"""Some utilized $featureNameToPrint are marked as experimental:
|$bulletPointList""".stripMargin
}
s"""$message
|$experimentalNote""".stripMargin
}

def experimentalSubcommandUsed(name: String): String =
def experimentalSubcommandWarning(name: String): String =
s"""The `$name` sub-command is experimental.
|$experimentalNote""".stripMargin

Expand Down
10 changes: 5 additions & 5 deletions modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala
Expand Up @@ -289,12 +289,12 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
message =
s"""${hm.message}
|
|${WarningMessages.experimentalSubcommandUsed(name)}""".stripMargin,
|${WarningMessages.experimentalSubcommandWarning(name)}""".stripMargin,
detailedMessage =
if hm.detailedMessage.nonEmpty then
s"""${hm.detailedMessage}
|
|${WarningMessages.experimentalSubcommandUsed(name)}""".stripMargin
|${WarningMessages.experimentalSubcommandWarning(name)}""".stripMargin
else hm.detailedMessage
)
)
Expand Down Expand Up @@ -347,8 +347,6 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
* start of running every [[ScalaCommand]].
*/
final override def run(options: T, remainingArgs: RemainingArgs): Unit = {
logger.flushExperimentalWarnings

CurrentParams.verbosity = options.global.logging.verbosity
if shouldExcludeInSip then
logger.error(WarningMessages.powerCommandUsedInSip(
Expand All @@ -357,13 +355,15 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
))
sys.exit(1)
else if isExperimental && !shouldSuppressExperimentalFeatureWarnings then
logger.message(WarningMessages.experimentalSubcommandUsed(name))
logger.experimentalWarning(name, FeatureType.Subcommand)

maybePrintWarnings(options)
maybePrintGroupHelp(options)
buildOptions(options).foreach { bo =>
maybePrintSimpleScalacOutput(options, bo)
maybePrintToolsHelp(options, bo)
}
logger.flushExperimentalWarnings
runCommand(options, remainingArgs, options.global.logging.logger)
}
}
24 changes: 15 additions & 9 deletions modules/cli/src/main/scala/scala/cli/internal/CliLogger.scala
Expand Up @@ -209,22 +209,28 @@ class CliLogger(
// Allow to disable that?
def compilerOutputStream = out

private var experimentalWarnings: Map[FeatureType, Set[String]] = Map()
def experimentalWarning(featureName: String, featureType: FeatureType): Unit = {
experimentalWarnings ++= experimentalWarnings.updatedWith(featureType) {
case None => Some(Set(featureName))
case Some(namesSet) => Some(namesSet + featureName)
}
}
private var experimentalWarnings: Map[FeatureType, Set[String]] = Map.empty
private var reported: Map[FeatureType, Set[String]] = Map.empty
def experimentalWarning(featureName: String, featureType: FeatureType): Unit =
if (!reported.get(featureType).exists(_.contains(featureName)))
experimentalWarnings ++= experimentalWarnings.updatedWith(featureType) {
case None => Some(Set(featureName))
case Some(namesSet) => Some(namesSet + featureName)
}
def flushExperimentalWarnings: Unit = if (experimentalWarnings.nonEmpty) {
val messageStr = {
val messageStr: String = {
val namesAndTypes = for {
(featureType, names) <- experimentalWarnings.toSeq
(featureType, names) <- experimentalWarnings.toSeq.sortBy(_._1) // by feature type
name <- names
} yield name -> featureType
WarningMessages.experimentalFeaturesUsed(namesAndTypes)
}
message(messageStr)
reported = for {
(featureType, names) <- experimentalWarnings
reportedNames = reported.getOrElse(featureType, Set.empty[String])
} yield featureType -> (names ++ reportedNames)
experimentalWarnings = Map.empty
}
}

Expand Down
Expand Up @@ -8,3 +8,14 @@ enum FeatureType(stringRepr: String) {
case Subcommand extends FeatureType("sub-command")
case ConfigKey extends FeatureType("configuration key")
}

object FeatureType {
private val ordering = Map(
FeatureType.Subcommand -> 0,
FeatureType.Option -> 1,
FeatureType.Directive -> 2,
FeatureType.ConfigKey -> 3
)

given Ordering[FeatureType] = Ordering.by(ordering)
}
Expand Up @@ -8,13 +8,12 @@ import scala.util.Properties
class SipScalaTests extends ScalaCliSuite {

implicit class StringEnrichment(s: String) {
def containsExperimentalWarningOf(featureNameAndType: String) =
def containsExperimentalWarningOf(featureNameAndType: String): Boolean =
s.contains(s"The $featureNameAndType is experimental") ||
s.linesIterator
.dropWhile(_ != "Some utilized features are marked as experimental:")
.takeWhile(_ != "Please bear in mind that non-ideal user experience should be expected.")
.tapEach(println)
.contains(s" - $featureNameAndType")
s.linesIterator
.dropWhile(!_.endsWith("are marked as experimental:"))
.takeWhile(_ != "Please bear in mind that non-ideal user experience should be expected.")
.contains(s" - $featureNameAndType")
}

implicit class BinaryNameOps(binaryName: String) {
Expand Down Expand Up @@ -208,15 +207,15 @@ class SipScalaTests extends ScalaCliSuite {
case (true, false) =>
expect(res.exitCode == 0)
expect(errOutput.containsExperimentalWarningOf(
"`//> using publish.name \"my-library\"` directive"
"`//> using publish.name \"my-library\"`"
))
expect(errOutput.containsExperimentalWarningOf("`//> using python` directive"))
expect(errOutput.containsExperimentalWarningOf("`//> using python`"))
case (true, true) =>
expect(res.exitCode == 0)
expect(!errOutput.containsExperimentalWarningOf(
"`//> using publish.name \"my-library\"` directive"
"`//> using publish.name \"my-library\"`"
))
expect(!errOutput.containsExperimentalWarningOf("`//> using python` directive"))
expect(!errOutput.containsExperimentalWarningOf("`//> using python`"))
}
}

Expand Down Expand Up @@ -248,7 +247,9 @@ class SipScalaTests extends ScalaCliSuite {
isPowerMode -> areWarningsSuppressed match {
case (false, _) =>
expect(res.exitCode == 1)
expect(errOutput.contains("Unrecognized argument: The `--markdown` option is experimental."))
expect(
errOutput.contains("Unrecognized argument: The `--markdown` option is experimental.")
)
case (true, false) =>
expect(res.exitCode == 0)
expect(errOutput.containsExperimentalWarningOf("`--markdown` option"))
Expand Down Expand Up @@ -415,4 +416,46 @@ class SipScalaTests extends ScalaCliSuite {
}
)
}

test("test multiple sources of experimental features") {
val inputs = TestInputs(
os.rel / "Main.scala" ->
"""//> using target.scope main
|//> using target.platform jvm
|//> using publish.name "my-library"
|
|object Main {
| def main(args: Array[String]): Unit = {
| println("Hello World!")
| }
|}
|""".stripMargin
)

inputs.fromRoot { root =>
val res = os.proc(TestUtil.cli, "--power", "export", ".", "--object-wrapper", "--md")
.call(cwd = root, mergeErrIntoOut = true)

val output = res.out.trim

assertNoDiff(
output,
s"""Some utilized features are marked as experimental:
| - `export` sub-command
| - `--object-wrapper` option
| - `--md` option
|Please bear in mind that non-ideal user experience should be expected.
|If you encounter any bugs or have feedback to share, make sure to reach out to the maintenance team at https://github.com/VirtusLab/scala-cli
|Exporting to a sbt project...
|Some utilized directives are marked as experimental:
| - `//> using publish.name "my-library"`
| - `//> using target.scope "main"`
| - `//> using target.platform "jvm"`
|Please bear in mind that non-ideal user experience should be expected.
|If you encounter any bugs or have feedback to share, make sure to reach out to the maintenance team at https://github.com/VirtusLab/scala-cli
|Exported to: $root/dest
|""".stripMargin
)
}
}
}

0 comments on commit f9df159

Please sign in to comment.