From c9aa0c52853c158e71d36f03eb7316c6a90380ee 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 project configurations. --- .../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/src/main/scala/sbt/ScriptedPlugin.scala | 2 +- .../main/scala/sbt/internal/IvyConsole.scala | 2 +- .../main/scala/sbt/internal/KeyIndex.scala | 104 +++++-- main/src/main/scala/sbt/internal/Load.scala | 28 +- main/src/main/scala/sbt/internal/Script.scala | 2 +- main/src/test/scala/ParseKey.scala | 256 +++++++++++++++++- main/src/test/scala/ParserSpec.scala | 77 ++++++ main/src/test/scala/PluginCommandTest.scala | 5 +- .../test/scala/sbt/internal/TestBuild.scala | 105 +++++-- .../internal/server/SettingQueryTest.scala | 4 +- notes/1.2.0/unknown-config-warning.md | 8 + 18 files changed, 534 insertions(+), 86 deletions(-) create mode 100644 main/src/test/scala/ParserSpec.scala create mode 100644 notes/1.2.0/unknown-config-warning.md 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 09bf9a8a3..31c1ca67a 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) + } } } @@ -383,8 +387,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] = { @@ -396,7 +400,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. */ @@ -746,6 +750,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 ce444e229..ab160524b 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 @@ -166,7 +166,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 df318d323..17ef8c3c0 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 896cd9fe1..bae0a3cb4 100644 --- a/main-command/src/main/scala/sbt/BasicCommands.scala +++ b/main-command/src/main/scala/sbt/BasicCommands.scala @@ -146,7 +146,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 20c2115a1..bc6decb10 100644 --- a/main/src/main/scala/sbt/Extracted.scala +++ b/main/src/main/scala/sbt/Extracted.scala @@ -148,7 +148,7 @@ final case class Extracted( ): 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 49094156b..4e1bab596 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -459,7 +459,7 @@ object BuiltinCommands { val loggerInject = LogManager.settingsLogger(s) val withLogger = newSession.appendRaw(loggerInject :: Nil) val show = Project.showContextKey2(newSession) - 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 ea0d3fda1..634f3890e 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 { 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/ScriptedPlugin.scala b/main/src/main/scala/sbt/ScriptedPlugin.scala index b17886904..d871b78b6 100644 --- a/main/src/main/scala/sbt/ScriptedPlugin.scala +++ b/main/src/main/scala/sbt/ScriptedPlugin.scala @@ -129,7 +129,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)) <~ token('/') // A parser for page definitions diff --git a/main/src/main/scala/sbt/internal/IvyConsole.scala b/main/src/main/scala/sbt/internal/IvyConsole.scala index a2cd16b1d..360e18da2 100644 --- a/main/src/main/scala/sbt/internal/IvyConsole.scala +++ b/main/src/main/scala/sbt/internal/IvyConsole.scala @@ -57,7 +57,7 @@ object IvyConsole { 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 fc6ef184c..34762eb81 100644 --- a/main/src/main/scala/sbt/internal/KeyIndex.scala +++ b/main/src/main/scala/sbt/internal/KeyIndex.scala @@ -41,11 +41,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 { @@ -61,6 +65,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)) @@ -74,7 +79,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) } @@ -109,6 +114,7 @@ trait KeyIndex { ): 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 @@ -121,45 +127,74 @@ 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] + val data: Map[String, ConfigData], + val inverse: Map[String, String], + val noConfigKeys: AKeyIndex ) { def add( - config: Option[String], + 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 keyIndex(conf: Option[String]): AKeyIndex = getOr(data, conf, emptyAKeyIndex) - def configs: Set[String] = keySet(data) + 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) + } - private[sbt] val configIdentsInverse: Map[String, String] = - identData map { _.swap } + def addKeyWithoutConfig(task: Option[AttributeKey[_]], key: AttributeKey[_]): ConfigIndex = { + new ConfigIndex(data, inverse, noConfigKeys.add(task, key)) + } - private[sbt] lazy val idents: Set[String] = - configs map { config => - identData.getOrElse(config, Scope.guessConfigIdent(config)) - } + def keyIndex(conf: Option[String]): AKeyIndex = conf match { + case Some(c) => data.get(c).map(_.keys).getOrElse(emptyAKeyIndex) + case None => noConfigKeys + } + + 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 = @@ -171,7 +206,7 @@ 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 = @@ -189,11 +224,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( @@ -247,6 +301,8 @@ private[sbt] final class KeyIndex0(val data: BuildIndex) extends ExtendableKeyIn config: ScopeAxis[ConfigKey], task: ScopeAxis[AttributeKey[_]], key: AttributeKey[_] - ): ExtendableKeyIndex = - new KeyIndex0(data.add(uri, id, config.toOption.map(_.name), task.toOption, key)) + ): 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 b0d8b53dc..0f1dc625d 100755 --- a/main/src/main/scala/sbt/internal/Load.scala +++ b/main/src/main/scala/sbt/internal/Load.scala @@ -267,7 +267,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( @@ -321,7 +321,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) @@ -330,6 +331,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), @@ -340,15 +342,33 @@ 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 2192785a6..f03dcacde 100644 --- a/main/src/main/scala/sbt/internal/Script.scala +++ b/main/src/main/scala/sbt/internal/Script.scala @@ -65,7 +65,7 @@ object Script { 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 ecc6374dc..9af98f56f 100644 --- a/main/src/test/scala/ParseKey.scala +++ b/main/src/test/scala/ParseKey.scala @@ -7,11 +7,18 @@ package sbt -import Def.{ displayFull, displayMasked, ScopedKey } -import sbt.internal.{ TestBuild, Resolve }, TestBuild._ -import sbt.internal.util.complete.Parser +import java.net.URI -import org.scalacheck._, Arbitrary.arbitrary, Gen._, Prop._ +import org.scalacheck.Arbitrary.{ arbBool, arbitrary } +import org.scalacheck.Gen._ +import org.scalacheck.Prop._ +import org.scalacheck._ +import sbt.Def.{ ScopedKey, displayFull, displayMasked } +import sbt.internal.TestBuild._ +import sbt.internal.util.AttributeKey +import sbt.internal.util.complete.{ DefaultParsers, Parser } +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. @@ -26,10 +33,7 @@ object ParseKey extends Properties("Key parser test") { // Note that this explicitly displays the configuration axis set to Zero. // This is to disambiguate `proj/Zero/name`, which could render potentially // as `Zero/name`, but could be interpreted as `Zero/Zero/name`. - val expected = ScopedKey( - Resolve(structure.extra, Select(structure.current), key.key, mask)(key.scope), - key.key - ) + val expected = resolve(structure, key, mask) parseCheck(structure, key, mask, hasZeroConfig)( sk => Project.equal(sk, expected, mask) @@ -75,14 +79,20 @@ object ParseKey extends Properties("Key parser test") { scopes <- pickN(loadFactor, env.allFullScopes) current <- oneOf(env.allProjects.unzip._1) structure <- { - 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) } } yield structure } - final class StructureKeyMask(val structure: Structure, val key: ScopedKey[_], val mask: ScopeMask) + 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("")) + } + + final case class StructureKeyMask(structure: Structure, key: ScopedKey[_], mask: ScopeMask) implicit val arbStructureKeyMask: Arbitrary[StructureKeyMask] = Arbitrary { for { @@ -92,9 +102,35 @@ object ParseKey extends Properties("Key parser test") { scope <- TestBuild.scope(structure.env) key <- oneOf(structure.allAttributeKeys.toSeq) } yield ScopedKey(scope, key) - } yield new StructureKeyMask(structure, key, mask) + skm = StructureKeyMask(structure, key, mask) + if configExistsInIndex(skm) + } yield skm } + 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), + key.key + ) + def parseCheck( structure: Structure, key: ScopedKey[_], @@ -118,4 +154,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((load * from.size).toInt max 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 000000000..72f44dbaf --- /dev/null +++ b/main/src/test/scala/ParserSpec.scala @@ -0,0 +1,77 @@ +/* + * 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/PluginCommandTest.scala b/main/src/test/scala/PluginCommandTest.scala index d40863578..627c6b568 100644 --- a/main/src/test/scala/PluginCommandTest.scala +++ b/main/src/test/scala/PluginCommandTest.scala @@ -10,7 +10,6 @@ package sbt import java.io._ import org.specs2.mutable.Specification - import sbt.internal._ import sbt.internal.util.{ AttributeEntry, @@ -20,6 +19,7 @@ import sbt.internal.util.{ MainAppender, Settings } +import sbt.util.Logger object PluginCommandTestPlugin0 extends AutoPlugin { override def requires = empty } @@ -103,7 +103,8 @@ object FakeState { val data: Settings[Scope] = Def.make(settings)(delegates, scopeLocal, Def.showFullKey) val extra: KeyIndex => BuildUtil[_] = (keyIndex) => BuildUtil(base.toURI, Map.empty, keyIndex, data) - val structureIndex: StructureIndex = Load.structureIndex(data, settings, extra, Map.empty) + val structureIndex: StructureIndex = + Load.structureIndex(data, settings, extra, Map.empty, Logger.Null) val streams: (State) => BuildStreams.Streams = null val loadedDefinitions: LoadedDefinitions = new LoadedDefinitions( diff --git a/main/src/test/scala/sbt/internal/TestBuild.scala b/main/src/test/scala/sbt/internal/TestBuild.scala index 825412d35..826e71867 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]) { @@ -119,7 +119,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 @@ -159,7 +159,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) @@ -167,10 +167,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 ") + @@ -178,7 +178,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(", ") + ")" } @@ -234,19 +234,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] = { @@ -255,18 +253,69 @@ 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) + } + + val idGen: Gen[String] = idGen(upperLetters, idCharGen, _.isUpper) + + val lowerIDGen: Gen[String] = idGen(lowerLetters, idCharGen, _.isLower) + + val scalaIDGen: Gen[String] = idGen(upperLetters, scalaIDCharGen, _.isUpper) + + def idGen(start: Gen[Char], end: Gen[Char], headFilter: Char => Boolean): Gen[String] = { for { size <- chooseShrinkable(1, MaxIDSize) - cs <- listOfN(size, alphaChar) - } yield { - val xs = cs.mkString - xs.take(1).toLowerCase + xs.drop(1) - } + idStart <- start + idEnd <- listOfN(size - 1, end) + } yield idStart + idEnd.mkString + } filter { id => + // The filter ensure that shrinking works + id.headOption.exists(headFilter) && 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/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala b/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala index a971c818a..4342afeb2 100644 --- a/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala +++ b/main/src/test/scala/sbt/internal/server/SettingQueryTest.scala @@ -16,7 +16,7 @@ import java.util.concurrent._ import scala.collection.mutable -import xsbti._ +import xsbti.{ Logger => _, _ } import sbt.io.IO import sbt.internal.util._ import sbt.internal.BuildStreams.{ Streams => _, _ } @@ -174,7 +174,7 @@ object SettingQueryTest extends org.specs2.mutable.Specification { val data: Settings[Scope] = Def.make(settings)(delegates, scopeLocal, display) val extra: KeyIndex => BuildUtil[_] = index => BuildUtil(baseUri, units, index, data) - val index: StructureIndex = structureIndex(data, settings, extra, units) + val index: StructureIndex = structureIndex(data, settings, extra, units, Logger.Null) val streams: State => Streams = mkStreams(units, baseUri, data) val structure: BuildStructure = diff --git a/notes/1.2.0/unknown-config-warning.md b/notes/1.2.0/unknown-config-warning.md new file mode 100644 index 000000000..645cbedf9 --- /dev/null +++ b/notes/1.2.0/unknown-config-warning.md @@ -0,0 +1,8 @@ +[@steinybot]: https://github.com/steinybot + +[#4065]: https://github.com/sbt/sbt/issues/4065 +[#4231]: https://github.com/sbt/sbt/pull/4231 + +### Improvements + +- Add a warning for unknown project configurations. [#4065][]/[#4231][] by [@steinybot][]