Bug 882541 part 1. Don't require all arguments after an optional argument to be optional. r=khuey
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 11 Oct 2013 12:28:23 -0400
changeset 150526 877a227c502f07a77a1600edf3285665218ae0fb
parent 150525 46029c9fbfe4c0f6cdea54ed3763c5cefa67dca8
child 150527 d8636e485e85f186a2d86d2f8c0304c92d802112
push id3006
push userkwierso@gmail.com
push dateSat, 12 Oct 2013 02:00:44 +0000
treeherderfx-team@3ff6c5ec4dc5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs882541
milestone27.0a1
Bug 882541 part 1. Don't require all arguments after an optional argument to be optional. r=khuey
dom/bindings/parser/WebIDL.py
dom/bindings/parser/tests/test_dictionary.py
dom/bindings/parser/tests/test_optional_constraints.py
dom/bindings/parser/tests/test_overload.py
dom/bindings/parser/tests/test_variadic_constraints.py
--- a/dom/bindings/parser/WebIDL.py
+++ b/dom/bindings/parser/WebIDL.py
@@ -3138,35 +3138,32 @@ class IDLMethod(IDLInterfaceMember, IDLS
         return self
 
     def signatures(self):
         return [(overload.returnType, overload.arguments) for overload in
                 self._overloads]
 
     def finish(self, scope):
         for overload in self._overloads:
-            inOptionalArguments = False
             variadicArgument = None
 
             arguments = overload.arguments
             for (idx, argument) in enumerate(arguments):
-                if argument.isComplete():
-                    continue
-
-                argument.complete(scope)
+                if not argument.isComplete():
+                    argument.complete(scope)
                 assert argument.type.isComplete()
 
                 if (argument.type.isDictionary() or
                     (argument.type.isUnion() and
                      argument.type.unroll().hasDictionaryType)):
                     # Dictionaries and unions containing dictionaries at the
                     # end of the list or followed by optional arguments must be
                     # optional.
                     if (not argument.optional and
-                        (idx == len(arguments) - 1 or arguments[idx+1].optional)):
+                        all(arg.optional for arg in arguments[idx+1:])):
                         raise WebIDLError("Dictionary argument or union "
                                           "argument containing a dictionary "
                                           "not followed by a required argument "
                                           "must be optional",
                                           [argument.location])
 
                     # An argument cannot be a Nullable Dictionary
                     if argument.type.nullable():
@@ -3174,23 +3171,16 @@ class IDLMethod(IDLInterfaceMember, IDLS
                                           "dictionary or nullable union "
                                           "containing a dictionary",
                                           [argument.location])
 
                 # Only the last argument can be variadic
                 if variadicArgument:
                     raise WebIDLError("Variadic argument is not last argument",
                                       [variadicArgument.location])
-                # Once we see an optional argument, there can't be any non-optional
-                # arguments.
-                if inOptionalArguments and not argument.optional:
-                    raise WebIDLError("Non-optional argument after optional "
-                                      "arguments",
-                                      [argument.location])
-                inOptionalArguments = argument.optional
                 if argument.variadic:
                     variadicArgument = argument
 
             returnType = overload.returnType
             if returnType.isComplete():
                 continue
 
             type = returnType.complete(scope)
@@ -3225,17 +3215,17 @@ class IDLMethod(IDLInterfaceMember, IDLS
                             (self.identifier.name, argCount, idx,
                              distinguishingIndex),
                             [self.location, overload.location])
 
     def overloadsForArgCount(self, argc):
         return [overload for overload in self._overloads if
                 len(overload.arguments) == argc or
                 (len(overload.arguments) > argc and
-                 overload.arguments[argc].optional) or
+                 all(arg.optional for arg in overload.arguments[argc:])) or
                 (len(overload.arguments) < argc and
                  len(overload.arguments) > 0 and
                  overload.arguments[-1].variadic)]
 
     def signaturesForArgCount(self, argc):
         return [(overload.returnType, overload.arguments) for overload
                 in self.overloadsForArgCount(argc)]
 
@@ -4055,31 +4045,16 @@ class Parser(Tokenizer):
         if stringifier:
             if len(arguments) != 0:
                 raise WebIDLError("stringifier has wrong number of arguments",
                                   [self.getLocation(p, 2)])
             if not returnType.isDOMString():
                 raise WebIDLError("stringifier must have DOMString return type",
                                   [self.getLocation(p, 2)])
 
-        inOptionalArguments = False
-        variadicArgument = False
-        for argument in arguments:
-            # Only the last argument can be variadic
-            if variadicArgument:
-                raise WebIDLError("Only the last argument can be variadic",
-                                  [variadicArgument.location])
-            # Once we see an optional argument, there can't be any non-optional
-            # arguments.
-            if inOptionalArguments and not argument.optional:
-                raise WebIDLError("Cannot have a non-optional argument following an optional argument",
-                                  [argument.location])
-            inOptionalArguments = argument.optional
-            variadicArgument = argument if argument.variadic else None
-
         # identifier might be None.  This is only permitted for special methods.
         if not identifier:
             if not getter and not setter and not creator and \
                not deleter and not legacycaller and not stringifier:
                 raise WebIDLError("Identifier required for non-special methods",
                                   [self.getLocation(p, 2)])
 
             location = BuiltinLocation("<auto-generated-identifier>")
