Bug 171190 - test262: test262/test/language/expressions/object/method-definition/early-errors-object-method-duplicate-parameters.js
Summary: test262: test262/test/language/expressions/object/method-definition/early-err...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
: 161408 (view as bug list)
Depends on: 170979
Blocks:
  Show dependency treegraph
 
Reported: 2017-04-22 23:17 PDT by Joseph Pecoraro
Modified: 2017-05-01 11:45 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (29.46 KB, patch)
2017-04-22 23:22 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (30.69 KB, patch)
2017-04-23 00:08 PDT, Joseph Pecoraro
saam: review+
saam: commit-queue-
Details | Formatted Diff | Diff
[PATCH] For Landing (37.56 KB, patch)
2017-04-24 23:11 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2017-04-22 23:17:40 PDT
test262/test/language/expressions/object/method-definition/early-errors-object-method-duplicate-parameters.js

Test:

    ({ async f(a, a) {} });

Expected:
SyntaxError: duplicate parameter

Actual:
No Syntax error.

Notes:
- Applies to any method syntax, not just async (see below).
- Chrome: SyntaxError: Duplicate parameter name not allowed in this context
- Firefox: SyntaxError: duplicate argument names not allowed in this context
- Edge: SyntaxError: Duplicate formal parameter names not allowed in this context

Spec:

https://tc39.github.io/ecma262/#sec-function-definitions-static-semantics-early-errors
14.1.2 Static Semantics: Early Errors

> UniqueFormalParameters : FormalParameters
> 
> It is a Syntax Error if BoundNames of FormalParameters contains any duplicate elements.
Comment 1 Joseph Pecoraro 2017-04-22 23:22:31 PDT
Created attachment 307920 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2017-04-22 23:47:35 PDT
*** Bug 161408 has been marked as a duplicate of this bug. ***
Comment 3 Joseph Pecoraro 2017-04-22 23:48:59 PDT
This might be conflicting due to the other patches I have out for review. But I need to put up a new patch anyways to include addressing the FIXME + tests in 161408.
Comment 4 Joseph Pecoraro 2017-04-23 00:07:52 PDT
Yes, this adds a tiny change on top of the `yield` patch, so it should come after. Marked as blocking. I'll attach my patch so it doesn't get lost.
Comment 5 Joseph Pecoraro 2017-04-23 00:08:31 PDT
Created attachment 307923 [details]
[PATCH] Proposed Fix
Comment 6 Saam Barati 2017-04-24 17:21:07 PDT
Comment on attachment 307923 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=307923&action=review

r=me

> JSTests/stress/async-await-syntax.js:143
> +    testSyntaxError(`var outerObject = { async method(a, a) {} }`);
> +    testSyntaxError(`var outerObject = { async ['meth' + 'od'](a, a) {} }`);
> +    testSyntaxError(`var outerObject = { async 'method'(a, a) {} }`);
> +    testSyntaxError(`var outerObject = { async 0(a, a) {} }`);

Can you also add some tests that use destructuring and the rest parameter?

> LayoutTests/js/script-tests/parser-syntax-check.js:887
> +invalid("({ foo(a,a){} });");
> +invalid("({ foo(b, c, b){} });");
> +invalid("({ *foo(a,a){} });");
> +invalid("({ *foo(b, c, b){} });");
> +invalid("({ async foo(a,a){} });");
> +invalid("({ async foo(b, c, b){} });");
> +valid("({ foo: function(a,a){} });");
> +valid("({ foo: function(b, c, b){} });");
> +valid("({ foo: function*(a,a){} });");
> +valid("({ foo: function*(b, c, b){} });");
> +valid("({ foo: async function(a,a){} });");
> +valid("({ foo: async function(b, c, b){} });");

Can you also add some tests that use destructuring and the rest parameter?
Comment 7 Joseph Pecoraro 2017-04-24 23:11:47 PDT
Created attachment 308070 [details]
[PATCH] For Landing
Comment 8 WebKit Commit Bot 2017-04-24 23:52:08 PDT
Comment on attachment 308070 [details]
[PATCH] For Landing

Clearing flags on attachment: 308070

Committed r215723: <http://trac.webkit.org/changeset/215723>