Skip to content

Commit

Permalink
Lint multiarg infix operand
Browse files Browse the repository at this point in the history
Lint multiarg infix applications `x op (a, b, c)`
and also method definitions with multiple parameters
where the method name is either an operator identifier
or a plain identifier ending in an operator character.
  • Loading branch information
som-snytt committed May 3, 2020
1 parent 43b70d4 commit 1f53824
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 17 deletions.
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -381,6 +381,7 @@ object Reporting {
object LintBynameImplicit extends Lint; add(LintBynameImplicit)
object LintRecurseWithDefault extends Lint; add(LintRecurseWithDefault)
object LintUnitSpecialization extends Lint; add(LintUnitSpecialization)
object LintMultiargInfix extends Lint; add(LintMultiargInfix)

sealed trait Feature extends WarningCategory { override def summaryCategory: WarningCategory = Feature }
object Feature extends Feature { override def includes(o: WarningCategory): Boolean = o.isInstanceOf[Feature] }; add(Feature)
Expand Down
43 changes: 26 additions & 17 deletions src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
Expand Up @@ -16,12 +16,11 @@
package scala.tools.nsc
package ast.parser

import scala.collection.mutable
import mutable.ListBuffer
import scala.reflect.internal.{Precedence, ModifierFlags => Flags}
import scala.annotation.tailrec
import scala.collection.mutable, mutable.ListBuffer
import scala.reflect.internal.{Chars, ModifierFlags => Flags, Precedence}
import scala.reflect.internal.util.{FreshNameCreator, ListOfNil, Position, SourceFile}
import Tokens._
import scala.annotation.tailrec
import scala.tools.nsc.Reporting.WarningCategory

/** Historical note: JavaParsers started life as a direct copy of Parsers
Expand Down Expand Up @@ -868,11 +867,18 @@ self =>
}
}
}
def mkNamed(args: List[Tree]) = if (isExpr) args map treeInfo.assignmentToMaybeNamedArg else args
def mkNamed(args: List[Tree]) = if (isExpr) args.map(treeInfo.assignmentToMaybeNamedArg) else args
var isMultiarg = false
val arguments = right match {
case Parens(Nil) => literalUnit :: Nil
case Parens(args) => mkNamed(args)
case _ => right :: Nil
case Parens(Nil) => literalUnit :: Nil
case Parens(args @ (_ :: Nil)) => mkNamed(args)
case Parens(args) => isMultiarg = true ; mkNamed(args)
case _ => right :: Nil
}
def mkApply(fun: Tree, args: List[Tree]) = {
val apply = Apply(fun, args)
if (isMultiarg) apply.updateAttachment(MultiargInfixAttachment)
apply
}
if (isExpr) {
if (rightAssoc) {
Expand All @@ -881,15 +887,12 @@ self =>
val liftedArg = atPos(left.pos) {
ValDef(Modifiers(FINAL | SYNTHETIC | ARTIFACT), x, TypeTree(), stripParens(left))
}
Block(
liftedArg :: Nil,
Apply(mkSelection(right), List(Ident(x) setPos left.pos.focus)))
} else {
Apply(mkSelection(left), arguments)
}
} else {
Apply(Ident(op.encode), stripParens(left) :: arguments)
}
val apply = mkApply(mkSelection(right), List(Ident(x) setPos left.pos.focus))
Block(liftedArg :: Nil, apply)
} else
mkApply(mkSelection(left), arguments)
} else
mkApply(Ident(op.encode), stripParens(left) :: arguments)
}

/* --------- OPERAND/OPERATOR STACK --------------------------------------- */
Expand Down Expand Up @@ -2796,6 +2799,12 @@ self =>
}
expr()
}
if (settings.multiargInfix)
vparamss match {
case (_ :: _ :: _) :: Nil if settings.multiargInfix && Chars.isOperatorPart(name.decoded.last) =>
warning(nameOffset, "multiarg infix syntax looks like a tuple and will be deprecated", WarningCategory.LintMultiargInfix)
case _ =>
}
DefDef(newmods, name.toTermName, tparams, vparamss, restype, rhs)
}
signalParseProgress(result.pos)
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -192,6 +192,7 @@ trait Warnings {
val ByNameImplicit = LintWarning("byname-implicit", "Block adapted by implicit with by-name parameter.")
val RecurseWithDefault = LintWarning("recurse-with-default", "Recursive call used default argument.")
val UnitSpecialization = LintWarning("unit-special", "Warn for specialization of Unit in parameter position.")
val MultiargInfix = LintWarning("multiarg-infix", "Infix operator was defined or used with multiarg operand.")

def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
}
Expand Down Expand Up @@ -223,6 +224,7 @@ trait Warnings {
def warnByNameImplicit = lint contains ByNameImplicit
def warnRecurseWithDefault = lint contains RecurseWithDefault
def unitSpecialization = lint contains UnitSpecialization
def multiargInfix = lint contains MultiargInfix

// The Xlint warning group.
val lint = MultiChoiceSetting(
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -5127,6 +5127,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
// The enclosing context may be case c @ C(_) => or val c @ C(_) = v.
tree1 modifyType (_.finalResultType)
tree1
case tree1 @ Apply(_, args1) if tree.hasAttachment[MultiargInfixAttachment.type] && args1.lengthCompare(1) > 0 =>
context.warning(tree1.pos, "multiarg infix syntax looks like a tuple and will be deprecated", WarningCategory.LintMultiargInfix)
tree1
case tree1 => tree1
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/reflect/scala/reflect/internal/StdAttachments.scala
Expand Up @@ -119,4 +119,7 @@ trait StdAttachments {
class QualTypeSymAttachment(val sym: Symbol)

case object ConstructorNeedsFence extends PlainAttachment

/** Mark the syntax for linting purposes. */
case object MultiargInfixAttachment extends PlainAttachment
}
35 changes: 35 additions & 0 deletions test/files/neg/sd503.check
@@ -0,0 +1,35 @@
sd503.scala:18: error: not enough arguments for method x_=: (i: Int, j: Int): Unit.
Unspecified value parameter j.
def f4() = c.x = (42, 27) // missing arg
^
sd503.scala:23: error: type mismatch;
found : (Int, Int)
required: Int
def f7() = d.x = (42, 27) // type mismatch (same as doti)
^
sd503.scala:9: warning: multiarg infix syntax looks like a tuple and will be deprecated
def % (i: Int, j: Int) = i + j // operator, warn
^
sd503.scala:29: warning: multiarg infix syntax looks like a tuple and will be deprecated
def x_=(i: Int, j: Int): Unit = value = i + j // multiarg, warn
^
sd503.scala:34: warning: multiarg infix syntax looks like a tuple and will be deprecated
def x_=(i: Int, j: Int = 1): Unit = devalue = i + j // multiarg, warn
^
sd503.scala:47: warning: multiarg infix syntax looks like a tuple and will be deprecated
def +=(x: A, y: A, zs: A*): this.type = addAll(x +: y +: zs) // very multiarg, warn
^
sd503.scala:13: warning: multiarg infix syntax looks like a tuple and will be deprecated
def f1(t: T) = t m (1, 2) // multiarg, warn
^
sd503.scala:15: warning: multiarg infix syntax looks like a tuple and will be deprecated
def f3(t: T) = t % (1, 2) // multiarg, warn
^
sd503.scala:19: warning: multiarg infix syntax looks like a tuple and will be deprecated
def f5() = c x_= (42, 27) // multiarg, warn
^
sd503.scala:52: warning: multiarg infix syntax looks like a tuple and will be deprecated
def f[A](as: Embiggen[A], x: A, y: A, z: A): as.type = as += (x, y, z) // very multiarg, warn
^
8 warnings
2 errors
53 changes: 53 additions & 0 deletions test/files/neg/sd503.scala
@@ -0,0 +1,53 @@
// scalac: -Xlint:multiarg-infix
//
// lint multiarg infix syntax, e.g., vs += (1, 2, 3)
// Since infix is encouraged by symbolic operator names, discourage defining def += (x: A, y: A, zs: A*)

trait T {
def m(i: Int, j: Int) = i + j

def % (i: Int, j: Int) = i + j // operator, warn


def f0(t: T) = t.m(1, 2) // ok
def f1(t: T) = t m (1, 2) // multiarg, warn
def f2(t: T) = t.%(1, 2) // ok
def f3(t: T) = t % (1, 2) // multiarg, warn

def c = new C
def f4() = c.x = (42, 27) // missing arg
def f5() = c x_= (42, 27) // multiarg, warn

def d = new D
def f6() = d.x = 42 // ok!
def f7() = d.x = (42, 27) // type mismatch (same as doti)
}

class C {
private var value: Int = _
def x: Int = value
def x_=(i: Int, j: Int): Unit = value = i + j // multiarg, warn
}
class D {
private var devalue: Int = _ // d.value
def x: Int = devalue
def x_=(i: Int, j: Int = 1): Unit = devalue = i + j // multiarg, warn
}

// If the application is adapted such that what looks like a tuple is taken as a tuple,
// then don't warn; eventually this will be normal behavior.
trait OK {
def f(p: (Int, Int)) = p == (42, 17) // nowarn!
def g(ps: Embiggen[(Int, Int)]) = ps :+ (42, 17) // nowarn!
}

// Growable is the paradigmatic example
trait Embiggen[A] {
def addAll(as: Seq[A]): this.type
def +=(x: A, y: A, zs: A*): this.type = addAll(x +: y +: zs) // very multiarg, warn
def :+(a: A): Embiggen[A]
}

trait NotOK {
def f[A](as: Embiggen[A], x: A, y: A, z: A): as.type = as += (x, y, z) // very multiarg, warn
}

0 comments on commit 1f53824

Please sign in to comment.