--- a/dom/bindings/parser/tests/test_dictionary.py
+++ b/dom/bindings/parser/tests/test_dictionary.py
@@ -173,16 +173,33 @@ def WebIDLTest(parser, harness):
 
     parser = parser.reset()
     threw = False
     try:
         parser.parse("""
             dictionary A {
             };
             interface X {
+              void doFoo(A arg1, optional long arg2, long arg3);
+            };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(not threw,
+               "Dictionary arg followed by non-optional arg doesn't have to be optional")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+            dictionary A {
+            };
+            interface X {
               void doFoo((A or DOMString) arg1, optional long arg2);
             };
         """)
         results = parser.finish()
     except:
         threw = True
 
     harness.ok(threw,
--- a/dom/bindings/parser/tests/test_optional_constraints.py
+++ b/dom/bindings/parser/tests/test_optional_constraints.py
@@ -6,19 +6,19 @@ def WebIDLTest(parser, harness):
               void foo(optional byte arg1, byte arg2);
             };
         """)
 
         results = parser.finish()
     except:
         threw = True
 
-    harness.ok(threw,
-               "Should have thrown on non-optional argument following optional "
-               "argument.")
+    harness.ok(not threw,
+               "Should not have thrown on non-optional argument following "
+               "optional argument.")
 
     parser = parser.reset()
     parser.parse("""
         interface OptionalConstraints2 {
           void foo(optional byte arg1 = 1, optional byte arg2 = 2,
                    optional byte arg3, optional byte arg4 = 4,
                    optional byte arg5, optional byte arg6 = 9);
         };
--- a/dom/bindings/parser/tests/test_overload.py
+++ b/dom/bindings/parser/tests/test_overload.py
@@ -6,29 +6,31 @@ def WebIDLTest(parser, harness):
           void basic();
           void basic(long arg1);
           boolean abitharder(TestOverloads foo);
           boolean abitharder(boolean foo);
           void abitharder(ArrayBuffer? foo);
           void withVariadics(long... numbers);
           void withVariadics(TestOverloads iface);
           void withVariadics(long num, TestOverloads iface);
+          void optionalTest();
+          void optionalTest(optional long num1, long num2);
         };
     """)
 
     results = parser.finish()
 
     harness.ok(True, "TestOverloads interface parsed without error.")
     harness.check(len(results), 1, "Should be one production.")
     iface = results[0]
     harness.ok(isinstance(iface, WebIDL.IDLInterface),
                "Should be an IDLInterface")
     harness.check(iface.identifier.QName(), "::TestOverloads", "Interface has the right QName")
     harness.check(iface.identifier.name, "TestOverloads", "Interface has the right name")
-    harness.check(len(iface.members), 3, "Expect %s members" % 3)
+    harness.check(len(iface.members), 4, "Expect %s members" % 4)
 
     member = iface.members[0]
     harness.check(member.identifier.QName(), "::TestOverloads::basic", "Method has the right QName")
     harness.check(member.identifier.name, "basic", "Method has the right name")
     harness.check(member.hasOverloads(), True, "Method has overloads")
 
     signatures = member.signatures()
     harness.check(len(signatures), 2, "Method should have 2 signatures")
@@ -43,8 +45,16 @@ def WebIDLTest(parser, harness):
     harness.check(len(argumentSet), 1, "Expect an argument set with one argument")
 
     argument = argumentSet[0]
     harness.ok(isinstance(argument, WebIDL.IDLArgument),
                "Should be an IDLArgument")
     harness.check(argument.identifier.QName(), "::TestOverloads::basic::arg1", "Argument has the right QName")
     harness.check(argument.identifier.name, "arg1", "Argument has the right name")
     harness.check(str(argument.type), "Long", "Argument has the right type")
+
+    member = iface.members[3]
+    harness.check(len(member.overloadsForArgCount(0)), 1,
+                  "Only one overload for no args")
+    harness.check(len(member.overloadsForArgCount(1)), 0,
+                  "No overloads for one arg")
+    harness.check(len(member.overloadsForArgCount(2)), 1,
+                  "Only one overload for two args")
--- a/dom/bindings/parser/tests/test_variadic_constraints.py
+++ b/dom/bindings/parser/tests/test_variadic_constraints.py
@@ -1,52 +1,63 @@
 def WebIDLTest(parser, harness):
     threw = False
     try:
-        results = parser.parse("""
+        parser.parse("""
             interface VariadicConstraints1 {
               void foo(byte... arg1, byte arg2);
             };
         """)
-
-    except:
-        threw = True
-
-    harness.ok(threw, "Should have thrown.")
-
-    threw = False
-    try:
-        results = parser.parse("""
-            interface VariadicConstraints2 {
-              void foo(byte... arg1, optional byte arg2);
-            };
-        """)
+        results = parser.finish()
 
     except:
         threw = True
 
-    harness.ok(threw, "Should have thrown.")
+    harness.ok(threw,
+               "Should have thrown on variadic argument followed by required "
+               "argument.")
 
+    parser = parser.reset()
     threw = False
     try:
-        results = parser.parse("""
+        parser.parse("""
+            interface VariadicConstraints2 {
+              void foo(byte... arg1, optional byte arg2);
+            };
+        """)
+        results = parser.finish();
+    except:
+        threw = True
+
+    harness.ok(threw,
+               "Should have thrown on variadic argument followed by optional "
+               "argument.")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
             interface VariadicConstraints3 {
               void foo(optional byte... arg1);
             };
         """)
+        results = parser.finish()
 
     except:
         threw = True
 
-    harness.ok(threw, "Should have thrown.")
+    harness.ok(threw,
+               "Should have thrown on variadic argument explicitly flagged as "
+               "optional.")
 
+    parser = parser.reset()
     threw = False
     try:
-        results = parser.parse("""
+        parser.parse("""
             interface VariadicConstraints4 {
               void foo(byte... arg1 = 0);
             };
         """)
-
+        results = parser.finish()
     except:
         threw = True
 
     harness.ok(threw, "Should have thrown on variadic argument with default value.")