Correctly count boolean default arg usage

build.sbt

@@ -169,6 +169,7 @@ lazy val commonSettings = instanceSettings ++ clearSourceAndResourceDirectories
     "-Wconf:cat=optimizer:is",
     // we use @nowarn for methods that are deprecated in JDK > 8, but CI/release is under JDK 8
     "-Wconf:cat=unused-nowarn:s",
+    //"-Wunnamed-boolean-literal-strict",
     ),
   Compile / doc / scalacOptions ++= Seq(
     "-doc-footer", "epfl",

src/compiler/scala/tools/nsc/settings/Warnings.scala

@@ -121,8 +121,8 @@ trait Warnings {
   val warnValueDiscard     = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") withAbbreviation "-Ywarn-value-discard"
   val warnNumericWiden     = BooleanSetting("-Wnumeric-widen", "Warn when numerics are widened.") withAbbreviation "-Ywarn-numeric-widen"
   val warnOctalLiteral     = BooleanSetting("-Woctal-literal", "Warn on obsolete octal syntax.") withAbbreviation "-Ywarn-octal-literal"
-  val warnNamedLiteral     = BooleanSetting("-Wunnamed-boolean-literal", "Warn about unnamed boolean literals if there is more than one or defaults are used, unless parameter has @deprecatedName.")
-  val warnNamedBoolean     = BooleanSetting("-Wunnamed-boolean-literal-strict", "Warn about all unnamed boolean literals, unless parameter has @deprecatedName or the method has a single leading boolean parameter.").enabling(warnNamedLiteral :: Nil)
+  val warnUnnamedBoolean   = BooleanSetting("-Wunnamed-boolean-literal", "Warn about unnamed boolean literals if there is more than one or defaults are used, unless parameter has @deprecatedName.")
+  val warnUnnamedStrict    = BooleanSetting("-Wunnamed-boolean-literal-strict", "Warn about all unnamed boolean literals, unless parameter has @deprecatedName or the method has a single leading boolean parameter.").enabling(warnUnnamedBoolean :: Nil)
 
   object PerformanceWarnings extends MultiChoiceEnumeration {
     val Captured       = Choice("captured",        "Modification of var in closure causes boxing.")

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -1717,7 +1717,7 @@ abstract class RefChecks extends Transform {
     /** Check that boolean literals are passed as named args.
      *  The rule is enforced when the type of the parameter is `Boolean`,
      *  and there is more than one parameter with an unnamed argument.
-     *  The stricter internal lint warns for any unnamed argument,
+     *  The stricter lint warns for any unnamed argument,
      *  except that the rule is relaxed when the method has exactly one boolean parameter
      *  and it is the first parameter, such as `assert(false, msg)`.
      */
@@ -1731,40 +1731,40 @@ abstract class RefChecks extends Transform {
           }
         loop(fn, 0)
       }
-      if (settings.warnNamedLiteral.value && !sym.isJavaDefined && !args.isEmpty) {
-        def isUnnamedArg(t: Tree) = t.hasAttachment[UnnamedArg.type]
-        def isDefaultArg(t: Tree) = t.symbol != null && (
-          t.symbol.isDefaultGetter || t.symbol.name.startsWith(nme.NAMEDARG_PREFIX) && {
-            analyzer.NamedApplyBlock.namedApplyInfo(currentApplication) match {
-              case Some(analyzer.NamedApplyInfo(_, _, _, _, original)) =>
-                val treeInfo.Applied(_, _, argss) = original
-                val orig = argss.head(t.symbol.name.decoded.stripPrefix(nme.NAMEDARG_PREFIX).toInt - 1)
-                orig.symbol != null && orig.symbol.isDefaultGetter
-              case _ => false
-            }
-          }
-        )
+      if (settings.warnUnnamedBoolean.value && !sym.isJavaDefined && !args.isEmpty) {
+        val strictly = settings.warnUnnamedStrict.value // warn about any unnamed boolean arg, modulo "assert"
         val params = sym.paramLists(applyDepth)
         val numBools = params.count(_.tpe == BooleanTpe)
         def onlyLeadingBool = numBools == 1 && params.head.tpe == BooleanTpe
-        val checkable = if (settings.warnNamedBoolean.value) numBools > 0 && !onlyLeadingBool else numBools >= 2
+        val checkable = if (strictly) numBools > 0 && !onlyLeadingBool else numBools >= 2
         if (checkable) {
-          val (unnamed, numSuspicious) = args.lazyZip(params).iterator
-            .foldLeft((List.empty[(Tree, Symbol)], 0)) { (acc, ap) =>
-              ap match {
-                case (arg, param) if param.tpe.typeSymbol == BooleanClass && !param.deprecatedParamName.contains(nme.NO_NAME) =>
-                  arg match {
-                    case Literal(Constant(_: Boolean)) =>
-                      if (isUnnamedArg(arg)) (ap :: acc._1, acc._2 + 1) else acc
-                    case _ =>
-                      if (isDefaultArg(arg)) (acc._1, acc._2 + 1) else acc
-                  }
-                case _ => acc
-              }
+          def isUnnamedArg(t: Tree) = t.hasAttachment[UnnamedArg.type]
+          def isNameableBoolean(param: Symbol) = param.tpe.typeSymbol == BooleanClass && !param.deprecatedParamName.contains(nme.NO_NAME)
+          val unnamed = args.lazyZip(params).filter {
+            case (arg @ Literal(Constant(_: Boolean)), param) => isNameableBoolean(param) && isUnnamedArg(arg)
+            case _ => false
+          }
+          def numSuspicious = unnamed.length + {
+            analyzer.NamedApplyBlock.namedApplyInfo(currentApplication) match {
+              case Some(analyzer.NamedApplyInfo(_, _, _, _, original)) =>
+                val treeInfo.Applied(_, _, argss) = original
+                argss match {
+                  case h :: _ =>
+                    val allParams = sym.paramLists.flatten
+                    h.count {
+                      case treeInfo.Applied(getter, _, _) if getter.symbol != null && getter.symbol.isDefaultGetter =>
+                        val (_, i) = nme.splitDefaultGetterName(getter.symbol.name)
+                        isNameableBoolean(allParams(i-1))
+                      case _ => false
+                    }
+                  case _ => 0
+                }
+              case _ => args.count(arg => arg.symbol != null && arg.symbol.isDefaultGetter)
             }
-          val warn = !unnamed.isEmpty && (settings.warnNamedBoolean.value || numSuspicious >= 2)
+          }
+          val warn = !unnamed.isEmpty && (strictly || numSuspicious >= 2)
           if (warn)
-            unnamed.reverse.foreach {
+            unnamed.foreach {
               case (arg, param) =>
                 val msg = s"Boolean literals should be passed using named argument syntax for parameter ${param.name}."
                 val action = runReporting.codeAction("name boolean literal", arg.pos.focusStart, s"${param.name} = ", msg)

test/files/neg/named-booleans-relaxed.check

@@ -40,6 +40,12 @@ named-booleans-relaxed.scala:80: warning: Boolean literals should be passed usin
 named-booleans-relaxed.scala:81: warning: Boolean literals should be passed using named argument syntax for parameter reverse. [quickfixable]
   def rev5 = rev(42, true, down=true) // warn, out of order so it's a named block, otherwise same as rev3
                      ^
+named-booleans-relaxed.scala:92: warning: Boolean literals should be passed using named argument syntax for parameter insideIf. [quickfixable]
+  def sus(s: String) = p.needsParentheses(s)(false) // warn
+                                             ^
+named-booleans-relaxed.scala:95: warning: Boolean literals should be passed using named argument syntax for parameter x. [quickfixable]
+  def f = p.f(true, z=42) // warn
+              ^
 error: No warnings can be incurred under -Werror.
-14 warnings
+16 warnings
 1 error

test/files/neg/named-booleans-relaxed.scala

@@ -80,3 +80,19 @@ class Defaulting {
   def rev4 = rev(42, false, true, false) // warn, swappable
   def rev5 = rev(42, true, down=true) // warn, out of order so it's a named block, otherwise same as rev3
 }
+
+class Printers {
+  def needsParentheses(parent: String)(insideIf: Boolean = true, insideMatch: Boolean = true, insideTry: Boolean = true, insideAnnotated: Boolean = true, insideBlock: Boolean = true, insideLabelDef: Boolean = true, insideAssign: Boolean = true): Boolean = true
+
+  def f(x: Boolean, y: String = "hi", z: Int = 2, b: Boolean = false) = if (x && b) y+z else y*z
+}
+object TestPrinters {
+  val p = new Printers
+  def ok(s: String) = p.needsParentheses(s)(insideLabelDef = false)
+  def sus(s: String) = p.needsParentheses(s)(false) // warn
+  def pick(s: String) = p.needsParentheses(s)(true, insideAssign=false, insideLabelDef=false, insideBlock=false, insideAnnotated=false, insideTry=false, insideMatch=false)
+
+  def f = p.f(true, z=42) // warn
+  def g = p.f(x=true, b=true) // nowarn, no unnamed
+  def h = p.f(true, b=true) // nowarn, one unnamed but other boolean is named; defaults are non-boolean
+}