From 765c451832a6f3797ef3b6e04d13ee34f36b9be6 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Thu, 3 Oct 2019 22:58:05 -0400 Subject: [PATCH] add lintBuild task to warn on unsed settings Fixes https://github.com/sbt/sbt/issues/3183 This implements an input task lintBuild that checks for unused settings/tasks. Because most settings are on the intermediary to other settings/tasks, they are included into the linting by default. The notable exceptions are settings used exclusively by a command. To opt-out, you can either append it to `Global / excludeLintKeys` or set the rank to invisible. On the other hand, many tasks are on the leaf (called by human), so task keys are excluded from linting by default. However there are notable tasks that trip up users, so they are opted-in using `Global / includeLintKeys`. --- main/src/main/scala/sbt/Defaults.scala | 2 +- main/src/main/scala/sbt/Keys.scala | 3 + .../main/scala/sbt/internal/LintBuild.scala | 145 ++++++++++++++++++ sbt/src/sbt-test/project/lint/build.sbt | 17 ++ sbt/src/sbt-test/project/lint/test | 1 + 5 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 main/src/main/scala/sbt/internal/LintBuild.scala create mode 100644 sbt/src/sbt-test/project/lint/build.sbt create mode 100644 sbt/src/sbt-test/project/lint/test diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 97bce5bf5..f98c761d3 100755 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -351,7 +351,7 @@ object Defaults extends BuildCommon { sys.env.contains("CI") || SysProp.ci, // watch related settings pollInterval :== Watch.defaultPollInterval, - ) + ) ++ LintBuild.lintSettings ) def defaultTestTasks(key: Scoped): Seq[Setting[_]] = diff --git a/main/src/main/scala/sbt/Keys.scala b/main/src/main/scala/sbt/Keys.scala index ae957c369..32a54ec13 100644 --- a/main/src/main/scala/sbt/Keys.scala +++ b/main/src/main/scala/sbt/Keys.scala @@ -491,6 +491,9 @@ object Keys { private[sbt] val postProgressReports = settingKey[Unit]("Internally used to modify logger.").withRank(DTask) @deprecated("No longer used", "1.3.0") private[sbt] val executeProgress = settingKey[State => TaskProgress]("Experimental task execution listener.").withRank(DTask) + val lintBuild = inputKey[Unit]("Stop a background job by providing its ID.") + val excludeLintKeys = settingKey[Set[Def.KeyedInitialize[_]]]("Keys excluded from lintBuild task") + val includeLintKeys = settingKey[Set[Def.KeyedInitialize[_]]]("Task keys that are included into lintBuild task") val stateStreams = AttributeKey[Streams]("stateStreams", "Streams manager, which provides streams for different contexts. Setting this on State will override the default Streams implementation.") val resolvedScoped = Def.resolvedScoped diff --git a/main/src/main/scala/sbt/internal/LintBuild.scala b/main/src/main/scala/sbt/internal/LintBuild.scala new file mode 100644 index 000000000..ad18e8fa3 --- /dev/null +++ b/main/src/main/scala/sbt/internal/LintBuild.scala @@ -0,0 +1,145 @@ +/* + * sbt + * Copyright 2011 - 2018, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt +package internal + +import Keys._ +import Def.{ Setting, ScopedKey } +import sbt.internal.util.{ FilePosition, NoPosition, SourcePosition } +import java.io.File +import Scope.Global +import sbt.Def._ + +object LintBuild { + lazy val lintSettings: Seq[Setting[_]] = Seq( + excludeLintKeys := Set( + aggregate, + concurrentRestrictions, + commands, + crossScalaVersions, + onLoadMessage, + sbt.nio.Keys.watchTriggers, + ), + includeLintKeys := Set( + scalacOptions, + javacOptions, + javaOptions, + incOptions, + compileOptions, + packageOptions, + mappings, + testOptions, + ), + lintBuild := lintBuildTask.evaluated, + ) + + def lintBuildTask: Def.Initialize[InputTask[Unit]] = Def.inputTask { + val _ = Def.spaceDelimited().parsed // not used yet + val state = Keys.state.value + val log = streams.value.log + val includeKeys = (includeLintKeys in Global).value map { _.scopedKey.key.label } + val excludeKeys = (excludeLintKeys in Global).value map { _.scopedKey.key.label } + val result = lint(state, includeKeys, excludeKeys) + if (result.isEmpty) log.success("ok") + else lintResultLines(result) foreach { log.warn(_) } + } + + def lintResultLines( + result: Seq[(ScopedKey[_], String, Vector[SourcePosition])] + ): Vector[String] = { + import scala.collection.mutable.ListBuffer + val buffer = ListBuffer.empty[String] + + val size = result.size + if (size == 1) buffer.append("there's a key that's not used by any other settings/tasks:") + else buffer.append(s"there are $size keys that are not used by any other settings/tasks:") + buffer.append(" ") + result foreach { + case (_, str, positions) => + buffer.append(s"* $str") + positions foreach { + case pos: FilePosition => buffer.append(s" +- ${pos.path}:${pos.startLine}") + case _ => () + } + } + buffer.append(" ") + buffer.append( + "note: a setting might still be used by a command; to exclude a key from this `lintBuild` check" + ) + buffer.append( + "either append it to `Global / excludeLintKeys` or set call .withRank(KeyRanks.Invisible) on the key" + ) + buffer.toVector + } + + def lint( + state: State, + includeKeys: Set[String], + excludeKeys: Set[String] + ): Seq[(ScopedKey[_], String, Vector[SourcePosition])] = { + val extracted = Project.extract(state) + val structure = extracted.structure + val display = Def.showShortKey(None) // extracted.showKey + val actual = true + val comp = + Def.compiled(structure.settings, actual)(structure.delegates, structure.scopeLocal, display) + val cMap = Def.flattenLocals(comp) + val used: Set[ScopedKey[_]] = cMap.values.flatMap(_.dependencies).toSet + val unused: Seq[ScopedKey[_]] = cMap.keys.filter(!used.contains(_)).toSeq + val withDefinedAts: Seq[UnusedKey] = unused map { u => + val definingScope = structure.data.definingScope(u.scope, u.key) + val definingScoped = definingScope match { + case Some(sc) => ScopedKey(sc, u.key) + case _ => u + } + val definedAt = comp.get(definingScoped) match { + case Some(c) => definedAtString(c.settings.toVector) + case _ => Vector.empty + } + val data = Project.scopedKeyData(structure, u.scope, u.key) + UnusedKey(u, definedAt, data) + } + + def isIncludeKey(u: UnusedKey): Boolean = includeKeys(u.scoped.key.label) + def isExcludeKey(u: UnusedKey): Boolean = excludeKeys(u.scoped.key.label) + def isSettingKey(u: UnusedKey): Boolean = u.data match { + case Some(data) => data.settingValue.isDefined + case _ => false + } + def isLocallyDefined(u: UnusedKey): Boolean = u.positions exists { + case pos: FilePosition => pos.path.contains(File.separator) + case _ => false + } + def isInvisible(u: UnusedKey): Boolean = u.scoped.key.rank == KeyRanks.Invisible + val unusedSettingKeys = withDefinedAts collect { + case u + if !isExcludeKey(u) && !isInvisible(u) + && (isSettingKey(u) || isIncludeKey(u)) + && isLocallyDefined(u) => + u + } + (unusedSettingKeys map { u => + (u.scoped, display.show(u.scoped), u.positions) + }).sortBy(_._2) + } + + private[this] case class UnusedKey( + scoped: ScopedKey[_], + positions: Vector[SourcePosition], + data: Option[ScopedKeyData[_]] + ) + + private def definedAtString(settings: Vector[Setting[_]]): Vector[SourcePosition] = { + settings flatMap { setting => + setting.pos match { + case NoPosition => Vector.empty + case pos => Vector(pos) + } + } + } +} diff --git a/sbt/src/sbt-test/project/lint/build.sbt b/sbt/src/sbt-test/project/lint/build.sbt new file mode 100644 index 000000000..c9bbcf716 --- /dev/null +++ b/sbt/src/sbt-test/project/lint/build.sbt @@ -0,0 +1,17 @@ +ThisBuild / doc / scalacOptions += "-Xsomething" + +lazy val lintBuildTest = taskKey[Unit]("") + +lazy val root = (project in file(".")) + .settings( + lintBuildTest := { + val state = Keys.state.value + val includeKeys = (includeLintKeys in Global).value map { _.scopedKey.key.label } + val excludeKeys = (excludeLintKeys in Global).value map { _.scopedKey.key.label } + val result = sbt.internal.LintBuild.lint(state, includeKeys, excludeKeys) + assert(result.size == 1) + assert(result(0)._2 == "ThisBuild / doc / scalacOptions", result(0)._2) + } + ) + +lazy val app = project diff --git a/sbt/src/sbt-test/project/lint/test b/sbt/src/sbt-test/project/lint/test new file mode 100644 index 000000000..0fff33bdb --- /dev/null +++ b/sbt/src/sbt-test/project/lint/test @@ -0,0 +1 @@ +> lintBuildTest