Rename -Xlint:named-booleans to -Wunnamed-boolean-literal (and no longer include it in -Xlint)

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/Reporting.scala

@@ -571,6 +571,7 @@ object Reporting {
         WFlagExtraImplicit,
         WFlagNumericWiden,
         WFlagSelfImplicit,
+        WFlagNamedLiteral,
         WFlagValueDiscard
       = wflag()
 
@@ -623,8 +624,7 @@ object Reporting {
         LintPerformance,
         LintIntDivToFloat,
         LintUniversalMethods,
-        LintNumericMethods,
-        LintNamedBooleans
+        LintNumericMethods
       = lint()
 
     sealed class Feature extends WarningCategory {

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

@@ -121,6 +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 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.")
@@ -217,7 +219,6 @@ trait Warnings {
     val NumericMethods         = LintWarning("numeric-methods",           "Dubious usages, such as `42.isNaN`.")
     val ArgDiscard             = LintWarning("arg-discard",               "-Wvalue-discard for adapted arguments.")
     val IntDivToFloat          = LintWarning("int-div-to-float",          "Warn when an integer division is converted (widened) to floating point: `(someInt / 2): Double`.")
-    val NamedBooleans          = LintWarning("named-booleans",            "Boolean literals should be named args unless param is @deprecatedName.")
     val PatternShadow          = LintWarning("pattern-shadow",            "Pattern variable id is also a term in scope.")
     val CloneableObject        = LintWarning("cloneable",                 "Modules (objects) should not be Cloneable.")
 
@@ -256,7 +257,6 @@ trait Warnings {
   def lintNumericMethods         = lint.contains(NumericMethods)
   def lintArgDiscard             = lint.contains(ArgDiscard)
   def lintIntDivToFloat          = lint.contains(IntDivToFloat)
-  def lintNamedBooleans          = lint.contains(NamedBooleans)
   def warnPatternShadow          = lint.contains(PatternShadow)
   def warnCloneableObject        = lint.contains(CloneableObject)
 

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

@@ -55,20 +55,24 @@ trait NamesDefaults { self: Analyzer =>
     qual:       Option[Tree],
     targs:      List[Tree],
     vargss:     List[List[Tree]],
-    blockTyper: Typer
-  ) { }
+    blockTyper: Typer,
+    original:   Tree,
+  )
   object NamedApplyBlock {
     private[this] val tag = reflect.classTag[NamedApplyInfo]
+    def namedApplyInfo(t: Tree): Option[NamedApplyInfo] = t.attachments.get[NamedApplyInfo](tag)
     def unapply(b: Tree): Option[NamedApplyInfo] = b match {
-      case _: Block => b.attachments.get[NamedApplyInfo](tag)
+      case _: Block => namedApplyInfo(b)
       case _ => None
     }
+    def apply(stats: List[Tree], expr: Tree)(nai: NamedApplyInfo): Block =
+      Block(stats, expr.updateAttachment(nai)).updateAttachment(nai)
   }
 
   private def nameOfNamedArg(arg: Tree) = Some(arg) collect { case NamedArg(Ident(name), _) => name }
   def isNamedArg(arg: Tree) = arg match {
     case NamedArg(Ident(_), _) => true
-    case _                             => false
+    case _                     => false
   }
 
   /** @param pos maps indices from old to new */
@@ -81,7 +85,7 @@ trait NamesDefaults { self: Analyzer =>
   /** @param pos maps indices from new to old (!) */
   private def reorderArgsInv[T: ClassTag](args: List[T], pos: Int => Int): List[T] = {
     val argsArray = args.toArray
-    (argsArray.indices map (i => argsArray(pos(i)))).toList
+    argsArray.indices.map(i => argsArray(pos(i))).toList
   }
 
   /** returns `true` if every element is equal to its index */
@@ -203,17 +207,14 @@ trait NamesDefaults { self: Analyzer =>
           else TypeApply(f, funTargs).setType(baseFun.tpe)
         }
 
-        val b = Block(List(vd), baseFunTransformed)
-                  .setType(baseFunTransformed.tpe).setPos(baseFun.pos.makeTransparent)
-        b.updateAttachment(NamedApplyInfo(Some(newQual), defaultTargs, Nil, blockTyper))
-        b
+        NamedApplyBlock(List(vd), baseFunTransformed)(NamedApplyInfo(Some(newQual), defaultTargs, Nil, blockTyper, tree))
+          .setType(baseFunTransformed.tpe)
+          .setPos(baseFun.pos.makeTransparent)
       }
 
-      def blockWithoutQualifier(defaultQual: Option[Tree]) = {
-        val b = atPos(baseFun.pos)(Block(Nil, baseFun).setType(baseFun.tpe))
-        b.updateAttachment(NamedApplyInfo(defaultQual, defaultTargs, Nil, blockTyper))
-        b
-      }
+      def blockWithoutQualifier(defaultQual: Option[Tree]) =
+        atPos(baseFun.pos)(NamedApplyBlock(Nil, baseFun)(NamedApplyInfo(defaultQual, defaultTargs, Nil, blockTyper, tree)))
+          .setType(baseFun.tpe)
 
       def moduleQual(pos: Position, classType: Type) = {
         // prefix does 'normalize', which fixes #3384
@@ -345,7 +346,7 @@ trait NamesDefaults { self: Analyzer =>
         val transformedFun = transformNamedApplication(typer, mode, pt)(fun, x => x)
         if (transformedFun.isErroneous) setError(tree)
         else {
-          val NamedApplyBlock(NamedApplyInfo(qual, targs, vargss, blockTyper)) = transformedFun: @unchecked
+          val NamedApplyBlock(NamedApplyInfo(qual, targs, vargss, blockTyper, _)) = transformedFun: @unchecked
           val Block(stats, funOnly) = transformedFun: @unchecked
 
           // type the application without names; put the arguments in definition-site order
@@ -373,16 +374,16 @@ trait NamesDefaults { self: Analyzer =>
                     tpe.typeSymbol match {
                       case ByNameParamClass   => Apply(ref, Nil)
                       case RepeatedParamClass => Typed(ref, Ident(tpnme.WILDCARD_STAR))
-                      case _                  => ref
+                      case _                  => origArg.attachments.get[UnnamedArg.type].foreach(ref.updateAttachment); ref
                     }
                   }
               })
               // cannot call blockTyper.typedBlock here, because the method expr might be partially applied only
               val res = blockTyper.doTypedApply(tree, expr, refArgs, mode, pt)
               res.setPos(res.pos.makeTransparent)
-              val block = Block(stats ::: valDefs.flatten, res).setType(res.tpe).setPos(tree.pos.makeTransparent)
-              block.updateAttachment(NamedApplyInfo(qual, targs, vargss :+ refArgs, blockTyper))
-              block
+              NamedApplyBlock(stats ::: valDefs.flatten, res)(NamedApplyInfo(qual, targs, vargss :+ refArgs, blockTyper, tree))
+                .setType(res.tpe)
+                .setPos(tree.pos.makeTransparent)
             case _ => tree
           }
         }

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

@@ -1703,47 +1703,75 @@ abstract class RefChecks extends Transform {
 
           transform(qual)
       case Apply(fn, args) =>
+        currentApplication = tree
         // sensicality should be subsumed by the unreachability/exhaustivity/irrefutability
         // analyses in the pattern matcher
         if (!inPattern) {
           checkImplicitViewOptionApply(tree.pos, fn, args)
           checkSensible(tree.pos, fn, args) // TODO: this should move to preEraseApply, as reasoning about runtime semantics makes more sense in the JVM type system
           checkNamedBooleanArgs(fn, args)
         }
-        currentApplication = tree
         tree
     }
 
     /** Check that boolean literals are passed as named args.
-     *  The rule is enforced when the type of the parameter is `Boolean`.
-     *  The rule is relaxed when the method has exactly one boolean parameter
+     *  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 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)`.
      */
     private def checkNamedBooleanArgs(fn: Tree, args: List[Tree]): Unit = {
       val sym = fn.symbol
       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)
       }
-      def isAssertParadigm(params: List[Symbol]): Boolean = !sym.isConstructor && !sym.isCaseApplyOrUnapply && {
-        params match {
-          case h :: t => h.tpe == BooleanTpe && !t.exists(_.tpe == BooleanTpe)
-          case _ => false
-        }
-      }
-      if (settings.lintNamedBooleans && !sym.isJavaDefined && !args.isEmpty) {
+      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)
-        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) =>
-              runReporting.warning(arg.pos, s"Boolean literals should be passed using named argument syntax for parameter ${param.name}.", WarningCategory.LintNamedBooleans, sym)
-            case _ =>
-          })
+        val numBools = params.count(_.tpe == BooleanTpe)
+        def onlyLeadingBool = numBools == 1 && params.head.tpe == BooleanTpe
+        val checkable = if (strictly) numBools > 0 && !onlyLeadingBool else numBools >= 2
+        if (checkable) {
+          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)
+                        i > 0 && isNameableBoolean(allParams(i-1))
+                      case _ => false
+                    }
+                  case _ => 0
+                }
+              case _ => args.count(arg => arg.symbol != null && arg.symbol.isDefaultGetter)
+            }
+          }
+          val warn = !unnamed.isEmpty && (strictly || numSuspicious >= 2)
+          if (warn)
+            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)
+                runReporting.warning(arg.pos, msg, WarningCategory.WFlagNamedLiteral, sym, action)
+              case _ =>
+            }
+        }
       }
     }
 

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

