dict: address review comments

mesonbuild/interpreterbase.py

@@ -300,8 +300,7 @@ class InterpreterBase:
 
     def evaluate_dictstatement(self, cur):
         (arguments, kwargs) = self.reduce_arguments(cur.args)
-        if len(arguments) > 0:
-            raise InvalidCode('Only key:value pairs are valid in dict construction.')
+        assert (not arguments, 'parser bug, arguments should be empty')
         return kwargs
 
     def evaluate_notstatement(self, cur):

mesonbuild/mparser.py

@@ -642,21 +642,22 @@ class Parser:
 
         while not isinstance(s, EmptyNode):
             potential = self.current
-            if self.accept('comma'):
-                a.commas.append(potential)
-                a.append(s)
-            elif self.accept('colon'):
+            if self.accept('colon'):
                 if not isinstance(s, StringNode):
                     raise ParseException('Key must be a string.',
                                          self.getline(), s.lineno, s.colno)
+                if s.value in a.kwargs:
+                    # + 1 to colno to point to the actual string, not the opening quote
+                    raise ParseException('Duplicate dictionary key: {}'.format(s.value),
+                                         self.getline(), s.lineno, s.colno + 1)
                 a.set_kwarg(s.value, self.statement())
                 potential = self.current
                 if not self.accept('comma'):
                     return a
                 a.commas.append(potential)
             else:
-                a.append(s)
-                return a
+                raise ParseException('Only key:value pairs are valid in dict construction.',
+                    self.getline(), s.lineno, s.colno)
             s = self.statement()
         return a
 
@@ -671,7 +672,7 @@ class Parser:
                 a.append(s)
             elif self.accept('colon'):
                 if not isinstance(s, IdNode):
-                    raise ParseException('Keyword argument must be a plain identifier.',
+                    raise ParseException('Dictionary key must be a plain identifier.',
                                          self.getline(), s.lineno, s.colno)
                 a.set_kwarg(s.value, self.statement())
                 potential = self.current
@@ -708,7 +709,7 @@ class Parser:
         varname = t
         varnames = [t]
 
-        if (self.accept('comma')):
+        if self.accept('comma'):
             t = self.current
             self.expect('id')
             varnames.append(t)

run_unittests.py

@@ -582,6 +582,7 @@ class BasePlatformTests(unittest.TestCase):
         if inprocess:
             try:
                 (returncode, out, err) = run_configure(self.meson_mainfile, self.meson_args + args + extra_args)
+                print (out)
                 if 'MESON_SKIP_TEST' in out:
                     raise unittest.SkipTest('Project requested skipping.')
                 if returncode != 0:
@@ -2328,6 +2329,16 @@ class FailureTests(BasePlatformTests):
         self.assertEqual(cm.exception.returncode, 2)
         self.wipe()
 
+    def test_dict_requires_key_value_pairs(self):
+        self.assertMesonRaises("dict = {3, 'foo': 'bar'}",
+            'Only key:value pairs are valid in dict construction.')
+        self.assertMesonRaises("{'foo': 'bar', 3}",
+            'Only key:value pairs are valid in dict construction.')
+
+    def test_dict_forbids_duplicate_keys(self):
+        self.assertMesonRaises("dict = {'a': 41, 'a': 42}",
+            'Duplicate dictionary key: a.*')
+
 
 class WindowsTests(BasePlatformTests):
     '''

test cases/common/199 dict/meson.build

@@ -10,3 +10,9 @@ foreach key, value : dict
   test('dict test @0@'.format(key), exe,
     args : [dict[key], value])
 endforeach
+
+empty_dict = {}
+
+foreach key, value : empty_dict
+  assert(false, 'This dict should be empty')
+endforeach