From efb9a12811b5707410b0e21ad13f498d477d3746 Mon Sep 17 00:00:00 2001 From: "Lasse R.H. Nielsen" Date: Fri, 14 Oct 2016 11:25:25 +0200 Subject: [PATCH] Add `=` as default-value separator for named parameters. Fixes issue #27559. BUG= http://dartbug.com/27559 R=asiva@google.com, eernst@google.com, floitsch@google.com, sigmund@google.com Review URL: https://codereview.chromium.org/2411633002 . --- docs/language/dartLangSpec.tex | 45 +++++---- pkg/analyzer/lib/error/error.dart | 1 - .../lib/src/dart/error/syntactic_errors.dart | 4 - pkg/analyzer/lib/src/generated/parser.dart | 6 +- pkg/analyzer/test/generated/parser_test.dart | 8 -- .../lib/src/diagnostics/messages.dart | 14 --- pkg/compiler/lib/src/parser/parser.dart | 2 - runtime/vm/parser.cc | 2 +- tests/co19/co19-co19.status | 2 + .../named_parameters_default_eq_test.dart | 96 +++++++++++++++++++ 10 files changed, 129 insertions(+), 51 deletions(-) create mode 100644 tests/language/named_parameters_default_eq_test.dart diff --git a/docs/language/dartLangSpec.tex b/docs/language/dartLangSpec.tex index 9a6ad52bbfa..d0f62f2bbce 100644 --- a/docs/language/dartLangSpec.tex +++ b/docs/language/dartLangSpec.tex @@ -741,14 +741,25 @@ Optional parameters may be specified and provided with default values. \begin{grammar} {\bf defaultFormalParameter:} - normalFormalParameter ('=' expression)? + normalFormalParameter (`=' expression)? . -{\bf defaultNamedParameter:} +{\bf defaultNamedParameter:}normalFormalParameter (`=' expression)?; normalFormalParameter ( `{\escapegrammar :}' expression)? . \end{grammar} +A {\bf defaultNamedParameter} on the form: +\begin{code} + normalFormalParameter : expression +\end{code} +is equivalent to one on the form: +\begin{code} + normalFormalParameter = expression +\end{code} +The colon-syntax is included only for backwards compatibility. +It is deprecated and will be removed in a later version of the language specification. + \LMHash{} It is a compile-time error if the default value of an optional parameter is not a compile-time constant (\ref{constants}). If no default is explicitly specified for an optional parameter an implicit default of \NULL{} is provided. @@ -3150,7 +3161,7 @@ Here, a naive definition of $flatten$ diverges; there is not even a fixed point. \LMHash{} The static type of a function literal of the form -$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} : d_1, \ldots, T_{n+k}$ $x_{n+k} : d_k\}) => e$ +$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} = d_1, \ldots, T_{n+k}$ $x_{n+k} = d_k\}) => e$ is $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow T_0$, where $T_0$ is the static type of $e$. @@ -3158,7 +3169,7 @@ $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow \LMHash{} The static type of a function literal of the form -$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} : d_1, \ldots, T_{n+k}$ $x_{n+k} : d_k\})$ \ASYNC{} $=> e$ +$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} = d_1, \ldots, T_{n+k}$ $x_{n+k} = d_k\})$ \ASYNC{} $=> e$ is $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow Future$, where $T_0$ is the static type of $e$. @@ -3199,21 +3210,21 @@ is $(T_1 \ldots, T_n, [T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}]) \rightarro \LMHash{} The static type of a function literal of the form -$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} : d_1, \ldots, T_{n+k}$ $x_{n+k} : d_k\})$ $\ASYNC{}$ $\{s\}$ +$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} = d_1, \ldots, T_{n+k}$ $x_{n+k} = d_k\})$ $\ASYNC{}$ $\{s\}$ is $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow Future{}$. \LMHash{} The static type of a function literal of the form -$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} : d_1, \ldots, T_{n+k}$ $x_{n+k} : d_k\})$ $\ASYNC*{}$ $\{s\}$ +$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} = d_1, \ldots, T_{n+k}$ $x_{n+k} = d_k\})$ $\ASYNC*{}$ $\{s\}$ is $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow Stream{}$. \LMHash{} The static type of a function literal of the form -$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} : d_1, \ldots, T_{n+k}$ $x_{n+k} : d_k\})$ $\SYNC*{}$ $\{s\}$ +$(T_1$ $a_1, \ldots, T_n$ $a_n, \{T_{n+1}$ $x_{n+1} = d_1, \ldots, T_{n+k}$ $x_{n+k} = d_k\})$ $\SYNC*{}$ $\{s\}$ is $(T_1 \ldots, T_n, \{T_{n+1}$ $x_{n+1}, \ldots, T_{n+k}$ $x_{n+k}\}) \rightarrow Iterable{}$. @@ -3859,15 +3870,15 @@ If the method lookup succeeded, the body of $f$ is executed with respect to the If the method lookup has failed, then let $g$ be the result of looking up getter (\ref{getterAndSetterLookup}) $m$ in $v_o$ with respect to $L$. If $v_o$ is an instance of \code{Type} but $o$ is not a constant type literal, then if $g$ is a getter that forwards to a static getter, getter lookup fails. If the getter lookup succeeded, let $v_g$ be the value of the getter invocation $o.m$. Then the value of $i$ is the result of invoking -the static method \code{Function.apply()} with arguments $v.g, [o_1, \ldots , o_n], \{x_{n+1}: o_{n+1}, \ldots , x_{n+k}: o_{n+k}\}$. +the static method \code{Function.apply()} with arguments $v.g, [o_1, \ldots , o_n], \{\#x_{n+1}: o_{n+1}, \ldots , \#x_{n+k}: o_{n+k}\}$. \LMHash{} -If getter lookup has also failed, then a new instance $im$ of the predefined class \code{Invocation} is created, such that : +If getter lookup has also failed, then a new instance $im$ of the predefined class \code{Invocation} is created, such that: \begin{itemize} \item \code{im.isMethod} evaluates to \code{\TRUE{}}. \item \code{im.memberName} evaluates to the symbol \code{m}. -\item \code{im.positionalArguments} evaluates to an immutable list with the same values as \code{[$o_1, \ldots, o_n$]}. -\item \code{im.namedArguments} evaluates to an immutable map with the same keys and values as \code{\{$x_{n+1}: o_{n+1}, \ldots, x_{n+k} : o_{n+k}$\}}. +\item \code{im.positionalArguments} evaluates to an immutable list with the same values as \code{[$o_1, \ldots, o_n$]}. +\item \code{im.namedArguments} evaluates to an immutable map with the same keys and values as \code{\{$\#x_{n+1}: o_{n+1}, \ldots, \#x_{n+k} : o_{n+k}$\}}. \end{itemize} \LMHash{} @@ -3960,7 +3971,7 @@ If the method lookup succeeded, the body of $f$ is executed with respect to the \LMHash{} If the method lookup has failed, then let $g$ be the result of looking up getter (\ref{getterAndSetterLookup}) $m$ in $S_{dynamic}$ with respect to $L$. If the getter lookup succeeded, let $v_g$ be the value of the getter invocation $\SUPER{}.m$. Then the value of $i$ is the result of invoking -the static method \code{Function.apply()} with arguments $v_g, [o_1, \ldots , o_n], \{x_{n+1}: o_{n+1}, \ldots , x_{n+k}: o_{n+k}\}$. +the static method \code{Function.apply()} with arguments $v_g, [o_1, \ldots , o_n], \{x_{n+1} = o_{n+1}, \ldots , x_{n+k} = o_{n+k}\}$. \LMHash{} If getter lookup has also failed, then a new instance $im$ of the predefined class \code{Invocation} is created, such that : @@ -3968,7 +3979,7 @@ If getter lookup has also failed, then a new instance $im$ of the predefined c \item \code{im.isMethod} evaluates to \code{\TRUE{}}. \item \code{im.memberName} evaluates to the symbol \code{m}. \item \code{im.positionalArguments} evaluates to an immutable list with the same values as \code{[$o_1, \ldots, o_n$]}. -\item \code{im.namedArguments} evaluates to an immutable map with the same keys and values as \code{\{$x_{n+1}: o_{n+1}, \ldots, x_{n+k} : o_{n+k}$\}}. +\item \code{im.namedArguments} evaluates to an immutable map with the same keys and values as \code{\{$\#x_{n+1}: o_{n+1}, \ldots, \#x_{n+k} : o_{n+k}$\}}. \end{itemize} Then the method \code{noSuchMethod()} is looked up in $S_{dynamic}$ and invoked on \THIS{} with argument $im$, and the result of this invocation is the result of evaluating $i$. However, if the implementation found cannot be invoked with a single positional argument, the implementation of \code{noSuchMethod()} in class \code{Object} is invoked on \THIS{} with argument $im'$, where $im'$ is an instance of \code{Invocation} such that : \begin{itemize} @@ -4273,7 +4284,7 @@ The {\em closurization of method $f$ on object $o$} is defined to be equivalent \item $(a, b) \{\RETURN{}$ $u[a] = b;$\} if $f$ is named $[]=$. \item \begin{dartCode} -$(r_1, \ldots, r_n, \{p_1 : d_1, \ldots , p_k : d_k\})$ \{ +$(r_1, \ldots, r_n, \{p_1 = d_1, \ldots , p_k = d_k\})$ \{ \RETURN{} $ u.m(r_1, \ldots, r_n, p_1: p_1, \ldots, p_k: p_k);$ \} \end{dartCode} @@ -4323,7 +4334,7 @@ The {\em closurization of constructor $f$ of type $T$} is defined to be equivale \begin{itemize} \item \begin{dartCode} -$(r_1, \ldots, r_n, \{p_1 : d_1, \ldots , p_k : d_k\})$ \{ +$(r_1, \ldots, r_n, \{p_1 = d_1, \ldots , p_k = d_k\})$ \{ \RETURN{} \NEW{} $T.m(r_1, \ldots, r_n, p_1: p_1, \ldots, p_k: p_k);$ \} \end{dartCode} @@ -4354,7 +4365,7 @@ The {\em closurization of anonymous constructor $f$ of type $T$} is defined to b \begin{itemize} \item \begin{dartCode} -$(r_1, \ldots, r_n, \{p_1 : d_1, \ldots , p_k : d_k\})$ \{ +$(r_1, \ldots, r_n, \{p_1 = d_1, \ldots , p_k = d_k\})$ \{ \RETURN{} \NEW{} $T(r_1, \ldots, r_n, p_1: p_1, \ldots, p_k: p_k);$ \} \end{dartCode} @@ -4388,7 +4399,7 @@ The {\em closurization of method $f$ with respect to superclass $S$} is defined \item $(a, b) \{\RETURN{}$ $\SUPER[a] = b;$\} if $f$ is named $[]=$. \item \begin{dartCode} -$(r_1, \ldots, r_n, \{p_1 : d_1, \ldots , p_k : d_k\})$ \{ +$(r_1, \ldots, r_n, \{p_1 = d_1, \ldots , p_k = d_k\})$ \{ \RETURN{} \SUPER$.m(r_1, \ldots, r_n, p_1: p_1, \ldots, p_k: p_k);$ \} \end{dartCode} diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart index 712e6ae48b6..648a447e14a 100644 --- a/pkg/analyzer/lib/error/error.dart +++ b/pkg/analyzer/lib/error/error.dart @@ -798,7 +798,6 @@ abstract class ErrorCode { ParserErrorCode.UNEXPECTED_TOKEN, ParserErrorCode.WITH_BEFORE_EXTENDS, ParserErrorCode.WITH_WITHOUT_EXTENDS, - ParserErrorCode.WRONG_SEPARATOR_FOR_NAMED_PARAMETER, ParserErrorCode.WRONG_SEPARATOR_FOR_POSITIONAL_PARAMETER, ParserErrorCode.WRONG_TERMINATOR_FOR_PARAMETER_GROUP, ParserErrorCode.VAR_AND_TYPE, diff --git a/pkg/analyzer/lib/src/dart/error/syntactic_errors.dart b/pkg/analyzer/lib/src/dart/error/syntactic_errors.dart index 31ee7269016..706abd1d23a 100644 --- a/pkg/analyzer/lib/src/dart/error/syntactic_errors.dart +++ b/pkg/analyzer/lib/src/dart/error/syntactic_errors.dart @@ -614,10 +614,6 @@ class ParserErrorCode extends ErrorCode { 'WITH_WITHOUT_EXTENDS', "The with clause cannot be used without an extends clause"); - static const ParserErrorCode WRONG_SEPARATOR_FOR_NAMED_PARAMETER = - const ParserErrorCode('WRONG_SEPARATOR_FOR_NAMED_PARAMETER', - "The default value of a named parameter should be preceeded by ':'"); - static const ParserErrorCode WRONG_SEPARATOR_FOR_POSITIONAL_PARAMETER = const ParserErrorCode('WRONG_SEPARATOR_FOR_POSITIONAL_PARAMETER', "The default value of a positional parameter should be preceeded by '='"); diff --git a/pkg/analyzer/lib/src/generated/parser.dart b/pkg/analyzer/lib/src/generated/parser.dart index fb111fff621..1f51d664820 100644 --- a/pkg/analyzer/lib/src/generated/parser.dart +++ b/pkg/analyzer/lib/src/generated/parser.dart @@ -2780,6 +2780,7 @@ class Parser { * normalFormalParameter ('=' expression)? * * defaultNamedParameter ::= + * normalFormalParameter ('=' expression)? * normalFormalParameter (':' expression)? */ FormalParameter parseFormalParameter(ParameterKind kind) { @@ -2788,10 +2789,7 @@ class Parser { if (type == TokenType.EQ) { Token separator = getAndAdvance(); Expression defaultValue = parseExpression2(); - if (kind == ParameterKind.NAMED) { - _reportErrorForToken( - ParserErrorCode.WRONG_SEPARATOR_FOR_NAMED_PARAMETER, separator); - } else if (kind == ParameterKind.REQUIRED) { + if (kind == ParameterKind.REQUIRED) { _reportErrorForNode( ParserErrorCode.POSITIONAL_PARAMETER_OUTSIDE_GROUP, parameter); } diff --git a/pkg/analyzer/test/generated/parser_test.dart b/pkg/analyzer/test/generated/parser_test.dart index 0fbcf084956..2e46dfeb9b9 100644 --- a/pkg/analyzer/test/generated/parser_test.dart +++ b/pkg/analyzer/test/generated/parser_test.dart @@ -2770,14 +2770,6 @@ void main() { listener.assertErrorsWithCodes([ParserErrorCode.WITH_WITHOUT_EXTENDS]); } - void test_wrongSeparatorForNamedParameter() { - createParser('(a, {b = 0})'); - FormalParameterList list = parser.parseFormalParameterList(); - expectNotNullIfNoErrors(list); - listener.assertErrorsWithCodes( - [ParserErrorCode.WRONG_SEPARATOR_FOR_NAMED_PARAMETER]); - } - void test_wrongSeparatorForPositionalParameter() { createParser('(a, [b : 0])'); FormalParameterList list = parser.parseFormalParameterList(); diff --git a/pkg/compiler/lib/src/diagnostics/messages.dart b/pkg/compiler/lib/src/diagnostics/messages.dart index e17157442c7..63fa370d6cd 100644 --- a/pkg/compiler/lib/src/diagnostics/messages.dart +++ b/pkg/compiler/lib/src/diagnostics/messages.dart @@ -326,7 +326,6 @@ enum MessageKind { MULTI_INHERITANCE, NAMED_ARGUMENT_NOT_FOUND, NAMED_FUNCTION_EXPRESSION, - NAMED_PARAMETER_WITH_EQUALS, NATIVE_NOT_SUPPORTED, NO_BREAK_TARGET, NO_CATCH_NOR_FINALLY, @@ -1372,19 +1371,6 @@ main() { }""" ]), - MessageKind.NAMED_PARAMETER_WITH_EQUALS: const MessageTemplate( - MessageKind.NAMED_PARAMETER_WITH_EQUALS, - "Named optional parameters can't use '=' to specify a default " - "value.", - howToFix: "Try replacing '=' with ':'.", - examples: const [ - """ -main() { - foo({a = 1}) => print(a); - foo(a: 2); -}""" - ]), - MessageKind.POSITIONAL_PARAMETER_WITH_EQUALS: const MessageTemplate( MessageKind.POSITIONAL_PARAMETER_WITH_EQUALS, "Positional optional parameters can't use ':' to specify a " diff --git a/pkg/compiler/lib/src/parser/parser.dart b/pkg/compiler/lib/src/parser/parser.dart index 0d451624440..3cac0379d32 100644 --- a/pkg/compiler/lib/src/parser/parser.dart +++ b/pkg/compiler/lib/src/parser/parser.dart @@ -480,8 +480,6 @@ class Parser { if (type.isRequired) { listener.reportError( equal, MessageKind.REQUIRED_PARAMETER_WITH_DEFAULT); - } else if (type.isNamed && identical('=', value)) { - listener.reportError(equal, MessageKind.NAMED_PARAMETER_WITH_EQUALS); } else if (type.isPositional && identical(':', value)) { listener.reportError( equal, MessageKind.POSITIONAL_PARAMETER_WITH_EQUALS); diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc index 66ef0266d73..20886521af0 100644 --- a/runtime/vm/parser.cc +++ b/runtime/vm/parser.cc @@ -2185,7 +2185,7 @@ void Parser::ParseFormalParameter(bool allow_explicit_default_value, if (params->has_optional_positional_parameters) { ExpectToken(Token::kASSIGN); } else { - ExpectToken(Token::kCOLON); + ConsumeToken(); } params->num_optional_parameters++; params->has_explicit_default_values = true; // Also if explicitly NULL. diff --git a/tests/co19/co19-co19.status b/tests/co19/co19-co19.status index 8b1627d10ee..0934228c8f3 100644 --- a/tests/co19/co19-co19.status +++ b/tests/co19/co19-co19.status @@ -18,6 +18,8 @@ LibTest/typed_data/ByteData/buffer_A01_t01: Fail # co19 r736 bug - sent comment. LibTest/core/RegExp/firstMatch_A01_t01: Fail # co19 issue 742 +Language/Functions/Formal_Parameters/Optional_Formals/syntax_t14: Fail # co19 issue 80 + [ $system == linux ] LibTest/collection/ListBase/ListBase_class_A01_t01: Fail # co19 issue 72 LibTest/collection/ListBase/ListBase_class_A01_t02: Fail # co19 issue 72 diff --git a/tests/language/named_parameters_default_eq_test.dart b/tests/language/named_parameters_default_eq_test.dart new file mode 100644 index 00000000000..83abffb93dd --- /dev/null +++ b/tests/language/named_parameters_default_eq_test.dart @@ -0,0 +1,96 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Check that both `=` and `:` are allowed for named parameters. + +import "package:expect/expect.dart"; + +// Default values are not allowed on typedefs. +typedef int F1({x = 3, y}); /// 01: compile-time error + +typedef int functype({x, y, z}); + +int topF({x = 3, y : 5, z}) => x * y * (z ?? 2); + +class A { + int x; + int y; + int z; + A({this.x = 3, this.y : 5, z }) : z = z ?? 2; + A.redirect({x = 3, y : 5, z}) : this(x: x, y: y, z: z); + factory A.factory({x = 3, y : 5, z}) => new A(x: x, y: y, z: z ?? 2); + factory A.redirectFactory({x, y, z}) = A; + + // Default values are not allowed on redirecting factory constructors. + factory A.badRedirectFactory({x = 3, y}) = A; /// 02: compile-time error + + int get value => x * y * z; + + static int staticF({x = 3, y : 5, z}) => x * y * (z ?? 2); + int instanceF({x = 3, y : 5, z}) => x * y * (z ?? 2); +} + +main() { + var a = new A(); + + int local({x = 3, y : 5, z}) => x * y * (z ?? 2); + var expr = ({x = 3, y : 5, z}) => x * y * (z ?? 2); + var tearOff = a.instanceF; + + test(function) { + Expect.equals(30, function()); + Expect.equals(70, function(x: 7)); + Expect.equals(42, function(y: 7)); + Expect.equals(28, function(x: 7, y: 2)); + Expect.equals(15, function(z: 1)); + Expect.equals(21, function(y: 7, z: 1)); + Expect.equals(35, function(x: 7, z: 1)); + Expect.equals(14, function(x: 7, y: 2, z: 1)); + Expect.isTrue(function is functype); + } + + test(topF); + test(A.staticF); + test(a.instanceF); + test(local); + test(expr); + test(tearOff); + + // Can't tear off constructors. + Expect.equals(30, new A().value); + Expect.equals(70, new A(x: 7).value); + Expect.equals(42, new A(y: 7).value); + Expect.equals(28, new A(x: 7, y: 2).value); + Expect.equals(15, new A(z: 1).value); + Expect.equals(21, new A(y: 7, z: 1).value); + Expect.equals(35, new A(x: 7, z: 1).value); + Expect.equals(14, new A(x: 7, y: 2, z: 1).value); + + Expect.equals(30, new A.redirect().value); + Expect.equals(70, new A.redirect(x: 7).value); + Expect.equals(42, new A.redirect(y: 7).value); + Expect.equals(28, new A.redirect(x: 7, y: 2).value); + Expect.equals(15, new A.redirect(z: 1).value); + Expect.equals(21, new A.redirect(y: 7, z: 1).value); + Expect.equals(35, new A.redirect(x: 7, z: 1).value); + Expect.equals(14, new A.redirect(x: 7, y: 2, z: 1).value); + + Expect.equals(30, new A.factory().value); + Expect.equals(70, new A.factory(x: 7).value); + Expect.equals(42, new A.factory(y: 7).value); + Expect.equals(28, new A.factory(x: 7, y: 2).value); + Expect.equals(15, new A.factory(z: 1).value); + Expect.equals(21, new A.factory(y: 7, z: 1).value); + Expect.equals(35, new A.factory(x: 7, z: 1).value); + Expect.equals(14, new A.factory(x: 7, y: 2, z: 1).value); + + Expect.equals(30, new A.redirectFactory().value); + Expect.equals(70, new A.redirectFactory(x: 7).value); + Expect.equals(42, new A.redirectFactory(y: 7).value); + Expect.equals(28, new A.redirectFactory(x: 7, y: 2).value); + Expect.equals(15, new A.redirectFactory(z: 1).value); + Expect.equals(21, new A.redirectFactory(y: 7, z: 1).value); + Expect.equals(35, new A.redirectFactory(x: 7, z: 1).value); + Expect.equals(14, new A.redirectFactory(x: 7, y: 2, z: 1).value); +}