@@ -3829,7 +3829,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
               val fun1 = transformNamedApplication(Typer.this, mode, pt)(fun, x => x)
               if (fun1.isErroneous) duplErrTree
               else {
-                val NamedApplyBlock(NamedApplyInfo(qual, targs, previousArgss, _)) = fun1: @unchecked
+                val NamedApplyBlock(NamedApplyInfo(qual, targs, previousArgss, _, _)) = fun1: @unchecked
                 val blockIsEmpty = fun1 match {
                   case Block(Nil, _) =>
                     // if the block does not have any ValDef we can remove it. Note that the call to

src/reflect/scala/reflect/internal/StdNames.scala

@@ -551,7 +551,12 @@ trait StdNames {
           case -1  => (name.toTermName, -1)
           case idx => (name.toTermName.take(idx), idx + DEFAULT_GETTER_STRING.length)
         }
-      if (i >= 0) (n, name.encoded.substring(i).toInt) else (n, -1)
+      if (i < 0) (n, -1)
+      else {
+        val j = name.indexOf('$', i) // f$default$7$extension
+        val idx = name.subSequence(i, if (j < 0) name.length else j)
+        (n, idx.toString.toInt)
+      }
     }
 
     def localDummyName(clazz: Symbol): TermName = newTermName(LOCALDUMMY_PREFIX + clazz.name + ">")

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

@@ -0,0 +1,54 @@
+named-booleans-relaxed.scala:22: warning: Boolean literals should be passed using named argument syntax for parameter x. [quickfixable]
+  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. [quickfixable]
+  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. [quickfixable]
+  c.uncheck(false, "OK", true)
+            ^
+named-booleans-relaxed.scala:44: warning: Boolean literals should be passed using named argument syntax for parameter flag. [quickfixable]
+  c.uncheck(false, "OK", true)
+                         ^
+named-booleans-relaxed.scala:63: warning: Boolean literals should be passed using named argument syntax for parameter isKlazz. [quickfixable]
+  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. [quickfixable]
+  def test = Klazz(true, false) // warn case class apply as for ctor
+                         ^
+named-booleans-relaxed.scala:71: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
+  def g3 = f(42, false) // warn, unnamed could mean either param with default
+                 ^
+named-booleans-relaxed.scala:72: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
+  def g4 = f(42, false, true) // warn, swappable
+                 ^
+named-booleans-relaxed.scala:72: warning: Boolean literals should be passed using named argument syntax for parameter down. [quickfixable]
+  def g4 = f(42, false, true) // warn, swappable
+                        ^
+named-booleans-relaxed.scala:79: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
+  def rev3 = rev(42, reverse=true, false) // warn, unnamed could mean either param with default
+                                   ^
+named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter reverse. [quickfixable]
+  def rev4 = rev(42, false, true, false) // warn, swappable
+                     ^
+named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter up. [quickfixable]
+  def rev4 = rev(42, false, true, false) // warn, swappable
+                            ^
+named-booleans-relaxed.scala:80: warning: Boolean literals should be passed using named argument syntax for parameter down. [quickfixable]
+  def rev4 = rev(42, false, true, false) // warn, swappable
+                                  ^
+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
+              ^
+named-booleans-relaxed.scala:106: warning: Boolean literals should be passed using named argument syntax for parameter y. [quickfixable]
+  def w = new V(true).combo(false)
+                            ^
+error: No warnings can be incurred under -Werror.
+17 warnings
+1 error