Skip to content

Commit e6e5b14

Browse files
lrytzlrytz
committed
Merge pull request scala#4302 from som-snytt/issue/deprecate-named-param
SI-9140 Allow deprecating current parameter name
2 parents b40b81c + e99378f commit e6e5b14

File tree

6 files changed

+93
-71
lines changed

6 files changed

+93
-71
lines changed

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

Lines changed: 57 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package typechecker
99
import symtab.Flags._
1010
import scala.collection.mutable
1111
import scala.reflect.ClassTag
12+
import PartialFunction.{ cond => when }
1213

1314
/**
1415
* @author Lukas Rytz
@@ -537,64 +538,73 @@ trait NamesDefaults { self: Analyzer =>
537538
}
538539
}
539540

540-
/**
541-
* Removes name assignments from args. Additionally, returns an array mapping
542-
* argument indices from call-site-order to definition-site-order.
541+
/** Removes name assignments from args. Additionally, returns an array mapping
542+
* argument indices from call-site-order to definition-site-order.
543543
*
544-
* Verifies that names are not specified twice, positional args don't appear
545-
* after named ones.
544+
* Verifies that names are not specified twice, and positional args don't appear after named ones.
546545
*/
547546
def removeNames(typer: Typer)(args: List[Tree], params: List[Symbol]): (List[Tree], Array[Int]) = {
548547
implicit val context0 = typer.context
549-
// maps indices from (order written by user) to (order of definition)
550-
val argPos = Array.fill(args.length)(-1)
551-
var positionalAllowed = true
552-
val namelessArgs = mapWithIndex(args) { (arg, argIndex) =>
553-
arg match {
554-
case arg @ AssignOrNamedArg(Ident(name), rhs) =>
555-
def matchesName(param: Symbol) = !param.isSynthetic && (
556-
(param.name == name) || (param.deprecatedParamName match {
557-
case Some(`name`) =>
558-
context0.deprecationWarning(arg.pos, param,
559-
s"the parameter name $name has been deprecated. Use ${param.name} instead.")
560-
true
561-
case _ => false
562-
})
563-
)
564-
val paramPos = params indexWhere matchesName
565-
if (paramPos == -1) {
566-
if (positionalAllowed) {
567-
argPos(argIndex) = argIndex
568-
// prevent isNamed from being true when calling doTypedApply recursively,
569-
// treat the arg as an assignment of type Unit
570-
Assign(arg.lhs, rhs) setPos arg.pos
571-
}
572-
else UnknownParameterNameNamesDefaultError(arg, name)
573-
}
574-
else if (argPos contains paramPos) {
548+
def matchesName(param: Symbol, name: Name, argIndex: Int) = {
549+
def warn(w: String) = context0.deprecationWarning(args(argIndex).pos, param, w)
550+
def checkDeprecation(anonOK: Boolean) =
551+
when (param.deprecatedParamName) {
552+
case Some(`name`) => true
553+
case Some(nme.NO_NAME) => anonOK
554+
}
555+
def checkName = {
556+
val res = param.name == name
557+
if (res && checkDeprecation(true)) warn(s"naming parameter $name has been deprecated.")
558+
res
559+
}
560+
def checkAltName = {
561+
val res = checkDeprecation(false)
562+
if (res) warn(s"the parameter name $name has been deprecated. Use ${param.name} instead.")
563+
res
564+
}
565+
!param.isSynthetic && (checkName || checkAltName)
566+
}
567+
// argPos maps indices from (order written by user) to (order of definition)
568+
val argPos = Array.fill(args.length)(-1)
569+
val namelessArgs = {
570+
var positionalAllowed = true
571+
def stripNamedArg(arg: AssignOrNamedArg, argIndex: Int): Tree = {
572+
val AssignOrNamedArg(Ident(name), rhs) = arg
573+
params indexWhere (p => matchesName(p, name, argIndex)) match {
574+
case -1 if positionalAllowed =>
575+
// prevent isNamed from being true when calling doTypedApply recursively,
576+
// treat the arg as an assignment of type Unit
577+
Assign(arg.lhs, rhs) setPos arg.pos
578+
case -1 =>
579+
UnknownParameterNameNamesDefaultError(arg, name)
580+
case paramPos if argPos contains paramPos =>
575581
val existingArgIndex = argPos.indexWhere(_ == paramPos)
576-
val otherName = args(paramPos) match {
577-
case AssignOrNamedArg(Ident(oName), rhs) if oName != name => Some(oName)
578-
case _ => None
582+
val otherName = Some(args(paramPos)) collect {
583+
case AssignOrNamedArg(Ident(oName), _) if oName != name => oName
579584
}
580585
DoubleParamNamesDefaultError(arg, name, existingArgIndex+1, otherName)
581-
} else if (isAmbiguousAssignment(typer, params(paramPos), arg))
586+
case paramPos if isAmbiguousAssignment(typer, params(paramPos), arg) =>
582587
AmbiguousReferenceInNamesDefaultError(arg, name)
583-
else {
584-
// if the named argument is on the original parameter
585-
// position, positional after named is allowed.
586-
if (argIndex != paramPos)
587-
positionalAllowed = false
588-
argPos(argIndex) = paramPos
588+
case paramPos if paramPos != argIndex =>
589+
positionalAllowed = false // named arg is not in original parameter order: require names after this
590+
argPos(argIndex) = paramPos // fix up the arg position
589591
rhs
590-
}
591-
case _ =>
592-
argPos(argIndex) = argIndex
593-
if (positionalAllowed) arg
594-
else PositionalAfterNamedNamesDefaultError(arg)
592+
case _ => rhs
593+
}
594+
}
595+
mapWithIndex(args) {
596+
case (arg: AssignOrNamedArg, argIndex) =>
597+
val t = stripNamedArg(arg, argIndex)
598+
if (!t.isErroneous && argPos(argIndex) < 0) argPos(argIndex) = argIndex
599+
t
600+
case (arg, argIndex) =>
601+
if (positionalAllowed) {
602+
argPos(argIndex) = argIndex
603+
arg
604+
} else
605+
PositionalAfterNamedNamesDefaultError(arg)
595606
}
596607
}
597-
598608
(namelessArgs, argPos)
599609
}
600610
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2197,7 +2197,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
21972197
val allParams = meth.paramss.flatten
21982198
for (p <- allParams) {
21992199
for (n <- p.deprecatedParamName) {
2200-
if (allParams.exists(p1 => p1.name == n || (p != p1 && p1.deprecatedParamName.exists(_ == n))))
2200+
if (allParams.exists(p1 => p != p1 && (p1.name == n || p1.deprecatedParamName.exists(_ == n))))
22012201
DeprecatedParamNameError(p, n)
22022202
}
22032203
}

‎src/library/scala/deprecatedName.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,6 @@ import scala.annotation.meta._
2929
* @since 2.8.1
3030
*/
3131
@param
32-
class deprecatedName(name: Symbol) extends scala.annotation.StaticAnnotation
32+
class deprecatedName(name: Symbol) extends scala.annotation.StaticAnnotation {
33+
def this() = this(Symbol("<none>"))
34+
}

‎src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
858858
def isDeprecated = hasAnnotation(DeprecatedAttr)
859859
def deprecationMessage = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 0)
860860
def deprecationVersion = getAnnotation(DeprecatedAttr) flatMap (_ stringArg 1)
861-
def deprecatedParamName = getAnnotation(DeprecatedNameAttr) flatMap (_ symbolArg 0)
861+
def deprecatedParamName = getAnnotation(DeprecatedNameAttr) flatMap (_ symbolArg 0 orElse Some(nme.NO_NAME))
862862
def hasDeprecatedInheritanceAnnotation
863863
= hasAnnotation(DeprecatedInheritanceAttr)
864864
def deprecatedInheritanceMessage

‎test/files/neg/names-defaults-neg.check

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -118,68 +118,74 @@ names-defaults-neg.scala:93: warning: the parameter name y has been deprecated.
118118
names-defaults-neg.scala:93: error: parameter 'b' is already specified at parameter position 1
119119
deprNam3(y = 10, b = 2)
120120
^
121-
names-defaults-neg.scala:98: error: unknown parameter name: m
121+
names-defaults-neg.scala:96: warning: naming parameter deprNam4Arg has been deprecated.
122+
deprNam4(deprNam4Arg = null)
123+
^
124+
names-defaults-neg.scala:98: warning: naming parameter deprNam5Arg has been deprecated.
125+
deprNam5(deprNam5Arg = null)
126+
^
127+
names-defaults-neg.scala:102: error: unknown parameter name: m
122128
f3818(y = 1, m = 1)
123129
^
124-
names-defaults-neg.scala:131: error: reference to var2 is ambiguous; it is both a method parameter and a variable in scope.
130+
names-defaults-neg.scala:135: error: reference to var2 is ambiguous; it is both a method parameter and a variable in scope.
125131
delay(var2 = 40)
126132
^
127-
names-defaults-neg.scala:134: error: missing parameter type for expanded function ((x$1) => a = x$1)
133+
names-defaults-neg.scala:138: error: missing parameter type for expanded function ((x$1) => a = x$1)
128134
val taf2: Int => Unit = testAnnFun(a = _, b = get("+"))
129135
^
130-
names-defaults-neg.scala:134: error: not found: value a
136+
names-defaults-neg.scala:138: error: not found: value a
131137
val taf2: Int => Unit = testAnnFun(a = _, b = get("+"))
132138
^
133-
names-defaults-neg.scala:134: error: not found: value get
139+
names-defaults-neg.scala:138: error: not found: value get
134140
val taf2: Int => Unit = testAnnFun(a = _, b = get("+"))
135141
^
136-
names-defaults-neg.scala:135: error: parameter 'a' is already specified at parameter position 1
142+
names-defaults-neg.scala:139: error: parameter 'a' is already specified at parameter position 1
137143
val taf3 = testAnnFun(b = _: String, a = get(8))
138144
^
139-
names-defaults-neg.scala:136: error: missing parameter type for expanded function ((x$3) => testAnnFun(x$3, ((x$4) => b = x$4)))
145+
names-defaults-neg.scala:140: error: missing parameter type for expanded function ((x$3) => testAnnFun(x$3, ((x$4) => b = x$4)))
140146
val taf4: (Int, String) => Unit = testAnnFun(_, b = _)
141147
^
142-
names-defaults-neg.scala:136: error: missing parameter type for expanded function ((x$4) => b = x$4)
148+
names-defaults-neg.scala:140: error: missing parameter type for expanded function ((x$4) => b = x$4)
143149
val taf4: (Int, String) => Unit = testAnnFun(_, b = _)
144150
^
145-
names-defaults-neg.scala:136: error: not found: value b
151+
names-defaults-neg.scala:140: error: not found: value b
146152
val taf4: (Int, String) => Unit = testAnnFun(_, b = _)
147153
^
148-
names-defaults-neg.scala:144: error: variable definition needs type because 'x' is used as a named argument in its body.
154+
names-defaults-neg.scala:148: error: variable definition needs type because 'x' is used as a named argument in its body.
149155
def t3 { var x = t.f(x = 1) }
150156
^
151-
names-defaults-neg.scala:147: error: variable definition needs type because 'x' is used as a named argument in its body.
157+
names-defaults-neg.scala:151: error: variable definition needs type because 'x' is used as a named argument in its body.
152158
object t6 { var x = t.f(x = 1) }
153159
^
154-
names-defaults-neg.scala:147: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
160+
names-defaults-neg.scala:151: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
155161
in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for x.
156162
object t6 { var x = t.f(x = 1) }
157163
^
158-
names-defaults-neg.scala:150: error: variable definition needs type because 'x' is used as a named argument in its body.
164+
names-defaults-neg.scala:154: error: variable definition needs type because 'x' is used as a named argument in its body.
159165
class t9 { var x = t.f(x = 1) }
160166
^
161-
names-defaults-neg.scala:150: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
167+
names-defaults-neg.scala:154: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
162168
in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for x.
163169
class t9 { var x = t.f(x = 1) }
164170
^
165-
names-defaults-neg.scala:164: error: variable definition needs type because 'x' is used as a named argument in its body.
171+
names-defaults-neg.scala:168: error: variable definition needs type because 'x' is used as a named argument in its body.
166172
def u3 { var x = u.f(x = 1) }
167173
^
168-
names-defaults-neg.scala:167: error: variable definition needs type because 'x' is used as a named argument in its body.
174+
names-defaults-neg.scala:171: error: variable definition needs type because 'x' is used as a named argument in its body.
169175
def u6 { var x = u.f(x = "32") }
170176
^
171-
names-defaults-neg.scala:170: error: reference to x is ambiguous; it is both a method parameter and a variable in scope.
177+
names-defaults-neg.scala:174: error: reference to x is ambiguous; it is both a method parameter and a variable in scope.
172178
def u9 { var x: Int = u.f(x = 1) }
173179
^
174-
names-defaults-neg.scala:177: error: variable definition needs type because 'x' is used as a named argument in its body.
180+
names-defaults-neg.scala:181: error: variable definition needs type because 'x' is used as a named argument in its body.
175181
class u15 { var x = u.f(x = 1) }
176182
^
177-
names-defaults-neg.scala:177: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
183+
names-defaults-neg.scala:181: warning: type-checking the invocation of method f checks if the named argument expression 'x = ...' is a valid assignment
178184
in the current scope. The resulting type inference error (see above) can be fixed by providing an explicit type in the local definition for x.
179185
class u15 { var x = u.f(x = 1) }
180186
^
181-
names-defaults-neg.scala:180: error: reference to x is ambiguous; it is both a method parameter and a variable in scope.
187+
names-defaults-neg.scala:184: error: reference to x is ambiguous; it is both a method parameter and a variable in scope.
182188
class u18 { var x: Int = u.f(x = 1) }
183189
^
184-
four warnings found
190+
6 warnings found
185191
46 errors found

‎test/files/neg/names-defaults-neg.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ object Test extends App {
9292
def deprNam3(@deprecatedName('x) a: Int, @deprecatedName('y) b: Int) = a + b
9393
deprNam3(y = 10, b = 2)
9494

95+
def deprNam4(@deprecatedName('deprNam4Arg) deprNam4Arg: String) = 0
96+
deprNam4(deprNam4Arg = null)
97+
def deprNam5(@deprecatedName deprNam5Arg: String) = 0
98+
deprNam5(deprNam5Arg = null)
9599

96100
// t3818
97101
def f3818(x: Int = 1, y: Int, z: Int = 1) = 0

0 commit comments

Comments
 (0)