From c632802804fd7b4f344a901439d9bee2985ffc24 Mon Sep 17 00:00:00 2001 From: Jason Pickens <4659562+steinybot@users.noreply.github.com> Date: Mon, 4 Jun 2018 22:14:38 +1200 Subject: [PATCH] Add warning for unknown configurations Fixes #3432 --- .../sbt/internal/util/complete/Parser.scala | 13 +- .../sbt/internal/util/complete/Parsers.scala | 2 +- .../src/test/scala/ParserTest.scala | 4 +- .../src/main/scala/sbt/BasicCommands.scala | 2 +- main/src/main/scala/sbt/Extracted.scala | 2 +- main/src/main/scala/sbt/Main.scala | 2 +- main/src/main/scala/sbt/PluginCross.scala | 2 +- .../main/scala/sbt/internal/IvyConsole.scala | 2 +- .../main/scala/sbt/internal/KeyIndex.scala | 102 +++++-- main/src/main/scala/sbt/internal/Load.scala | 27 +- main/src/main/scala/sbt/internal/Script.scala | 2 +- main/src/test/scala/ParseKey.scala | 270 ++++++++++++++++-- main/src/test/scala/ParserSpec.scala | 75 +++++ .../test/scala/sbt/internal/TestBuild.scala | 122 ++++++-- .../src/main/scala/sbt/ScriptedPlugin.scala | 2 +- 15 files changed, 536 insertions(+), 93 deletions(-) create mode 100644 main/src/test/scala/ParserSpec.scala diff --git a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parser.scala b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parser.scala index 94417e1c4b..89195e4171 100644 --- a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parser.scala +++ b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parser.scala @@ -209,7 +209,11 @@ object Parser extends ParserMain { a.ifValid { a.result match { case Some(av) => success(f(av)) - case None => new MapParser(a, f) + case None => + a match { + case m: MapParser[_, A] => m.map(f) + case _ => new MapParser(a, f) + } } } @@ -381,8 +385,8 @@ trait ParserMain { } /** Presents a Char range as a Parser. A single Char is parsed only if it is in the given range.*/ - implicit def range(r: collection.immutable.NumericRange[Char]): Parser[Char] = - charClass(r contains _).examples(r.map(_.toString): _*) + implicit def range(r: collection.immutable.NumericRange[Char], label: String): Parser[Char] = + charClass(r contains _, label).examples(r.map(_.toString): _*) /** Defines a Parser that parses a single character only if it is contained in `legal`.*/ def chars(legal: String): Parser[Char] = { @@ -394,7 +398,7 @@ trait ParserMain { * Defines a Parser that parses a single character only if the predicate `f` returns true for that character. * If this parser fails, `label` is used as the failure message. */ - def charClass(f: Char => Boolean, label: String = ""): Parser[Char] = + def charClass(f: Char => Boolean, label: String): Parser[Char] = new CharacterClass(f, label) /** Presents a single Char `ch` as a Parser that only parses that exact character. */ @@ -744,6 +748,7 @@ private final class MapParser[A, B](a: Parser[A], f: A => B) extends ValidParser def completions(level: Int) = a.completions(level) override def isTokenStart = a.isTokenStart override def toString = "map(" + a + ")" + def map[C](g: B => C) = new MapParser[A, C](a, f.andThen(g)) } private final class Filter[T](p: Parser[T], f: T => Boolean, seen: String, msg: String => String) diff --git a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala index b04b61127f..e57332178b 100644 --- a/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala +++ b/internal/util-complete/src/main/scala/sbt/internal/util/complete/Parsers.scala @@ -163,7 +163,7 @@ trait Parsers { }, "non-double-quote-backslash character") /** Matches a single character that is valid somewhere in a URI. */ - lazy val URIChar = charClass(alphanum) | chars("_-!.~'()*,;:$&+=?/[]@%#") + lazy val URIChar = charClass(alphanum, "alphanum") | chars("_-!.~'()*,;:$&+=?/[]@%#") /** Returns true if `c` is an ASCII letter or digit. */ def alphanum(c: Char) = diff --git a/internal/util-complete/src/test/scala/ParserTest.scala b/internal/util-complete/src/test/scala/ParserTest.scala index df318d3233..17ef8c3c00 100644 --- a/internal/util-complete/src/test/scala/ParserTest.scala +++ b/internal/util-complete/src/test/scala/ParserTest.scala @@ -121,8 +121,8 @@ object ParserTest extends Properties("Completing Parser") { property("repeatDep accepts two tokens") = matches(repeat, colors.toSeq.take(2).mkString(" ")) } object ParserExample { - val ws = charClass(_.isWhitespace).+ - val notws = charClass(!_.isWhitespace).+ + val ws = charClass(_.isWhitespace, "whitespace").+ + val notws = charClass(!_.isWhitespace, "not whitespace").+ val name = token("test") val options = (ws ~> token("quick" | "failed" | "new")).* diff --git a/main-command/src/main/scala/sbt/BasicCommands.scala b/main-command/src/main/scala/sbt/BasicCommands.scala index 97ed9d1d6b..c78645186e 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ b/main-command/src/main/scala/sbt/BasicCommands.scala @@ -114,7 +114,7 @@ object BasicCommands { } def multiParser(s: State): Parser[List[String]] = { - val nonSemi = token(charClass(_ != ';').+, hide = const(true)) + val nonSemi = token(charClass(_ != ';', "not ';'").+, hide = const(true)) val semi = token(';' ~> OptSpace) val part = semi flatMap (_ => matched((s.combinedParser & nonSemi) | nonSemi) <~ token(OptSpace)) diff --git a/main/src/main/scala/sbt/Extracted.scala b/main/src/main/scala/sbt/Extracted.scala index 4ed0e0c8bc..ef8affed01 100644 --- a/main/src/main/scala/sbt/Extracted.scala +++ b/main/src/main/scala/sbt/Extracted.scala @@ -144,7 +144,7 @@ final case class Extracted(structure: BuildStructure, ): State = { val appendSettings = Load.transformSettings(Load.projectScope(currentRef), currentRef.build, rootProject, settings) - val newStructure = Load.reapply(sessionSettings ++ appendSettings, structure) + val newStructure = Load.reapply(sessionSettings ++ appendSettings, structure, state.log) Project.setProject(session, newStructure, state) } } diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index 65b436be05..cbf062c3a1 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -410,7 +410,7 @@ object BuiltinCommands { val loggerInject = LogManager.settingsLogger(s) val withLogger = newSession.appendRaw(loggerInject :: Nil) val show = Project.showContextKey(newSession, structure) - val newStructure = Load.reapply(withLogger.mergeSettings, structure)(show) + val newStructure = Load.reapply(withLogger.mergeSettings, structure, s.log)(show) Project.setProject(newSession, newStructure, s) } diff --git a/main/src/main/scala/sbt/PluginCross.scala b/main/src/main/scala/sbt/PluginCross.scala index 91e823bbc3..418d1e2ca9 100644 --- a/main/src/main/scala/sbt/PluginCross.scala +++ b/main/src/main/scala/sbt/PluginCross.scala @@ -50,7 +50,7 @@ private[sbt] object PluginCross { Seq(scalaVersion := scalaVersionSetting.value) ) val cleared = session.mergeSettings.filterNot(crossExclude) - val newStructure = Load.reapply(cleared ++ add, structure) + val newStructure = Load.reapply(cleared ++ add, structure, state.log) Project.setProject(session, newStructure, command :: state) } } diff --git a/main/src/main/scala/sbt/internal/IvyConsole.scala b/main/src/main/scala/sbt/internal/IvyConsole.scala index 9cce3a74f6..dd8ce831f8 100644 --- a/main/src/main/scala/sbt/internal/IvyConsole.scala +++ b/main/src/main/scala/sbt/internal/IvyConsole.scala @@ -55,7 +55,7 @@ object IvyConsole { rootProject, depSettings) - val newStructure = Load.reapply(session.original ++ append, structure) + val newStructure = Load.reapply(session.original ++ append, structure, state.log) val newState = state.copy(remainingCommands = Exec(Keys.consoleQuick.key.label, None) :: Nil) Project.setProject(session, newStructure, newState) } diff --git a/main/src/main/scala/sbt/internal/KeyIndex.scala b/main/src/main/scala/sbt/internal/KeyIndex.scala index 78183984e6..79480f39e4 100644 --- a/main/src/main/scala/sbt/internal/KeyIndex.scala +++ b/main/src/main/scala/sbt/internal/KeyIndex.scala @@ -35,11 +35,15 @@ object KeyIndex { } yield { val data = ids map { id => val configs = configurations.getOrElse(id, Seq()) - Option(id) -> new ConfigIndex(Map.empty, configs.map(c => (c.name, c.id)).toMap) + val namedConfigs = configs.map { config => + (config.name, ConfigData(Some(config.id), emptyAKeyIndex)) + }.toMap + val inverse = namedConfigs.map((ConfigIndex.invert _).tupled) + Option(id) -> new ConfigIndex(namedConfigs, inverse, emptyAKeyIndex) } Option(uri) -> new ProjectIndex(data.toMap) } - new KeyIndex0(new BuildIndex(data.toMap)) + new KeyIndex0(new BuildIndex(data)) } def combine(indices: Seq[KeyIndex]): KeyIndex = new KeyIndex { @@ -55,6 +59,7 @@ object KeyIndex { case Some(idx) => idx.fromConfigIdent(proj)(configIdent) case _ => Scope.unguessConfigIdent(configIdent) } + private[sbt] def guessedConfigIdents = concat(_.guessedConfigIdents) def tasks(proj: Option[ResolvedReference], conf: Option[String]) = concat(_.tasks(proj, conf)) def tasks(proj: Option[ResolvedReference], conf: Option[String], key: String) = concat(_.tasks(proj, conf, key)) @@ -68,7 +73,7 @@ object KeyIndex { private[sbt] def getOr[A, B](m: Map[A, B], key: A, or: B): B = m.getOrElse(key, or) private[sbt] def keySet[A, B](m: Map[Option[A], B]): Set[A] = m.keys.flatten.toSet private[sbt] val emptyAKeyIndex = new AKeyIndex(Relation.empty) - private[sbt] val emptyConfigIndex = new ConfigIndex(Map.empty, Map.empty) + private[sbt] val emptyConfigIndex = new ConfigIndex(Map.empty, Map.empty, emptyAKeyIndex) private[sbt] val emptyProjectIndex = new ProjectIndex(Map.empty) private[sbt] val emptyBuildIndex = new BuildIndex(Map.empty) } @@ -97,6 +102,7 @@ trait KeyIndex { task: Option[AttributeKey[_]]): Set[String] private[sbt] def configIdents(project: Option[ResolvedReference]): Set[String] private[sbt] def fromConfigIdent(proj: Option[ResolvedReference])(configIdent: String): String + private[sbt] def guessedConfigIdents: Set[(Option[ProjectReference], String, String)] } trait ExtendableKeyIndex extends KeyIndex { def add(scoped: ScopedKey[_]): ExtendableKeyIndex @@ -109,40 +115,67 @@ private[sbt] final class AKeyIndex(val data: Relation[Option[AttributeKey[_]], S def keys(task: Option[AttributeKey[_]]): Set[String] = data.forward(task) def allKeys: Set[String] = data._2s.toSet def tasks: Set[AttributeKey[_]] = data._1s.flatten.toSet - def tasks(key: String): Set[AttributeKey[_]] = data.reverse(key).flatten.toSet + def tasks(key: String): Set[AttributeKey[_]] = data.reverse(key).flatten } +private[sbt] case class IdentifiableConfig(name: String, ident: Option[String]) + +private[sbt] case class ConfigData(ident: Option[String], keys: AKeyIndex) + /* - * data contains the mapping between a configuration and keys. - * identData contains the mapping between a configuration and its identifier. + * data contains the mapping between a configuration name and its ident and keys. + * noConfigKeys contains the keys without a configuration. */ -private[sbt] final class ConfigIndex(val data: Map[Option[String], AKeyIndex], - val identData: Map[String, String]) { - def add(config: Option[String], +private[sbt] final class ConfigIndex(val data: Map[String, ConfigData], + val inverse: Map[String, String], + val noConfigKeys: AKeyIndex) { + def add(config: Option[IdentifiableConfig], task: Option[AttributeKey[_]], key: AttributeKey[_]): ConfigIndex = { - new ConfigIndex(data updated (config, keyIndex(config).add(task, key)), this.identData) + config match { + case Some(c) => addKeyWithConfig(c, task, key) + case None => addKeyWithoutConfig(task, key) + } + } + + def addKeyWithConfig(config: IdentifiableConfig, + task: Option[AttributeKey[_]], + key: AttributeKey[_]): ConfigIndex = { + val oldConfigData = data.getOrElse(config.name, ConfigData(None, emptyAKeyIndex)) + val newConfigData = ConfigData( + ident = oldConfigData.ident.orElse(config.ident), + keys = oldConfigData.keys.add(task, key) + ) + val newData = data.updated(config.name, newConfigData) + val newInverse = (inverse.updated _).tupled(ConfigIndex.invert(config.name, newConfigData)) + new ConfigIndex(newData, newInverse, noConfigKeys) } - def keyIndex(conf: Option[String]): AKeyIndex = getOr(data, conf, emptyAKeyIndex) - def configs: Set[String] = keySet(data) + def addKeyWithoutConfig(task: Option[AttributeKey[_]], key: AttributeKey[_]): ConfigIndex = { + new ConfigIndex(data, inverse, noConfigKeys.add(task, key)) + } - private[sbt] val configIdentsInverse: Map[String, String] = - identData map { _.swap } + def keyIndex(conf: Option[String]): AKeyIndex = conf match { + case Some(c) => data.get(c).map(_.keys).getOrElse(emptyAKeyIndex) + case None => noConfigKeys + } - private[sbt] lazy val idents: Set[String] = - configs map { config => - identData.getOrElse(config, Scope.guessConfigIdent(config)) - } + def configs: Set[String] = data.keySet // guess Configuration name from an identifier. // There's a guessing involved because we could have scoped key that Project is not aware of. private[sbt] def fromConfigIdent(ident: String): String = - configIdentsInverse.getOrElse(ident, Scope.unguessConfigIdent(ident)) + inverse.getOrElse(ident, Scope.unguessConfigIdent(ident)) +} +private[sbt] object ConfigIndex { + def invert(name: String, data: ConfigData): (String, String) = data match { + case ConfigData(Some(ident), _) => ident -> name + case ConfigData(None, _) => Scope.guessConfigIdent(name) -> name + } } private[sbt] final class ProjectIndex(val data: Map[Option[String], ConfigIndex]) { def add(id: Option[String], - config: Option[String], + config: Option[IdentifiableConfig], task: Option[AttributeKey[_]], key: AttributeKey[_]): ProjectIndex = new ProjectIndex(data updated (id, confIndex(id).add(config, task, key))) @@ -152,7 +185,7 @@ private[sbt] final class ProjectIndex(val data: Map[Option[String], ConfigIndex] private[sbt] final class BuildIndex(val data: Map[Option[URI], ProjectIndex]) { def add(build: Option[URI], project: Option[String], - config: Option[String], + config: Option[IdentifiableConfig], task: Option[AttributeKey[_]], key: AttributeKey[_]): BuildIndex = new BuildIndex(data updated (build, projectIndex(build).add(project, config, task, key))) @@ -169,11 +202,30 @@ private[sbt] final class KeyIndex0(val data: BuildIndex) extends ExtendableKeyIn def configs(project: Option[ResolvedReference]): Set[String] = confIndex(project).configs private[sbt] def configIdents(project: Option[ResolvedReference]): Set[String] = - confIndex(project).idents + confIndex(project).configs private[sbt] def fromConfigIdent(proj: Option[ResolvedReference])(configIdent: String): String = confIndex(proj).fromConfigIdent(configIdent) + private[sbt] def guessedConfigIdents: Set[(Option[ProjectReference], String, String)] = { + val guesses = for { + (build, projIndex) <- data.data + (project, confIndex) <- projIndex.data + (config, data) <- confIndex.data + if data.ident.isEmpty && !Scope.configIdents.contains(config) + } yield (projRef(build, project), config, Scope.guessConfigIdent(config)) + guesses.toSet + } + + private def projRef(build: Option[URI], project: Option[String]): Option[ProjectReference] = { + (build, project) match { + case (Some(uri), Some(proj)) => Some(ProjectRef(uri, proj)) + case (Some(uri), None) => Some(RootProject(uri)) + case (None, Some(proj)) => Some(LocalProject(proj)) + case (None, None) => None + } + } + def tasks(proj: Option[ResolvedReference], conf: Option[String]): Set[AttributeKey[_]] = keyIndex(proj, conf).tasks def tasks(proj: Option[ResolvedReference], @@ -221,6 +273,8 @@ private[sbt] final class KeyIndex0(val data: BuildIndex) extends ExtendableKeyIn id: Option[String], config: ScopeAxis[ConfigKey], task: ScopeAxis[AttributeKey[_]], - key: AttributeKey[_]): ExtendableKeyIndex = - new KeyIndex0(data.add(uri, id, config.toOption.map(_.name), task.toOption, key)) + key: AttributeKey[_]): ExtendableKeyIndex = { + val keyConfig = config.toOption.map(c => IdentifiableConfig(c.name, None)) + new KeyIndex0(data.add(uri, id, keyConfig, task.toOption, key)) + } } diff --git a/main/src/main/scala/sbt/internal/Load.scala b/main/src/main/scala/sbt/internal/Load.scala index c6264f6719..3c06256fa7 100755 --- a/main/src/main/scala/sbt/internal/Load.scala +++ b/main/src/main/scala/sbt/internal/Load.scala @@ -269,7 +269,7 @@ private[sbt] object Load { } Project.checkTargets(data) foreach sys.error val index = timed("Load.apply: structureIndex", log) { - structureIndex(data, settings, loaded.extra(data), projects) + structureIndex(data, settings, loaded.extra(data), projects, log) } val streams = timed("Load.apply: mkStreams", log) { mkStreams(projects, loaded.root, data) } val bs = new BuildStructure( @@ -322,7 +322,8 @@ private[sbt] object Load { data: Settings[Scope], settings: Seq[Setting[_]], extra: KeyIndex => BuildUtil[_], - projects: Map[URI, LoadedBuildUnit] + projects: Map[URI, LoadedBuildUnit], + log: Logger ): StructureIndex = { val keys = Index.allKeys(settings) val attributeKeys = Index.attributeKeys(data) ++ keys.map(_.key) @@ -331,6 +332,7 @@ private[sbt] object Load { val configsMap: Map[String, Seq[Configuration]] = projects.values.flatMap(bu => bu.defined map { case (k, v) => (k, v.configurations) }).toMap val keyIndex = KeyIndex(scopedKeys.toVector, projectsMap, configsMap) + checkConfigurations(keyIndex, log) val aggIndex = KeyIndex.aggregate(scopedKeys.toVector, extra(keyIndex), projectsMap, configsMap) new StructureIndex( Index.stringToKeyMap(attributeKeys), @@ -341,15 +343,32 @@ private[sbt] object Load { ) } + private def checkConfigurations(keyIndex: KeyIndex, log: Logger): Unit = { + keyIndex.guessedConfigIdents + .collect { + // Filter out any global configurations since we don't have a way of fixing them. + // Chances are this is only going to be the Test configuration which will have guessed correctly. + case (Some(projectRef), config, guess) => + (Reference.display(projectRef), config, guess) + } + .foreach { + case (project, config, guess) => + log.warn( + s"""The project $project references an unknown configuration "$config" and was guessed to be "$guess".""") + log.warn("This configuration should be explicitly added to the project.") + } + } + // Reevaluates settings after modifying them. Does not recompile or reload any build components. def reapply( newSettings: Seq[Setting[_]], - structure: BuildStructure + structure: BuildStructure, + log: Logger )(implicit display: Show[ScopedKey[_]]): BuildStructure = { val transformed = finalTransforms(newSettings) val newData = Def.make(transformed)(structure.delegates, structure.scopeLocal, display) def extra(index: KeyIndex) = BuildUtil(structure.root, structure.units, index, newData) - val newIndex = structureIndex(newData, transformed, extra, structure.units) + val newIndex = structureIndex(newData, transformed, extra, structure.units, log) val newStreams = mkStreams(structure.units, structure.root, newData) new BuildStructure( units = structure.units, diff --git a/main/src/main/scala/sbt/internal/Script.scala b/main/src/main/scala/sbt/internal/Script.scala index addf7d4ed3..77cb5c77ed 100644 --- a/main/src/main/scala/sbt/internal/Script.scala +++ b/main/src/main/scala/sbt/internal/Script.scala @@ -60,7 +60,7 @@ object Script { rootProject, scriptSettings ++ embeddedSettings) - val newStructure = Load.reapply(session.original ++ append, structure) + val newStructure = Load.reapply(session.original ++ append, structure, state.log) val arguments = state.remainingCommands.drop(1).map(e => s""""${e.commandLine}"""") val newState = arguments.mkString("run ", " ", "") :: state.copy(remainingCommands = Nil) Project.setProject(session, newStructure, newState) diff --git a/main/src/test/scala/ParseKey.scala b/main/src/test/scala/ParseKey.scala index 2d6991af7f..5a419d546c 100644 --- a/main/src/test/scala/ParseKey.scala +++ b/main/src/test/scala/ParseKey.scala @@ -7,15 +7,17 @@ package sbt -import Def.{ displayFull, displayMasked, ScopedKey } -import sbt.internal.{ TestBuild, Resolve } -import TestBuild._ -import sbt.internal.util.complete._ +import java.net.URI +import org.scalacheck.Gen._ +import org.scalacheck.Prop._ import org.scalacheck._ -import Gen._ -import Prop._ -import Arbitrary.arbBool +import sbt.Def.{ScopedKey, displayFull, displayMasked} +import sbt.internal.TestBuild._ +import sbt.internal.util.AttributeKey +import sbt.internal.util.complete._ +import sbt.internal.{Resolve, TestBuild} +import sbt.librarymanagement.Configuration /** * Tests that the scoped key parser in Act can correctly parse a ScopedKey converted by Def.show*Key. @@ -28,8 +30,8 @@ object ParseKey extends Properties("Key parser test") { implicit val gstructure = genStructure property("An explicitly specified axis is always parsed to that explicit value") = - forAllNoShrink(structureDefinedKey) { (skm: StructureKeyMask) => - import skm.{ structure, key, mask => mask0 } + forAll(structureDefinedKey) { (skm: StructureKeyMask) => + import skm.{key, structure, mask => mask0} val hasZeroConfig = key.scope.config == Zero val mask = if (hasZeroConfig) mask0.copy(project = true) else mask0 @@ -43,8 +45,8 @@ object ParseKey extends Properties("Key parser test") { } property("An unspecified project axis resolves to the current project") = - forAllNoShrink(structureDefinedKey) { (skm: StructureKeyMask) => - import skm.{ structure, key } + forAll(structureDefinedKey) { (skm: StructureKeyMask) => + import skm.{key, structure} val mask = skm.mask.copy(project = false) val string = displayMasked(key, mask) @@ -61,9 +63,9 @@ object ParseKey extends Properties("Key parser test") { } } - property("An unspecified task axis resolves to Zero") = forAllNoShrink(structureDefinedKey) { + property("An unspecified task axis resolves to Zero") = forAll(structureDefinedKey) { (skm: StructureKeyMask) => - import skm.{ structure, key } + import skm.{key, structure} val mask = skm.mask.copy(task = false) val string = displayMasked(key, mask) @@ -77,8 +79,8 @@ object ParseKey extends Properties("Key parser test") { property( "An unspecified configuration axis resolves to the first configuration directly defining the key or else Zero") = - forAllNoShrink(structureDefinedKey) { (skm: StructureKeyMask) => - import skm.{ structure, key } + forAll(structureDefinedKey) { (skm: StructureKeyMask) => + import skm.{key, structure} val mask = ScopeMask(config = false) val string = displayMasked(key, mask) val resolvedConfig = Resolve.resolveConfig(structure.extra, key.key, mask)(key.scope).config @@ -102,9 +104,32 @@ object ParseKey extends Properties("Key parser test") { def structureKeyMask(genKey: Structure => Gen[ScopedKey[_]])( implicit maskGen: Gen[ScopeMask], structureGen: Gen[Structure]): Gen[StructureKeyMask] = - for (mask <- maskGen; structure <- structureGen; key <- genKey(structure)) - yield new StructureKeyMask(structure, key, mask) - final class StructureKeyMask(val structure: Structure, val key: ScopedKey[_], val mask: ScopeMask) + for { + mask <- maskGen + structure <- structureGen + key <- genKey(structure) + skm = StructureKeyMask(structure, key, mask) + if configExistsInIndex(skm) + } yield skm + final case class StructureKeyMask(structure: Structure, key: ScopedKey[_], mask: ScopeMask) + + private def configExistsInIndex(skm: StructureKeyMask): Boolean = { + import skm._ + val resolvedKey = resolve(structure, key, mask) + val proj = resolvedKey.scope.project.toOption + val maybeResolvedProj = proj.collect { + case ref: ResolvedReference => ref + } + val checkName = for { + configKey <- resolvedKey.scope.config.toOption + } yield { + val configID = Scope.display(configKey) + // This only works for known configurations or those that were guessed correctly. + val name = structure.keyIndex.fromConfigIdent(maybeResolvedProj)(configID) + name == configKey.name + } + checkName.getOrElse(true) + } def resolve(structure: Structure, key: ScopedKey[_], mask: ScopeMask): ScopedKey[_] = ScopedKey(Resolve(structure.extra, Select(structure.current), key.key, mask)(key.scope), @@ -139,14 +164,17 @@ object ParseKey extends Properties("Key parser test") { // from TestBuild. def genStructure(implicit mkEnv: Gen[Env]): Gen[Structure] = structureGenF { (scopes: Seq[Scope], env: Env, current: ProjectRef) => - val settings = - for { - scope <- scopes - t <- env.tasks - } yield Def.setting(ScopedKey(scope, t.key), Def.value("")) + val settings = structureSettings(scopes, env) TestBuild.structure(env, settings, current) } + def structureSettings(scopes: Seq[Scope], env: Env): Seq[Def.Setting[String]] = { + for { + scope <- scopes + t <- env.tasks + } yield Def.setting(ScopedKey(scope, t.key), Def.value("")) + } + // Here we're shadowing the in-scope implicit called `mkEnv` for this method // so that it will use the passed-in `Gen` rather than the one imported // from TestBuild. @@ -170,4 +198,200 @@ object ParseKey extends Properties("Key parser test") { // The rest of the tests expect at least one item, so I changed it to return 1 in case of 0. def pickN[T](load: Double, from: Seq[T]): Gen[Seq[T]] = pick(Math.max((load * from.size).toInt, 1), from) + + implicit val shrinkStructureKeyMask: Shrink[StructureKeyMask] = Shrink { skm => + Shrink + .shrink(skm.structure) + .map(s => skm.copy(structure = s)) + .flatMap(fixKey) + } + + def fixKey(skm: StructureKeyMask): Stream[StructureKeyMask] = { + for { + scope <- fixScope(skm) + attributeKey <- fixAttributeKey(skm) + } yield skm.copy(key = ScopedKey(scope, attributeKey)) + } + + def fixScope(skm: StructureKeyMask): Stream[Scope] = { + def validScope(scope: Scope) = scope match { + case Scope(Select(BuildRef(build)), _, _, _) if !validBuild(build) => false + case Scope(Select(ProjectRef(build, project)), _, _, _) if !validProject(build, project) => + false + case Scope(Select(ProjectRef(build, project)), Select(ConfigKey(config)), _, _) + if !validConfig(build, project, config) => + false + case Scope(_, Select(ConfigKey(config)), _, _) if !configExists(config) => + false + case Scope(_, _, Select(task), _) => validTask(task) + case _ => true + } + def validBuild(build: URI) = skm.structure.env.buildMap.contains(build) + def validProject(build: URI, project: String) = { + skm.structure.env.buildMap + .get(build) + .exists(_.projectMap.contains(project)) + } + def validConfig(build: URI, project: String, config: String) = { + skm.structure.env.buildMap + .get(build) + .toSeq + .flatMap(_.projectMap.get(project)) + .flatMap(_.configurations.map(_.name)) + .contains(config) + } + def configExists(config: String) = { + val configs = for { + build <- skm.structure.env.builds + project <- build.projects + config <- project.configurations + } yield config.name + configs.contains(config) + } + def validTask(task: AttributeKey[_]) = skm.structure.env.taskMap.contains(task) + if (validScope(skm.key.scope)) { + Stream(skm.key.scope) + } else { + // We could return all scopes here but we want to explore the other paths first since there + // is a greater chance of a successful shrink. If necessary these could be appended to the end. + Stream.empty + } + } + + def fixAttributeKey(skm: StructureKeyMask): Stream[AttributeKey[_]] = { + if (skm.structure.allAttributeKeys.contains(skm.key.key)) { + Stream(skm.key.key) + } else { + // Likewise here, we should try other paths before trying different attribute keys. + Stream.empty + } + } + + implicit val shrinkStructure: Shrink[Structure] = Shrink { s => + Shrink.shrink(s.env).flatMap { env => + val scopes = s.data.scopes intersect env.allFullScopes.toSet + val settings = structureSettings(scopes.toSeq, env) + if (settings.nonEmpty) { + val currents = env.allProjects.find { + case (ref, _) => ref == s.current + } match { + case Some((current, _)) => Stream(current) + case None => env.allProjects.map(_._1).toStream + } + currents.map(structure(env, settings, _)) + } else { + Stream.empty + } + } + } + + implicit val shrinkEnv: Shrink[Env] = Shrink { env => + val shrunkBuilds = Shrink + .shrink(env.builds) + .filter(_.nonEmpty) + .map(b => env.copy(builds = b)) + .map(fixProjectRefs) + .map(fixConfigurations) + val shrunkTasks = shrinkTasks(env.tasks) + .map(t => env.copy(tasks = t)) + shrunkBuilds ++ shrunkTasks + } + + private def fixProjectRefs(env: Env): Env = { + def fixBuild(build: Build): Build = { + build.copy(projects = build.projects.map(fixProject)) + } + def fixProject(project: Proj): Proj = { + project.copy(delegates = project.delegates.filter(delegateExists)) + } + def delegateExists(delegate: ProjectRef): Boolean = { + env.buildMap + .get(delegate.build) + .flatMap(_.projectMap.get(delegate.project)) + .nonEmpty + } + env.copy(builds = env.builds.map(fixBuild)) + } + + private def fixConfigurations(env: Env): Env = { + val configs = env.allProjects.map { + case (_, proj) => proj -> proj.configurations.toSet + }.toMap + + def fixBuild(build: Build): Build = { + build.copy(projects = build.projects.map(fixProject(build.uri))) + } + def fixProject(buildURI: URI)(project: Proj): Proj = { + val projConfigs = configs(project) + project.copy(configurations = project.configurations.map(fixConfig(projConfigs))) + } + def fixConfig(projConfigs: Set[Configuration])(config: Configuration): Configuration = { + import config.{name => configName, _} + val extendsConfigs = config.extendsConfigs.filter(projConfigs.contains) + Configuration.of(id, configName, description, isPublic, extendsConfigs, transitive) + } + env.copy(builds = env.builds.map(fixBuild)) + } + + implicit val shrinkBuild: Shrink[Build] = Shrink { build => + Shrink + .shrink(build.projects) + .filter(_.nonEmpty) + .map(p => build.copy(projects = p)) + // Could also shrink the URI here but that requires updating all the references. + } + + implicit val shrinkProject: Shrink[Proj] = Shrink { project => + val shrunkDelegates = Shrink + .shrink(project.delegates) + .map(d => project.copy(delegates = d)) + val shrunkConfigs = Shrink + .shrink(project.configurations) + .map(c => project.copy(configurations = c)) + val shrunkID = shrinkID(project.id) + .map(id => project.copy(id = id)) + shrunkDelegates ++ shrunkConfigs ++ shrunkID + } + + implicit val shrinkDelegate: Shrink[ProjectRef] = Shrink { delegate => + val shrunkBuild = Shrink + .shrink(delegate.build) + .map(b => delegate.copy(build = b)) + val shrunkProject = Shrink + .shrink(delegate.project) + .map(p => delegate.copy(project = p)) + shrunkBuild ++ shrunkProject + } + + implicit val shrinkConfiguration: Shrink[Configuration] = Shrink { configuration => + import configuration.{name => configName, _} + val shrunkExtends = Shrink + .shrink(configuration.extendsConfigs) + .map(configuration.withExtendsConfigs) + val shrunkID = Shrink.shrink(id.tail).map { tail => + Configuration + .of(id.head + tail, configName, description, isPublic, extendsConfigs, transitive) + } + shrunkExtends ++ shrunkID + } + + val shrinkStringLength: Shrink[String] = Shrink { s => + // Only change the string length don't change the characters. + implicit val shrinkChar: Shrink[Char] = Shrink.shrinkAny + Shrink.shrinkContainer[List, Char].shrink(s.toList).map(_.mkString) + } + + def shrinkID(id: String): Stream[String] = { + Shrink.shrink(id).filter(DefaultParsers.validID) + } + + def shrinkTasks(tasks: Vector[Taskk]): Stream[Vector[Taskk]] = { + Shrink.shrink(tasks) + } + + implicit val shrinkTask: Shrink[Taskk] = Shrink { task => + Shrink.shrink((task.delegates, task.key)).map { + case (delegates, key) => Taskk(key, delegates) + } + } } diff --git a/main/src/test/scala/ParserSpec.scala b/main/src/test/scala/ParserSpec.scala new file mode 100644 index 0000000000..0eaf064286 --- /dev/null +++ b/main/src/test/scala/ParserSpec.scala @@ -0,0 +1,75 @@ +/* + * sbt + * Copyright 2011 - 2017, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under BSD-3-Clause license (see LICENSE) + */ + +import java.net.URI + +import org.scalatest.prop.PropertyChecks +import org.scalatest.{ Matchers, PropSpec } +import sbt.Def._ +import sbt._ +import sbt.internal.TestBuild +import sbt.internal.TestBuild._ +import sbt.internal.util.AttributeKey +import sbt.internal.util.complete.DefaultParsers +import sbt.librarymanagement.Configuration + +class ParserSpec extends PropSpec with PropertyChecks with Matchers { + + property("can parse any build") { + forAll(TestBuild.uriGen) { uri => + parse(buildURI = uri) + } + } + + property("can parse any project") { + forAll(TestBuild.idGen) { id => + parse(projectID = id) + } + } + + property("can parse any configuration") { + forAll(TestBuild.scalaIDGen) { name => + parse(configName = name) + } + } + + property("can parse any attribute") { + forAll(TestBuild.lowerIDGen) { name => + parse(attributeName = name) + } + } + + private def parse(buildURI: URI = new java.net.URI("s", "p", null), + projectID: String = "p", + configName: String = "c", + attributeName: String = "a") = { + val attributeKey = AttributeKey[String](attributeName) + val scope = Scope( + Select(BuildRef(buildURI)), + Select(ConfigKey(configName)), + Select(attributeKey), + Zero + ) + val scopedKey = ScopedKey(scope, attributeKey) + val config = Configuration.of(configName.capitalize, configName) + val project = Proj(projectID, Nil, Seq(config)) + val projects = Vector(project) + val build = Build(buildURI, projects) + val builds = Vector(build) + val task = Taskk(attributeKey, Nil) + val tasks = Vector(task) + val env = Env(builds, tasks) + val settings = env.tasks.map { t => + Def.setting(ScopedKey(scope, t.key), Def.value("value")) + } + val structure = TestBuild.structure(env, settings, build.allProjects.head._1) + val string = displayMasked(scopedKey, ScopeMask()) + val parser = makeParser(structure) + val result = DefaultParsers.result(parser, string).left.map(_().toString) + result shouldBe Right(scopedKey) + } +} diff --git a/main/src/test/scala/sbt/internal/TestBuild.scala b/main/src/test/scala/sbt/internal/TestBuild.scala index 7b53b1c7c6..1b9ca10705 100644 --- a/main/src/test/scala/sbt/internal/TestBuild.scala +++ b/main/src/test/scala/sbt/internal/TestBuild.scala @@ -39,8 +39,8 @@ abstract class TestBuild { def chooseShrinkable(min: Int, max: Int): Gen[Int] = sized(sz => choose(min, (max min sz) max 1)) - implicit val cGen = Arbitrary { genConfigs(idGen, MaxDepsGen, MaxConfigsGen) } - implicit val tGen = Arbitrary { genTasks(idGen, MaxDepsGen, MaxTasksGen) } + implicit val cGen = Arbitrary { genConfigs(scalaIDGen, MaxDepsGen, MaxConfigsGen) } + implicit val tGen = Arbitrary { genTasks(lowerIDGen, MaxDepsGen, MaxTasksGen) } val seed = rng.Seed.random final class Keys(val env: Env, val scopes: Seq[Scope]) { @@ -113,7 +113,7 @@ abstract class TestBuild { (taskAxes, zero.toSet, single.toSet, multi.toSet) } } - final class Env(val builds: Vector[Build], val tasks: Vector[Taskk]) { + final case class Env(builds: Vector[Build], tasks: Vector[Taskk]) { override def toString = "Env:\n " + " Tasks:\n " + tasks.mkString("\n ") + "\n" + builds.mkString("\n ") val root = builds.head @@ -154,7 +154,7 @@ abstract class TestBuild { } def getKey: Taskk => AttributeKey[_] = _.key def toConfigKey: Configuration => ConfigKey = c => ConfigKey(c.name) - final class Build(val uri: URI, val projects: Seq[Proj]) { + final case class Build(uri: URI, projects: Seq[Proj]) { override def toString = "Build " + uri.toString + " :\n " + projects.mkString("\n ") val allProjects = projects map { p => (ProjectRef(uri, p.id), p) @@ -162,10 +162,10 @@ abstract class TestBuild { val root = projects.head val projectMap = mapBy(projects)(_.id) } - final class Proj( - val id: String, - val delegates: Seq[ProjectRef], - val configurations: Seq[Configuration] + final case class Proj( + id: String, + delegates: Seq[ProjectRef], + configurations: Seq[Configuration] ) { override def toString = "Project " + id + "\n Delegates:\n " + delegates.mkString("\n ") + @@ -173,7 +173,7 @@ abstract class TestBuild { val confMap = mapBy(configurations)(_.name) } - final class Taskk(val key: AttributeKey[String], val delegates: Seq[Taskk]) { + final case class Taskk(key: AttributeKey[String], delegates: Seq[Taskk]) { override def toString = key.label + " (delegates: " + delegates.map(_.key.label).mkString(", ") + ")" } @@ -229,19 +229,17 @@ abstract class TestBuild { val keyMap = keys.map(k => (k.key.label, k.key)).toMap[String, AttributeKey[_]] val projectsMap = env.builds.map(b => (b.uri, b.projects.map(_.id).toSet)).toMap val confs = for { - b <- env.builds.toVector - p <- b.projects.toVector - c <- p.configurations.toVector - } yield c - val confMap = confs.map(c => (c.name, Seq(c))).toMap - new Structure(env, current, data, KeyIndex(keys, projectsMap, confMap), keyMap) + b <- env.builds + p <- b.projects + } yield p.id -> p.configurations + val confMap = confs.toMap + Structure(env, current, data, KeyIndex(keys, projectsMap, confMap), keyMap) } implicit lazy val mkEnv: Gen[Env] = { - implicit val cGen = genConfigs(idGen, MaxDepsGen, MaxConfigsGen) - implicit val tGen = genTasks(idGen, MaxDepsGen, MaxTasksGen) - implicit val pGen = (uri: URI) => genProjects(uri)(idGen, MaxDepsGen, MaxProjectsGen, cGen) - envGen(buildGen(uriGen, pGen), tGen) + implicit val pGen = (uri: URI) => + genProjects(uri)(idGen, MaxDepsGen, MaxProjectsGen, cGen.arbitrary) + envGen(buildGen(uriGen, pGen), tGen.arbitrary) } implicit def maskGen(implicit arbBoolean: Arbitrary[Boolean]): Gen[ScopeMask] = { @@ -250,18 +248,86 @@ abstract class TestBuild { yield ScopeMask(project = p, config = c, task = t, extra = x) } - implicit lazy val idGen: Gen[String] = + val allChars: Seq[Char] = ((0x0000 to 0xD7FF) ++ (0xE000 to 0xFFFD)).map(_.toChar) + + val letters: Seq[Char] = allChars.filter(_.isLetter) + + val upperLetters: Gen[Char] = Gen.oneOf(letters.filter(_.isUpper)) + + val lowerLetters: Gen[Char] = Gen.oneOf(letters.filter(_.isLower)) + + val lettersAndDigits: Gen[Char] = Gen.oneOf(allChars.filter(_.isLetterOrDigit)) + + val scalaIDCharGen: Gen[Char] = { + val others = Gen.const('_') + frequency(19 -> lettersAndDigits, 1 -> others) + } + + val idCharGen: Gen[Char] = { + val others = Gen.const('-') + frequency(19 -> scalaIDCharGen, 1 -> others) + } + + def isIDChar(c: Char) = { + c.isLetterOrDigit || "-_".toSeq.contains(c) + } + + // TODO: Deduplicate + val idGen: Gen[String] = { for { size <- chooseShrinkable(1, MaxIDSize) - cs <- listOfN(size, alphaChar) - } yield { - val xs = cs.mkString - xs.take(1).toLowerCase + xs.drop(1) - } + idStart <- upperLetters + idEnd <- listOfN(size - 1, idCharGen) + } yield idStart + idEnd.mkString + } filter { id => + // The filter ensure that shrinking works + id.headOption.exists(_.isUpper) && id.tail.forall(isIDChar) + } + + val lowerIDGen: Gen[String] = { + for { + size <- chooseShrinkable(1, MaxIDSize) + idStart <- lowerLetters + idEnd <- listOfN(size - 1, idCharGen) + } yield idStart + idEnd.mkString + } filter { id => + // The filter ensure that shrinking works + id.headOption.exists(_.isLower) && id.tail.forall(isIDChar) + } + + val scalaIDGen: Gen[String] = { + for { + size <- chooseShrinkable(1, MaxIDSize) + idStart <- upperLetters + idEnd <- listOfN(size - 1, scalaIDCharGen) + } yield idStart + idEnd.mkString + } filter { id => + // The filter ensure that shrinking works + id.headOption.exists(_.isUpper) && id.tail.forall(isIDChar) + } - implicit lazy val optIDGen: Gen[Option[String]] = frequency((1, idGen map some.fn), (1, None)) - implicit lazy val uriGen: Gen[URI] = for (sch <- idGen; ssp <- idGen; frag <- optIDGen) - yield new URI(sch, ssp, frag.orNull) + val schemeGen: Gen[String] = { + for { + schemeStart <- alphaChar + schemeEnd <- listOf(frequency(19 -> alphaNumChar, 1 -> oneOf('+', '-', '.'))) + } yield schemeStart + schemeEnd.mkString + } + + val uriChar: Gen[Char] = { + frequency(9 -> alphaNumChar, 1 -> oneOf(";/?:@&=+$,-_.!~*'()".toSeq)) + } + + val uriStringGen: Gen[String] = nonEmptyListOf(uriChar).map(_.mkString) + + val optIDGen: Gen[Option[String]] = oneOf(uriStringGen.map(some.fn), Gen.const(None)) + + val uriGen: Gen[URI] = { + for { + sch <- schemeGen + ssp <- uriStringGen + frag <- optIDGen + } yield new URI(sch, ssp, frag.orNull) + } implicit def envGen(implicit bGen: Gen[Build], tasks: Gen[Vector[Taskk]]): Gen[Env] = for (i <- MaxBuildsGen; bs <- containerOfN[Vector, Build](i, bGen); ts <- tasks) diff --git a/scripted/plugin/src/main/scala/sbt/ScriptedPlugin.scala b/scripted/plugin/src/main/scala/sbt/ScriptedPlugin.scala index 3e38a26ac4..37943e3881 100644 --- a/scripted/plugin/src/main/scala/sbt/ScriptedPlugin.scala +++ b/scripted/plugin/src/main/scala/sbt/ScriptedPlugin.scala @@ -95,7 +95,7 @@ object ScriptedPlugin extends AutoPlugin { } val pairMap = pairs.groupBy(_._1).mapValues(_.map(_._2).toSet); - val id = charClass(c => !c.isWhitespace && c != '/').+.string + val id = charClass(c => !c.isWhitespace && c != '/', "not whitespace and not '/'").+.string val groupP = token(id.examples(pairMap.keySet.toSet)) <~ token('/') // A parser for page definitions