Recursively validate Nav config option types (#2680)

mkdocs/config/config_options.py

@@ -529,23 +529,42 @@ class Nav(OptionallyRequired):
     Validate the Nav config.
     """
 
-    def run_validation(self, value):
-
-        if not isinstance(value, list):
-            raise ValidationError(f"Expected a list, got {type(value)}")
-
-        if len(value) == 0:
-            return
-
-        config_types = {type(item) for item in value}
-        if config_types.issubset({str, dict}):
-            return value
+    def run_validation(self, value, *, top=True):
+        if isinstance(value, list):
+            for subitem in value:
+                self._validate_nav_item(subitem)
+            if top and not value:
+                value = None
+        elif isinstance(value, dict) and value and not top:
+            # TODO: this should be an error.
+            self.warnings.append(f"Expected nav to be a list, got {self._repr_item(value)}")
+            for subitem in value.values():
+                self.run_validation(subitem, top=False)
+        elif isinstance(value, str) and not top:
+            pass
+        else:
+            raise ValidationError(f"Expected nav to be a list, got {self._repr_item(value)}")
+        return value
 
-        types = ', '.join(set(
-            item_type.__name__ for item_type in config_types
-        ))
-        raise ValidationError(
-            f"Invalid navigation config types. Expected str and dict, got: {types}")
+    def _validate_nav_item(self, value):
+        if isinstance(value, str):
+            pass
+        elif isinstance(value, dict):
+            if len(value) != 1:
+                raise ValidationError(f"Expected nav item to be a dict of size 1, got {self._repr_item(value)}")
+            for subnav in value.values():
+                self.run_validation(subnav, top=False)
+        else:
+            raise ValidationError(f"Expected nav item to be a string or dict, got {self._repr_item(value)}")
+
+    @classmethod
+    def _repr_item(cls, value):
+        if isinstance(value, dict) and value:
+            return f"dict with keys {tuple(value.keys())}"
+        elif isinstance(value, (str, type(None))):
+            return repr(value)
+        else:
+            return f"a {type(value).__name__}: {value!r}"
 
     def post_validation(self, config, key_name):
         # TODO: remove this when `pages` config setting is fully deprecated.

mkdocs/tests/config/config_options_tests.py

@@ -1,12 +1,14 @@
 import os
 import sys
+import textwrap
 import unittest
 from unittest.mock import patch
 
 import mkdocs
 from mkdocs.config import config_options
 from mkdocs.config.base import Config
 from mkdocs.tests.base import tempdir
+from mkdocs.utils import yaml_load
 
 
 class OptionallyRequiredTest(unittest.TestCase):
@@ -733,13 +735,12 @@ def test_post_validation_locale(self):
 class NavTest(unittest.TestCase):
 
     def test_old_format(self):
-
         option = config_options.Nav()
-        with self.assertRaises(config_options.ValidationError):
+        with self.assertRaises(config_options.ValidationError) as cm:
             option.validate([['index.md']])
+        self.assertEqual(str(cm.exception), "Expected nav item to be a string or dict, got a list: ['index.md']")
 
     def test_provided_dict(self):
-
         option = config_options.Nav()
         value = option.validate([
             'index.md',
@@ -748,26 +749,92 @@ def test_provided_dict(self):
         self.assertEqual(['index.md', {'Page': 'page.md'}], value)
 
         option.post_validation({'extra_stuff': []}, 'extra_stuff')
+        self.assertEqual(option.warnings, [])
 
     def test_provided_empty(self):
-
         option = config_options.Nav()
         value = option.validate([])
         self.assertEqual(None, value)
 
         option.post_validation({'extra_stuff': []}, 'extra_stuff')
+        self.assertEqual(option.warnings, [])
 
-    def test_invalid_type(self):
+    def test_normal_nav(self):
+        nav = yaml_load(textwrap.dedent('''\
+            - Home: index.md
+            - getting-started.md
+            - User Guide:
+                - Overview: user-guide/index.md
+                - Installation: user-guide/installation.md
+        ''').encode())
 
         option = config_options.Nav()
-        with self.assertRaises(config_options.ValidationError):
+        self.assertEqual(option.validate(nav), nav)
+        self.assertEqual(option.warnings, [])
+
+    def test_invalid_type_dict(self):
+        option = config_options.Nav()
+        with self.assertRaises(config_options.ValidationError) as cm:
             option.validate({})
+        self.assertEqual(str(cm.exception), "Expected nav to be a list, got a dict: {}")
+
+    def test_invalid_type_int(self):
+        option = config_options.Nav()
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate(5)
+        self.assertEqual(str(cm.exception), "Expected nav to be a list, got a int: 5")
 
-    def test_invalid_config(self):
+    def test_invalid_item_int(self):
+        option = config_options.Nav()
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate([1])
+        self.assertEqual(str(cm.exception), "Expected nav item to be a string or dict, got a int: 1")
 
+    def test_invalid_item_none(self):
         option = config_options.Nav()
-        with self.assertRaises(config_options.ValidationError):
-            option.validate([[], 1])
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate([None])
+        self.assertEqual(str(cm.exception), "Expected nav item to be a string or dict, got None")
+
+    def test_invalid_children_config_int(self):
+        option = config_options.Nav()
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate([{"foo.md": [{"bar.md": 1}]}])
+        self.assertEqual(str(cm.exception), "Expected nav to be a list, got a int: 1")
+
+    def test_invalid_children_config_none(self):
+        option = config_options.Nav()
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate([{"foo.md": None}])
+        self.assertEqual(str(cm.exception), "Expected nav to be a list, got None")
+
+    def test_invalid_children_empty_dict(self):
+        option = config_options.Nav()
+        nav = ['foo', {}]
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate(nav)
+        self.assertEqual(str(cm.exception), "Expected nav item to be a dict of size 1, got a dict: {}")
+
+    def test_invalid_nested_list(self):
+        option = config_options.Nav()
+        nav = [{'aaa': [[{"bbb": "user-guide/index.md"}]]}]
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate(nav)
+        msg = "Expected nav item to be a string or dict, got a list: [{'bbb': 'user-guide/index.md'}]"
+        self.assertEqual(str(cm.exception), msg)
+
+    def test_invalid_children_oversized_dict(self):
+        option = config_options.Nav()
+        nav = [{"aaa": [{"bbb": "user-guide/index.md", "ccc": "user-guide/installation.md"}]}]
+        with self.assertRaises(config_options.ValidationError) as cm:
+            option.validate(nav)
+        msg = "Expected nav item to be a dict of size 1, got dict with keys ('bbb', 'ccc')"
+        self.assertEqual(str(cm.exception), msg)
+
+    def test_warns_for_dict(self):
+        option = config_options.Nav()
+        option.validate([{"a": {"b": "c.md", "d": "e.md"}}])
+        self.assertEqual(option.warnings, ["Expected nav to be a list, got dict with keys ('b', 'd')"])
 
 
 class PrivateTest(unittest.TestCase):