Merge pull request #10635 from som-snytt/issue/7530-multiassign-warn

Nuance advice when var in multidef could be val

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

@@ -2952,8 +2952,9 @@ self =>
       }
       val trees = lhs.toList.init.flatMap(mkDefs(_, tp.duplicate, rhs.duplicate)) ::: mkDefs(lhs.last, tp, rhs)
       val hd = trees.head
-      hd setPos hd.pos.withStart(pos)
+      hd.setPos(hd.pos.withStart(pos))
       ensureNonOverlapping(hd, trees.tail)
+      if (trees.lengthCompare(1) > 0) trees.foreach(_.updateAttachment(MultiDefAttachment))
       trees
     }
 

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

@@ -1763,6 +1763,9 @@ trait Namers extends MethodSynthesis {
 
         patchSymInfo(pt)
 
+        if (vdef.hasAttachment[MultiDefAttachment.type])
+          vdef.symbol.updateAttachment(MultiDefAttachment)
+
         // derives the val's result type from type checking its rhs under the expected type `pt`
         // vdef.tpt is mutated, and `vdef.tpt.tpe` is `assignTypeToTree`'s result
         val tptFromRhsUnderPt = assignTypeToTree(vdef, typer, pt)

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

@@ -702,6 +702,7 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
           if (sym.isPrivate) settings.warnUnusedPrivates && !sym.isTopLevel
           else settings.warnUnusedLocals
         val valAdvice = "is never updated: consider using immutable val"
+        def varAdvice(v: Symbol) = if (v.accessedOrSelf.hasAttachment[MultiDefAttachment.type]) "is never updated: consider refactoring vars to a separate definition" else valAdvice
         def wcat(sym: Symbol) = if (sym.isPrivate) WarningCategory.UnusedPrivates else WarningCategory.UnusedLocals
         def termWarning(defn: SymTree): Unit = {
           val sym = defn.symbol
@@ -719,7 +720,7 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
           val what =
             if (sym.isDefaultGetter) "default argument"
             else if (sym.isConstructor) "constructor"
-            else if (sym.isSetter) { cond = valAdvice ; s"var ${getterNameString(sym)}" }
+            else if (sym.isSetter) { cond = varAdvice(sym); s"var ${getterNameString(sym)}" }
             else if (sym.isVar || sym.isGetter && sym.accessed.isVar) s"var ${sym.nameString}"
             else if (sym.isVal || sym.isGetter && sym.accessed.isVal || sym.isLazy) s"val ${sym.nameString}"
             else if (sym.isMethod) s"method ${sym.nameString}"
@@ -739,7 +740,7 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
         for (defn <- unusedPrivates.unusedTypes if shouldWarnOn(defn.symbol)) { typeWarning(defn) }
 
         for (v <- unusedPrivates.unsetVars) {
-          emitUnusedWarning(v.pos, s"local var ${v.nameString} in ${v.owner} ${valAdvice}", WarningCategory.UnusedPrivates, v)
+          emitUnusedWarning(v.pos, s"local var ${v.nameString} in ${v.owner} ${varAdvice(v)}", WarningCategory.UnusedPrivates, v)
         }
       }
       if (settings.warnUnusedPatVars) {

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -92,6 +92,11 @@ trait StdAttachments {
    */
   case object PatVarDefAttachment extends PlainAttachment
 
+  /** Indicates that a definition was part of either a pattern or "sequence shorthand"
+   *  that introduced multiple definitions. All variables must be either `val` or `var`.
+   */
+  case object MultiDefAttachment extends PlainAttachment
+
   /** Identifies trees are either result or intermediate value of for loop desugaring.
    */
   case object ForAttachment extends PlainAttachment

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -61,6 +61,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.AutoApplicationAttachment
     this.NoWarnAttachment
     this.PatVarDefAttachment
+    this.MultiDefAttachment
     this.ForAttachment
     this.SyntheticUnitAttachment
     this.SubpatternsAttachment

test/files/neg/t7530.check

@@ -0,0 +1,12 @@
+t7530.scala:4: warning: private var j in class C is never updated: consider refactoring vars to a separate definition
+  private var (i, j) = init()
+                  ^
+t7530.scala:11: warning: private var j in class D is never updated: consider refactoring vars to a separate definition
+  private var i, j = init()
+                 ^
+t7530.scala:19: warning: private var j in class E is never updated: consider refactoring vars to a separate definition
+  private var K(i, j) = init()
+                   ^
+error: No warnings can be incurred under -Werror.
+3 warnings
+1 error

test/files/neg/t7530.scala

@@ -0,0 +1,24 @@
+//> using options -Werror -Xlint
+
+class C {
+  private var (i, j) = init()
+
+  private def init() = (42, 27)
+  def f(): Unit = println(i+j)
+  def g(): Unit = i += 1
+}
+class D {
+  private var i, j = init()
+
+  private def init() = 42
+  def f(): Unit = println(i+j)
+  def g(): Unit = i += 1
+}
+case class K(i: Int, j: Int)
+class E {
+  private var K(i, j) = init()
+
+  private def init() = K(42, 27)
+  def f(): Unit = println(i+j)
+  def g(): Unit = i += 1
+}