Skip to content

Commit

Permalink
Ylint to warn strict else warn multiple boolean lits
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Feb 28, 2024
1 parent fa3b77b commit 0970a1b
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
* -Y "Private" settings
*/
val Yhelp = BooleanSetting ("-Y", "Print a synopsis of private options.")
val Ylint = BooleanSetting ("-Ylint", "Use strict lints.")
val breakCycles = BooleanSetting ("-Ybreak-cycles", "Attempt to break cycles encountered during typing")
val check = PhasesSetting ("-Ycheck", "Check the tree at the end of")
val Ycompacttrees = BooleanSetting ("-Ycompact-trees", "Use compact tree printer when displaying trees.")
Expand Down
21 changes: 14 additions & 7 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1724,7 +1724,7 @@ abstract class RefChecks extends Transform {
def applyDepth: Int = {
def loop(t: Tree, d: Int): Int =
t match {
case Apply(f, _) => loop(f, d+1)
case Apply(t, _) => loop(t, d+1)
case _ => d
}
loop(fn, 0)
Expand All @@ -1736,13 +1736,20 @@ abstract class RefChecks extends Transform {
}
if (settings.lintNamedBooleans && !sym.isJavaDefined && !args.isEmpty) {
val params = sym.paramLists(applyDepth)
if (!isAssertParadigm(params))
foreach2(args, params)((arg, param) => arg match {
case Literal(Constant(_: Boolean))
if arg.hasAttachment[UnnamedArg.type] && param.tpe.typeSymbol == BooleanClass && !param.deprecatedParamName.contains(nme.NO_NAME) =>
if (!settings.Ylint.value || !isAssertParadigm(params)) {
val suspicious = args.lazyZip(params).iterator
.filter {
case (arg @ Literal(Constant(_: Boolean)) , param) =>
arg.hasAttachment[UnnamedArg.type] && param.tpe.typeSymbol == BooleanClass && !param.deprecatedParamName.contains(nme.NO_NAME)
case _ => false
}
.toList
val warn = if (settings.Ylint.value) suspicious.nonEmpty else suspicious.lengthCompare(2) >= 0
if (warn)
suspicious.foreach { case (arg, param) =>
runReporting.warning(arg.pos, s"Boolean literals should be passed using named argument syntax for parameter ${param.name}.", WarningCategory.LintNamedBooleans, sym)
case _ =>
})
}
}
}
}

Expand Down
21 changes: 21 additions & 0 deletions test/files/neg/named-booleans-relaxed.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
named-booleans-relaxed.scala:22: warning: Boolean literals should be passed using named argument syntax for parameter x.
val x0 = c.f(17, true, false) // warn
^
named-booleans-relaxed.scala:22: warning: Boolean literals should be passed using named argument syntax for parameter y.
val x0 = c.f(17, true, false) // warn
^
named-booleans-relaxed.scala:44: warning: Boolean literals should be passed using named argument syntax for parameter cond.
c.uncheck(false, "OK", true)
^
named-booleans-relaxed.scala:44: warning: Boolean literals should be passed using named argument syntax for parameter flag.
c.uncheck(false, "OK", true)
^
named-booleans-relaxed.scala:63: warning: Boolean literals should be passed using named argument syntax for parameter isKlazz.
def test = Klazz(true, false) // warn case class apply as for ctor
^
named-booleans-relaxed.scala:63: warning: Boolean literals should be passed using named argument syntax for parameter isWarnable.
def test = Klazz(true, false) // warn case class apply as for ctor
^
error: No warnings can be incurred under -Werror.
6 warnings
1 error
64 changes: 64 additions & 0 deletions test/files/neg/named-booleans-relaxed.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//> using options -Werror -Xlint:named-booleans

class C {
def f(n: Int = 42, x: Boolean, y: Boolean) = if (x && y) n else 0

def g(x: Any) =
x match {
case (true, false) => 0
case _ => 1
}
var b = false
def fs(n: Int)(s: String, b: Boolean) = if (b) s*n else s
def gs[A](n: Int)(s: A, b: Boolean) = if (b) s.toString*n else s.toString

def check(cond: Boolean, msg: => String) = if (cond) println(msg)
def uncheck(cond: Boolean, msg: => String, flag: Boolean) = if (cond && flag) println(msg)
}

object Test extends App {
val c = new C
val b = false
val x0 = c.f(17, true, false) // warn
val x1 = c.f(17, true, b) // nowarn
val x2 = c.f(y = b, n = 17, x = true) // nowarn
c.b = true
val y = Some(false)
val z = Option(false)
val w = (true, false)
val v = c g true // nowarn infix

val s = collection.mutable.Set.empty[String]
def mutateS(): Unit = s("updater") = true
//def updateS(): Unit = s.update("updater", true)

val m = collection.mutable.Map.empty[String, true]
def mutateM(): Unit = m("updater") = true

val ss = c.fs(42)("hello", true)
val tt = c.gs(42)("hello", true)

def f(g: Boolean => Option[Boolean]) = g(true).getOrElse(false)

c.check(true, "OK")
c.uncheck(false, "OK", true)
}

class Arrays {
def test = Array(true, false, true)
}

class Tuples {
def test = (true, false, true)
}

class Functions {
val f: Boolean => Boolean = identity
def test = f(true)
}

case class Klazz(isKlazz: Boolean, isWarnable: Boolean)

class Klazzy {
def test = Klazz(true, false) // warn case class apply as for ctor
}
2 changes: 1 addition & 1 deletion test/files/neg/named-booleans.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// scalac: -Werror -Xlint:named-booleans
//> using options -Werror -Xlint:named-booleans -Ylint

class C {
def f(n: Int = 42, x: Boolean, y: Boolean) = if (x && y) n else 0
Expand Down

0 comments on commit 0970a1b

Please sign in to comment.