From 7a72282f8a2c8e7dc0f5869f9f2b3310d9b1036a Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Tue, 4 Oct 2022 00:37:00 +0200 Subject: [PATCH 1/7] Refactor Theme config option to use mainly run_validation --- mkdocs/config/config_options.py | 56 +++++++++---------- .../config/config_options_legacy_tests.py | 16 ++---- mkdocs/tests/config/config_options_tests.py | 16 ++---- 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 20f7e8c24c..f039131c50 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -712,53 +712,49 @@ def __init__(self, default=None): super().__init__() self.default = default - def run_validation(self, value): - if value is None and self.default is not None: - value = {'name': self.default} + def pre_validation(self, config, key_name): + self.config_file_path = config.config_file_path - if isinstance(value, str): - value = {'name': value} + def run_validation(self, value) -> theme.Theme: + if value is None and self.default is not None: + theme_config = {'name': self.default} + elif isinstance(value, str): + theme_config = {'name': value} + elif isinstance(value, dict): + if 'name' not in value: + raise ValidationError("No theme name set.") + theme_config = value + else: + raise ValidationError( + f'Invalid type {type(value)}. Expected a string or key/value pairs.' + ) themes = utils.get_theme_names() - - if isinstance(value, dict): - if 'name' in value: - if value['name'] is None or value['name'] in themes: - return value - - raise ValidationError( - f"Unrecognised theme name: '{value['name']}'. " - f"The available installed themes are: {', '.join(themes)}" - ) - - raise ValidationError("No theme name set.") - - raise ValidationError(f'Invalid type {type(value)}. Expected a string or key/value pairs.') - - def post_validation(self, config, key_name): - theme_config = config[key_name] - - if not theme_config['name'] and 'custom_dir' not in theme_config: + if theme_config['name'] is not None and theme_config['name'] not in themes: raise ValidationError( - f"At least one of '{key_name}.name' or '{key_name}.custom_dir' must be defined." + f"Unrecognised theme name: '{theme_config['name']}'. " + f"The available installed themes are: {', '.join(themes)}" ) + if not theme_config['name'] and 'custom_dir' not in theme_config: + raise ValidationError("At least one of 'name' or 'custom_dir' must be defined.") + # Ensure custom_dir is an absolute path if 'custom_dir' in theme_config and not os.path.isabs(theme_config['custom_dir']): - config_dir = os.path.dirname(config.config_file_path) + config_dir = os.path.dirname(self.config_file_path) theme_config['custom_dir'] = os.path.join(config_dir, theme_config['custom_dir']) if 'custom_dir' in theme_config and not os.path.isdir(theme_config['custom_dir']): raise ValidationError( - "The path set in {name}.custom_dir ('{path}') does not exist.".format( - path=theme_config['custom_dir'], name=key_name + "The path set in custom_dir ('{path}') does not exist.".format( + path=theme_config['custom_dir'] ) ) if 'locale' in theme_config and not isinstance(theme_config['locale'], str): - raise ValidationError(f"'{key_name}.locale' must be a string.") + raise ValidationError("'locale' must be a string.") - config[key_name] = theme.Theme(**theme_config) + return theme.Theme(**theme_config) class Nav(OptionallyRequired): diff --git a/mkdocs/tests/config/config_options_legacy_tests.py b/mkdocs/tests/config/config_options_legacy_tests.py index 8a3d545b76..4c2a0560a4 100644 --- a/mkdocs/tests/config/config_options_legacy_tests.py +++ b/mkdocs/tests/config/config_options_legacy_tests.py @@ -976,9 +976,7 @@ def test_theme_name_is_none(self): class Schema: option = c.Theme() - with self.expect_error( - option="At least one of 'option.name' or 'option.custom_dir' must be defined." - ): + with self.expect_error(option="At least one of 'name' or 'custom_dir' must be defined."): self.get_config(Schema, {'option': config}) def test_theme_config_missing_name(self): @@ -1028,9 +1026,7 @@ def test_post_validation_none_theme_name_and_missing_custom_dir(self): class Schema: theme = c.Theme() - with self.expect_error( - theme="At least one of 'theme.name' or 'theme.custom_dir' must be defined." - ): + with self.expect_error(theme="At least one of 'name' or 'custom_dir' must be defined."): self.get_config(Schema, config) @tempdir() @@ -1046,9 +1042,7 @@ def test_post_validation_inexisting_custom_dir(self, abs_base_path): class Schema: theme = c.Theme() - with self.expect_error( - theme=f"The path set in theme.custom_dir ('{path}') does not exist." - ): + with self.expect_error(theme=f"The path set in custom_dir ('{path}') does not exist."): self.get_config(Schema, config) def test_post_validation_locale_none(self): @@ -1062,7 +1056,7 @@ def test_post_validation_locale_none(self): class Schema: theme = c.Theme() - with self.expect_error(theme="'theme.locale' must be a string."): + with self.expect_error(theme="'locale' must be a string."): self.get_config(Schema, config) def test_post_validation_locale_invalid_type(self): @@ -1076,7 +1070,7 @@ def test_post_validation_locale_invalid_type(self): class Schema: theme = c.Theme() - with self.expect_error(theme="'theme.locale' must be a string."): + with self.expect_error(theme="'locale' must be a string."): self.get_config(Schema, config) def test_post_validation_locale(self): diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 71b3b14ecc..4e7de9501d 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1011,9 +1011,7 @@ def test_theme_name_is_none(self) -> None: class Schema(Config): option = c.Theme() - with self.expect_error( - option="At least one of 'option.name' or 'option.custom_dir' must be defined." - ): + with self.expect_error(option="At least one of 'name' or 'custom_dir' must be defined."): self.get_config(Schema, {'option': config}) def test_theme_config_missing_name(self) -> None: @@ -1063,9 +1061,7 @@ def test_post_validation_none_theme_name_and_missing_custom_dir(self) -> None: class Schema(Config): theme = c.Theme() - with self.expect_error( - theme="At least one of 'theme.name' or 'theme.custom_dir' must be defined." - ): + with self.expect_error(theme="At least one of 'name' or 'custom_dir' must be defined."): self.get_config(Schema, config) @tempdir() @@ -1081,9 +1077,7 @@ def test_post_validation_inexisting_custom_dir(self, abs_base_path) -> None: class Schema(Config): theme = c.Theme() - with self.expect_error( - theme=f"The path set in theme.custom_dir ('{path}') does not exist." - ): + with self.expect_error(theme=f"The path set in custom_dir ('{path}') does not exist."): self.get_config(Schema, config) def test_post_validation_locale_none(self) -> None: @@ -1097,7 +1091,7 @@ def test_post_validation_locale_none(self) -> None: class Schema(Config): theme = c.Theme() - with self.expect_error(theme="'theme.locale' must be a string."): + with self.expect_error(theme="'locale' must be a string."): self.get_config(Schema, config) def test_post_validation_locale_invalid_type(self) -> None: @@ -1111,7 +1105,7 @@ def test_post_validation_locale_invalid_type(self) -> None: class Schema(Config): theme = c.Theme() - with self.expect_error(theme="'theme.locale' must be a string."): + with self.expect_error(theme="'locale' must be a string."): self.get_config(Schema, config) def test_post_validation_locale(self) -> None: From 76709ab540b35e5fdd162d6f189c7e609fb6ad79 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Thu, 6 Oct 2022 23:25:53 +0200 Subject: [PATCH 2/7] Expand plugin testing --- mkdocs/plugins.py | 11 +- mkdocs/tests/config/config_options_tests.py | 202 +++++++++++++++++++- mkdocs/tests/plugin_tests.py | 183 ++---------------- 3 files changed, 227 insertions(+), 169 deletions(-) diff --git a/mkdocs/plugins.py b/mkdocs/plugins.py index 49db04441b..1167c9f51f 100644 --- a/mkdocs/plugins.py +++ b/mkdocs/plugins.py @@ -6,7 +6,6 @@ import logging import sys -from collections import OrderedDict from typing import ( TYPE_CHECKING, Any, @@ -14,6 +13,7 @@ Dict, Generic, List, + MutableMapping, Optional, Tuple, Type, @@ -461,7 +461,7 @@ def decorator(event_method): return decorator -class PluginCollection(OrderedDict): +class PluginCollection(dict, MutableMapping[str, BasePlugin]): """ A collection of plugins. @@ -480,8 +480,11 @@ def _register_event(self, event_name: str, method: Callable) -> None: self.events[event_name], method, key=lambda m: -getattr(m, 'mkdocs_priority', 0) ) - def __setitem__(self, key: str, value: BasePlugin, **kwargs) -> None: - super().__setitem__(key, value, **kwargs) + def __getitem__(self, key: str) -> BasePlugin: + return super().__getitem__(key) + + def __setitem__(self, key: str, value: BasePlugin) -> None: + super().__setitem__(key, value) # Register all of the event methods defined for this Plugin. for event_name in (x for x in dir(value) if x.startswith('on_')): method = getattr(value, event_name, None) diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 4e7de9501d..a0a72a43b5 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -20,6 +20,7 @@ def assert_type(val, typ): import mkdocs from mkdocs.config import config_options as c from mkdocs.config.base import Config +from mkdocs.plugins import BasePlugin, PluginCollection from mkdocs.tests.base import tempdir from mkdocs.theme import Theme from mkdocs.utils import write_file, yaml_load @@ -1619,6 +1620,205 @@ class Schema(Config): self.assertIsNone(conf.mdx_configs.get('toc')) +class _FakePluginConfig(Config): + foo = c.Type(str, default='default foo') + bar = c.Type(int, default=0) + dir = c.Optional(c.Dir(exists=False)) + + +class FakePlugin(BasePlugin[_FakePluginConfig]): + pass + + +class FakeEntryPoint: + @property + def name(self): + return 'sample' + + def load(self): + return FakePlugin + + +@patch('mkdocs.plugins.entry_points', return_value=[FakeEntryPoint()]) +class PluginsTest(TestCase): + def test_plugin_config_without_options(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': ['sample'], + } + conf = self.get_config(Schema, cfg) + + assert_type(conf.plugins, PluginCollection) + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertIn('sample', conf.plugins) + + plugin = conf.plugins['sample'] + assert_type(plugin, BasePlugin) + self.assertIsInstance(plugin, FakePlugin) + self.assertIsInstance(plugin.config, _FakePluginConfig) + expected = { + 'foo': 'default foo', + 'bar': 0, + 'dir': None, + } + self.assertEqual(plugin.config, expected) + + def test_plugin_config_with_options(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': [ + { + 'sample': { + 'foo': 'foo value', + 'bar': 42, + }, + } + ], + } + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertIn('sample', conf.plugins) + self.assertIsInstance(conf.plugins['sample'], BasePlugin) + expected = { + 'foo': 'foo value', + 'bar': 42, + 'dir': None, + } + self.assertEqual(conf.plugins['sample'].config, expected) + + def test_plugin_config_as_dict(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': { + 'sample': { + 'foo': 'foo value', + 'bar': 42, + }, + }, + } + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertIn('sample', conf.plugins) + self.assertIsInstance(conf.plugins['sample'], BasePlugin) + expected = { + 'foo': 'foo value', + 'bar': 42, + 'dir': None, + } + self.assertEqual(conf.plugins['sample'].config, expected) + + def test_plugin_config_empty_list_with_empty_default(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins(default=[]) + + cfg: Dict[str, Any] = {'plugins': []} + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertEqual(len(conf.plugins), 0) + + def test_plugin_config_empty_list_with_default(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins(default=['sample']) + + # Default is ignored + cfg: Dict[str, Any] = {'plugins': []} + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertEqual(len(conf.plugins), 0) + + def test_plugin_config_none_with_empty_default(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins(default=[]) + + cfg = {'plugins': None} + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertEqual(len(conf.plugins), 0) + + def test_plugin_config_none_with_default(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins(default=['sample']) + + # Default is used. + cfg = {'plugins': None} + conf = self.get_config(Schema, cfg) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertIn('sample', conf.plugins) + self.assertIsInstance(conf.plugins['sample'], BasePlugin) + expected = { + 'foo': 'default foo', + 'bar': 0, + 'dir': None, + } + self.assertEqual(conf.plugins['sample'].config, expected) + + def test_plugin_config_uninstalled(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = {'plugins': ['uninstalled']} + with self.expect_error(plugins='The "uninstalled" plugin is not installed'): + self.get_config(Schema, cfg) + + def test_plugin_config_not_list(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = {'plugins': 'sample'} + with self.expect_error(plugins="Invalid Plugins configuration. Expected a list or dict."): + self.get_config(Schema, cfg) + + def test_plugin_config_multivalue_dict(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': [ + { + 'sample': { + 'foo': 'foo value', + 'bar': 42, + }, + 'extra_key': 'baz', + } + ], + } + with self.expect_error(plugins="Invalid Plugins configuration"): + self.get_config(Schema, cfg) + + def test_plugin_config_not_string_or_dict(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': [('not a string or dict',)], + } + with self.expect_error(plugins="'('not a string or dict',)' is not a valid plugin name."): + self.get_config(Schema, cfg) + + def test_plugin_config_options_not_dict(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': [{'sample': 'not a dict'}], + } + with self.expect_error(plugins="Invalid config options for the 'sample' plugin."): + self.get_config(Schema, cfg) + + class TestHooks(TestCase): class Schema(Config): plugins = c.Plugins(default=[]) @@ -1646,7 +1846,7 @@ def test_hooks(self, src_dir) -> None: {**conf.plugins.events, 'page_markdown': [hook.on_page_markdown]}, conf.plugins.events, ) - self.assertEqual(hook.on_page_markdown('foo foo'), 'zoo zoo') + self.assertEqual(hook.on_page_markdown('foo foo'), 'zoo zoo') # type: ignore[call-arg] self.assertFalse(hasattr(hook, 'on_nav')) diff --git a/mkdocs/tests/plugin_tests.py b/mkdocs/tests/plugin_tests.py index 80c8231b18..4c33f06a62 100644 --- a/mkdocs/tests/plugin_tests.py +++ b/mkdocs/tests/plugin_tests.py @@ -3,9 +3,17 @@ import os import unittest -from unittest import mock +from typing import TYPE_CHECKING, Optional -from mkdocs import config, plugins +if TYPE_CHECKING: + from typing_extensions import assert_type +else: + + def assert_type(val, typ): + return None + + +from mkdocs import plugins from mkdocs.commands import build from mkdocs.config import base from mkdocs.config import config_options as c @@ -39,7 +47,7 @@ def on_pre_build(self, **kwargs): class TestPluginClass(unittest.TestCase): - def test_valid_plugin_options(self): + def test_valid_plugin_options(self) -> None: test_dir = 'test' options = { @@ -51,20 +59,24 @@ def test_valid_plugin_options(self): cfg_fname = os.path.abspath(cfg_fname) cfg_dirname = os.path.dirname(cfg_fname) - expected = os.path.join(cfg_dirname, test_dir) - expected = { 'foo': 'some value', 'bar': 0, - 'dir': expected, + 'dir': os.path.join(cfg_dirname, test_dir), } plugin = DummyPlugin() errors, warnings = plugin.load_config(options, config_file_path=cfg_fname) - self.assertEqual(plugin.config, expected) self.assertEqual(errors, []) self.assertEqual(warnings, []) + assert_type(plugin.config, _DummyPluginConfig) + self.assertEqual(plugin.config, expected) + + assert_type(plugin.config.bar, int) + self.assertEqual(plugin.config.bar, 0) + assert_type(plugin.config.dir, Optional[str]) + def test_invalid_plugin_options(self): plugin = DummyPlugin() errors, warnings = plugin.load_config({'foo': 42}) @@ -285,160 +297,3 @@ def on_build_error(self, error, **kwargs): self.assertEqual(str(build_errors[2]), 'page content error') self.assertIs(build_errors[3].__class__, ValueError) self.assertEqual(str(build_errors[3]), 'post page error') - - -MockEntryPoint = mock.Mock() -MockEntryPoint.configure_mock(**{'name': 'sample', 'load.return_value': DummyPlugin}) - - -@mock.patch('mkdocs.plugins.entry_points', return_value=[MockEntryPoint]) -class TestPluginConfig(unittest.TestCase): - def test_plugin_config_without_options(self, mock_class): - cfg = {'plugins': ['sample']} - option = c.Plugins() - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertIn('sample', cfg['plugins']) - self.assertIsInstance(cfg['plugins']['sample'], plugins.BasePlugin) - expected = { - 'foo': 'default foo', - 'bar': 0, - 'dir': None, - } - self.assertEqual(cfg['plugins']['sample'].config, expected) - - def test_plugin_config_with_options(self, mock_class): - cfg = { - 'plugins': [ - { - 'sample': { - 'foo': 'foo value', - 'bar': 42, - }, - } - ], - } - option = c.Plugins() - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertIn('sample', cfg['plugins']) - self.assertIsInstance(cfg['plugins']['sample'], plugins.BasePlugin) - expected = { - 'foo': 'foo value', - 'bar': 42, - 'dir': None, - } - self.assertEqual(cfg['plugins']['sample'].config, expected) - - def test_plugin_config_as_dict(self, mock_class): - cfg = { - 'plugins': { - 'sample': { - 'foo': 'foo value', - 'bar': 42, - }, - }, - } - option = c.Plugins() - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertIn('sample', cfg['plugins']) - self.assertIsInstance(cfg['plugins']['sample'], plugins.BasePlugin) - expected = { - 'foo': 'foo value', - 'bar': 42, - 'dir': None, - } - self.assertEqual(cfg['plugins']['sample'].config, expected) - - def test_plugin_config_empty_list_with_empty_default(self, mock_class): - cfg = {'plugins': []} - option = c.Plugins(default=[]) - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertEqual(len(cfg['plugins']), 0) - - def test_plugin_config_empty_list_with_default(self, mock_class): - # Default is ignored - cfg = {'plugins': []} - option = c.Plugins(default=['sample']) - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertEqual(len(cfg['plugins']), 0) - - def test_plugin_config_none_with_empty_default(self, mock_class): - cfg = {'plugins': None} - option = c.Plugins(default=[]) - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertEqual(len(cfg['plugins']), 0) - - def test_plugin_config_none_with_default(self, mock_class): - # Default is used. - cfg = {'plugins': None} - option = c.Plugins(default=['sample']) - cfg['plugins'] = option.validate(cfg['plugins']) - - self.assertIsInstance(cfg['plugins'], plugins.PluginCollection) - self.assertIn('sample', cfg['plugins']) - self.assertIsInstance(cfg['plugins']['sample'], plugins.BasePlugin) - expected = { - 'foo': 'default foo', - 'bar': 0, - 'dir': None, - } - self.assertEqual(cfg['plugins']['sample'].config, expected) - - def test_plugin_config_uninstalled(self, mock_class): - cfg = {'plugins': ['uninstalled']} - option = c.Plugins() - with self.assertRaises(config.base.ValidationError): - option.validate(cfg['plugins']) - - def test_plugin_config_not_list(self, mock_class): - cfg = {'plugins': 'sample'} # should be a list - option = c.Plugins() - with self.assertRaises(config.base.ValidationError): - option.validate(cfg['plugins']) - - def test_plugin_config_multivalue_dict(self, mock_class): - cfg = { - 'plugins': [ - { - 'sample': { - 'foo': 'foo value', - 'bar': 42, - }, - 'extra_key': 'baz', - } - ], - } - option = c.Plugins() - with self.assertRaises(config.base.ValidationError): - option.validate(cfg['plugins']) - - def test_plugin_config_not_string_or_dict(self, mock_class): - cfg = { - 'plugins': [('not a string or dict',)], - } - option = c.Plugins() - with self.assertRaises(config.base.ValidationError): - option.validate(cfg['plugins']) - - def test_plugin_config_options_not_dict(self, mock_class): - cfg = { - 'plugins': [ - { - 'sample': 'not a dict', - } - ], - } - option = c.Plugins() - with self.assertRaises(config.base.ValidationError): - option.validate(cfg['plugins']) From ca8a3c8d671b7df445a6ad301d38a27f0d924ca7 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Fri, 7 Oct 2022 22:57:00 +0200 Subject: [PATCH 3/7] Tighten warnings and type annotations in config_options --- mkdocs/config/base.py | 4 +- mkdocs/config/config_options.py | 164 ++++++++++-------- .../config/config_options_legacy_tests.py | 8 +- mkdocs/tests/config/config_options_tests.py | 15 +- 4 files changed, 110 insertions(+), 81 deletions(-) diff --git a/mkdocs/config/base.py b/mkdocs/config/base.py index f1cafd4102..25631c33a2 100644 --- a/mkdocs/config/base.py +++ b/mkdocs/config/base.py @@ -51,7 +51,7 @@ def default(self): def default(self, value): self._default = value - def validate(self, value) -> T: + def validate(self, value: object) -> T: return self.run_validation(value) def reset_warnings(self) -> None: @@ -64,7 +64,7 @@ def pre_validation(self, config: Config, key_name: str) -> None: The pre-validation process method should be implemented by subclasses. """ - def run_validation(self, value): + def run_validation(self, value: object): """ Perform validation for a value. diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index f039131c50..2b975830de 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -6,10 +6,23 @@ import string import sys import traceback +import types import typing as t import warnings from collections import UserString -from typing import Collection, Dict, Generic, List, NamedTuple, Tuple, TypeVar, Union, overload +from typing import ( + Any, + Collection, + Dict, + Generic, + List, + Mapping, + NamedTuple, + Tuple, + TypeVar, + Union, + overload, +) from urllib.parse import quote as urlquote from urllib.parse import urlsplit, urlunsplit @@ -72,7 +85,7 @@ def __init__(self, *config_options, validate=None): self._make_config = functools.partial(LegacyConfig, config_options) self._do_validation = bool(validate) - def run_validation(self, value): + def run_validation(self, value: object) -> SomeConfig: config = self._make_config() try: config.load_dict(value) @@ -82,7 +95,7 @@ def run_validation(self, value): if self._do_validation: # Capture errors and warnings - self.warnings = warnings + self.warnings = [f'Sub-option {key!r}: {msg}' for key, msg in warnings] if failed: # Get the first failing one key, err = failed[0] @@ -141,20 +154,20 @@ class ListOfItems(Generic[T], BaseConfigOption[List[T]]): required: Union[bool, None] = None # Only for subclasses to set. - def __init__(self, option_type: BaseConfigOption[T], default=None): + def __init__(self, option_type: BaseConfigOption[T], default=None) -> None: super().__init__() self.default = default self.option_type = option_type self.option_type.warnings = self.warnings - def __repr__(self): + def __repr__(self) -> str: return f'{type(self).__name__}: {self.option_type}' - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): self._config = config self._key_name = key_name - def run_validation(self, value): + def run_validation(self, value: object) -> List[T]: if value is None: if self.required or self.default is None: raise ValidationError("Required configuration not provided.") @@ -164,7 +177,7 @@ def run_validation(self, value): if not value: # Optimization for empty list return value - fake_config = Config(()) + fake_config = LegacyConfig(()) try: fake_config.config_file_path = self._config.config_file_path except AttributeError: @@ -202,7 +215,7 @@ def __init__(self, *config_options: PlainConfigSchemaItem): def __init__(self, *config_options: PlainConfigSchemaItem, required: bool): ... - def __init__(self, *config_options: PlainConfigSchemaItem, required=None): + def __init__(self, *config_options: PlainConfigSchemaItem, required=None) -> None: super().__init__(SubConfig(*config_options), default=[]) self._legacy_required = required self.required = bool(required) @@ -223,12 +236,12 @@ def __init__(self, type_: t.Type[T], length: t.Optional[int] = None, **kwargs): def __init__(self, type_: Tuple[t.Type[T], ...], length: t.Optional[int] = None, **kwargs): ... - def __init__(self, type_, length=None, **kwargs): + def __init__(self, type_, length=None, **kwargs) -> None: super().__init__(**kwargs) self._type = type_ self.length = length - def run_validation(self, value): + def run_validation(self, value: object) -> T: if not isinstance(value, self._type): msg = f"Expected type: {self._type} but received: {type(value)}" elif self.length is not None and len(value) != self.length: @@ -249,7 +262,7 @@ class Choice(Generic[T], OptionallyRequired[T]): Validate the config option against a strict set of values. """ - def __init__(self, choices: Collection[T], default: t.Optional[T] = None, **kwargs): + def __init__(self, choices: Collection[T], default: t.Optional[T] = None, **kwargs) -> None: super().__init__(default=default, **kwargs) try: length = len(choices) @@ -263,10 +276,10 @@ def __init__(self, choices: Collection[T], default: t.Optional[T] = None, **kwar self.choices = choices - def run_validation(self, value): + def run_validation(self, value: object) -> T: if value not in self.choices: raise ValidationError(f"Expected one of: {self.choices} but received: {value!r}") - return value + return value # type: ignore class Deprecated(BaseConfigOption): @@ -285,7 +298,7 @@ def __init__( message: t.Optional[str] = None, removed: bool = False, option_type: t.Optional[BaseConfigOption] = None, - ): + ) -> None: super().__init__() self.default = None self.moved_to = moved_to @@ -306,7 +319,7 @@ def __init__( self.warnings = self.option.warnings - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): self.option.pre_validation(config, key_name) if config.get(key_name) is not None: @@ -316,7 +329,7 @@ def pre_validation(self, config, key_name): if self.moved_to is not None: *parent_keys, target_key = self.moved_to.split('.') - target = config + target: Any = config for key in parent_keys: if target.get(key) is None: @@ -332,7 +345,7 @@ def pre_validation(self, config, key_name): def validate(self, value): return self.option.validate(value) - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): self.option.post_validation(config, key_name) def reset_warnings(self): @@ -344,7 +357,7 @@ class _IpAddressValue(NamedTuple): host: str port: int - def __str__(self): + def __str__(self) -> str: return f'{self.host}:{self.port}' @@ -355,11 +368,10 @@ class IpAddress(OptionallyRequired[_IpAddressValue]): Validate that an IP address is in an appropriate format """ - def run_validation(self, value): - try: - host, port = value.rsplit(':', 1) - except Exception: + def run_validation(self, value: object) -> _IpAddressValue: + if not isinstance(value, str) or ':' not in value: raise ValidationError("Must be a string of format 'IP:PORT'") + host, port_str = value.rsplit(':', 1) if host != 'localhost': if host.startswith('[') and host.endswith(']'): @@ -371,13 +383,13 @@ def run_validation(self, value): raise ValidationError(e) try: - port = int(port) + port = int(port_str) except Exception: - raise ValidationError(f"'{port}' is not a valid port") + raise ValidationError(f"'{port_str}' is not a valid port") return _IpAddressValue(host, port) - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): host = config[key_name].host if key_name == 'dev_addr' and host in ['0.0.0.0', '::']: self.warnings.append( @@ -403,14 +415,15 @@ def __init__(self, default=None, *, is_dir: bool = False): def __init__(self, default=None, *, required: bool, is_dir: bool = False): ... - def __init__(self, default=None, required=None, is_dir: bool = False): + def __init__(self, default=None, required=None, is_dir: bool = False) -> None: self.is_dir = is_dir super().__init__(default, required=required) - def run_validation(self, value): + def run_validation(self, value: object) -> str: + if not isinstance(value, str): + raise ValidationError(f"Expected a string, got {type(value)}") if value == '': return value - try: parsed_url = urlsplit(value) except (AttributeError, TypeError): @@ -430,7 +443,7 @@ class Optional(Generic[T], BaseConfigOption[Union[T, None]]): E.g. `my_field = config_options.Optional(config_options.Type(str))` """ - def __init__(self, config_option: BaseConfigOption[T]): + def __init__(self, config_option: BaseConfigOption[T]) -> None: if config_option.default is not None: raise ValueError( f"This option already has a default ({config_option.default!r}) " @@ -445,16 +458,16 @@ def __getattr__(self, key): raise AttributeError return getattr(self.option, key) - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): return self.option.pre_validation(config, key_name) - def run_validation(self, value): + def run_validation(self, value: object) -> Union[T, None]: if value is None: return None return self.option.validate(value) - def post_validation(self, config, key_name): - result = self.option.post_validation(config, key_name) + def post_validation(self, config: Config, key_name: str): + result = self.option.post_validation(config, key_name) # type: ignore self.warnings = self.option.warnings return result @@ -470,7 +483,7 @@ def __init__(self, *args, **kwargs): ) super().__init__(*args, **kwargs) - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): repo_host = urlsplit(config['repo_url']).netloc.lower() edit_uri = config.get('edit_uri') @@ -502,11 +515,11 @@ def post_validation(self, config, key_name): class EditURI(Type[str]): - def __init__(self, repo_url_key: str): + def __init__(self, repo_url_key: str) -> None: super().__init__(str) self.repo_url_key = repo_url_key - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): edit_uri = config.get(key_name) repo_url = config.get(self.repo_url_key) @@ -532,7 +545,7 @@ def convert_field(self, value, conversion): return super().convert_field(value, conversion) class Template(UserString): - def __init__(self, formatter, data): + def __init__(self, formatter, data) -> None: super().__init__(data) self.formatter = formatter try: @@ -543,17 +556,17 @@ def __init__(self, formatter, data): def format(self, path, path_noext): return self.formatter.format(self.data, path=path, path_noext=path_noext) - def __init__(self, edit_uri_key=None): + def __init__(self, edit_uri_key: t.Optional[str] = None) -> None: super().__init__() self.edit_uri_key = edit_uri_key - def run_validation(self, value): + def run_validation(self, value: object): try: return self.Template(self.Formatter(), value) except Exception as e: raise ValidationError(e) - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): if self.edit_uri_key and config.get(key_name) and config.get(self.edit_uri_key): self.warnings.append( f"The option '{self.edit_uri_key}' has no effect when '{key_name}' is set." @@ -561,11 +574,11 @@ def post_validation(self, config, key_name): class RepoName(Type[str]): - def __init__(self, repo_url_key: str): + def __init__(self, repo_url_key: str) -> None: super().__init__(str) self.repo_url_key = repo_url_key - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): repo_name = config.get(key_name) repo_url = config.get(self.repo_url_key) @@ -591,17 +604,17 @@ class FilesystemObject(Type[str]): existence_test = staticmethod(os.path.exists) name = 'file or directory' - def __init__(self, exists: bool = False, **kwargs): + def __init__(self, exists: bool = False, **kwargs) -> None: super().__init__(type_=str, **kwargs) self.exists = exists - self.config_dir = None + self.config_dir: t.Optional[str] = None - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): self.config_dir = ( os.path.dirname(config.config_file_path) if config.config_file_path else None ) - def run_validation(self, value): + def run_validation(self, value: object) -> str: value = super().run_validation(value) if self.config_dir and not os.path.isabs(value): value = os.path.join(self.config_dir, value) @@ -622,7 +635,7 @@ class Dir(FilesystemObject): class DocsDir(Dir): - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): if config.config_file_path is None: return @@ -665,7 +678,7 @@ def __init__(self, default=[]): def __init__(self, default=[], *, required: bool): ... - def __init__(self, default=[], required=None): + def __init__(self, default=[], required=None) -> None: super().__init__(FilesystemObject(exists=True), default) self.required = required @@ -677,7 +690,7 @@ class SiteDir(Dir): Validates the site_dir and docs_dir directories do not contain each other. """ - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): super().post_validation(config, key_name) docs_dir = config['docs_dir'] site_dir = config['site_dir'] @@ -708,14 +721,14 @@ class Theme(BaseConfigOption[theme.Theme]): Validate that the theme exists and build Theme instance. """ - def __init__(self, default=None): + def __init__(self, default=None) -> None: super().__init__() self.default = default - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): self.config_file_path = config.config_file_path - def run_validation(self, value) -> theme.Theme: + def run_validation(self, value: object) -> theme.Theme: if value is None and self.default is not None: theme_config = {'name': self.default} elif isinstance(value, str): @@ -741,6 +754,7 @@ def run_validation(self, value) -> theme.Theme: # Ensure custom_dir is an absolute path if 'custom_dir' in theme_config and not os.path.isabs(theme_config['custom_dir']): + assert self.config_file_path is not None config_dir = os.path.dirname(self.config_file_path) theme_config['custom_dir'] = os.path.join(config_dir, theme_config['custom_dir']) @@ -764,7 +778,7 @@ class Nav(OptionallyRequired): Validate the Nav config. """ - def run_validation(self, value, *, top=True): + def run_validation(self, value: object, *, top=True): if isinstance(value, list): for subitem in value: self._validate_nav_item(subitem) @@ -797,7 +811,7 @@ def _validate_nav_item(self, value): ) @classmethod - def _repr_item(cls, value): + def _repr_item(cls, value) -> str: if isinstance(value, dict) and value: return f"dict with keys {tuple(value.keys())}" elif isinstance(value, (str, type(None))): @@ -813,7 +827,7 @@ class Private(BaseConfigOption): A config option only for internal use. Raises an error if set by the user. """ - def run_validation(self, value): + def run_validation(self, value: object): if value is not None: raise ValidationError('For internal use only.') @@ -835,7 +849,7 @@ def __init__( configkey: str = 'mdx_configs', default: List[str] = [], **kwargs, - ): + ) -> None: super().__init__(default=default, **kwargs) self.builtins = builtins or [] self.configkey = configkey @@ -849,8 +863,8 @@ def validate_ext_cfg(self, ext, cfg): raise ValidationError(f"Invalid config options for Markdown Extension '{ext}'.") self.configdata[ext] = cfg - def run_validation(self, value): - self.configdata = {} + def run_validation(self, value: object): + self.configdata: Dict[str, dict] = {} if not isinstance(value, (list, tuple, dict)): raise ValidationError('Invalid Markdown Extensions configuration') extensions = [] @@ -879,7 +893,7 @@ def run_validation(self, value): try: md.registerExtensions((ext,), self.configdata) except Exception as e: - stack = [] + stack: list = [] for frame in reversed(traceback.extract_tb(sys.exc_info()[2])): if not frame.line: # Ignore frames before break @@ -892,7 +906,7 @@ def run_validation(self, value): return extensions - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): config[self.configkey] = self.configdata @@ -904,16 +918,16 @@ class Plugins(OptionallyRequired[plugins.PluginCollection]): initializing the plugin class. """ - def __init__(self, **kwargs): + def __init__(self, **kwargs) -> None: super().__init__(**kwargs) self.installed_plugins = plugins.get_plugins() - self.config_file_path = None + self.config_file_path: t.Optional[str] = None self.plugin_cache: Dict[str, plugins.BasePlugin] = {} - def pre_validation(self, config, key_name): + def pre_validation(self, config: Config, key_name: str): self.config_file_path = config.config_file_path - def run_validation(self, value): + def run_validation(self, value: object) -> plugins.PluginCollection: if not isinstance(value, (list, tuple, dict)): raise ValidationError('Invalid Plugins configuration. Expected a list or dict.') self.plugins = plugins.PluginCollection() @@ -966,15 +980,23 @@ def load_plugin(self, name, config): return plugin -class Hooks(ListOfItems): +class Hooks(BaseConfigOption[List[types.ModuleType]]): """A list of Python scripts to be treated as instances of plugins.""" - def __init__(self, plugins_key: str): - super().__init__(File(exists=True), default=[]) + def __init__(self, plugins_key: str) -> None: + super().__init__() + self.default = [] self.plugins_key = plugins_key - def run_validation(self, value): - paths = super().run_validation(value) + def pre_validation(self, config: Config, key_name: str): + self._base_option = ListOfItems(File(exists=True)) + self._base_option.pre_validation(config, key_name) + + def run_validation(self, value: object) -> Mapping[str, Any]: + paths = self._base_option.validate(value) + self.warnings.extend(self._base_option.warnings) + value = t.cast(List[str], value) + hooks = {} for name, path in zip(value, paths): hooks[name] = self._load_hook(name, path) @@ -991,7 +1013,7 @@ def _load_hook(self, name, path): spec.loader.exec_module(module) return module - def post_validation(self, config, key_name): + def post_validation(self, config: Config, key_name: str): plugins = config[self.plugins_key] for name, hook in config[key_name].items(): plugins[name] = hook diff --git a/mkdocs/tests/config/config_options_legacy_tests.py b/mkdocs/tests/config/config_options_legacy_tests.py index 4c2a0560a4..7d92651427 100644 --- a/mkdocs/tests/config/config_options_legacy_tests.py +++ b/mkdocs/tests/config/config_options_legacy_tests.py @@ -35,7 +35,7 @@ def get_config( self, schema: type, cfg: Dict[str, Any], - warnings={}, + warnings: Dict[str, str] = {}, config_file_path=None, ): config = base.LegacyConfig(base.get_schema(schema), config_file_path=config_file_path) @@ -460,7 +460,7 @@ def test_invalid_type(self): class Schema: option = c.URL() - with self.expect_error(option="Unable to parse the URL."): + with self.expect_error(option="Expected a string, got "): self.get_config(Schema, {'option': 1}) @@ -1235,7 +1235,7 @@ class Schema: conf = self.get_config( Schema, {'option': {'unknown': 0}}, - warnings=dict(option=('unknown', 'Unrecognised configuration name: unknown')), + warnings=dict(option="Sub-option 'unknown': Unrecognised configuration name: unknown"), ) self.assertEqual(conf['option'], {"unknown": 0}) @@ -1592,7 +1592,7 @@ class Schema: self.assertIsNone(conf['mdx_configs'].get('toc')) -class TestHooks(TestCase): +class HooksTest(TestCase): class Schema: plugins = c.Plugins(default=[]) hooks = c.Hooks('plugins') diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index a0a72a43b5..ce598fd030 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -47,7 +47,7 @@ def get_config( self, config_class: Type[SomeConfig], cfg: Dict[str, Any], - warnings={}, + warnings: Dict[str, str] = {}, config_file_path=None, ) -> SomeConfig: config = config_class(config_file_path=config_file_path) @@ -447,7 +447,7 @@ def test_invalid_type(self) -> None: class Schema(Config): option = c.URL() - with self.expect_error(option="Unable to parse the URL."): + with self.expect_error(option="Expected a string, got "): self.get_config(Schema, {'option': 1}) @@ -1240,7 +1240,7 @@ class Schema(Config): conf = self.get_config( Schema, {'option': {'unknown': 0}}, - warnings=dict(option=('unknown', 'Unrecognised configuration name: unknown')), + warnings=dict(option="Sub-option 'unknown': Unrecognised configuration name: unknown"), ) self.assertEqual(conf.option, {"unknown": 0}) @@ -1819,7 +1819,7 @@ class Schema(Config): self.get_config(Schema, cfg) -class TestHooks(TestCase): +class HooksTest(TestCase): class Schema(Config): plugins = c.Plugins(default=[]) hooks = c.Hooks('plugins') @@ -1849,6 +1849,13 @@ def test_hooks(self, src_dir) -> None: self.assertEqual(hook.on_page_markdown('foo foo'), 'zoo zoo') # type: ignore[call-arg] self.assertFalse(hasattr(hook, 'on_nav')) + def test_hooks_wrong_type(self) -> None: + with self.expect_error(hooks="Expected a list of items, but a was given."): + self.get_config(self.Schema, {'hooks': 6}) + + with self.expect_error(hooks="Expected type: but received: "): + self.get_config(self.Schema, {'hooks': [7]}) + class SchemaTest(TestCase): def test_copy(self) -> None: From 6b4d20e1eb922e010617ee65f48dc470a82a3584 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 8 Oct 2022 14:09:08 +0200 Subject: [PATCH 4/7] Refactor plugin_cache usage to avoid nested exception --- mkdocs/config/config_options.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 2b975830de..0c3fc5c286 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -956,18 +956,17 @@ def load_plugin(self, name, config): if not isinstance(config, dict): raise ValidationError(f"Invalid config options for the '{name}' plugin.") - try: - plugin = self.plugin_cache[name] - except KeyError: - Plugin = self.installed_plugins[name].load() + plugin = self.plugin_cache.get(name) + if plugin is None: + plugin_cls = self.installed_plugins[name].load() - if not issubclass(Plugin, plugins.BasePlugin): + if not issubclass(plugin_cls, plugins.BasePlugin): raise ValidationError( - f'{Plugin.__module__}.{Plugin.__name__} must be a subclass of' + f'{plugin_cls.__module__}.{plugin_cls.__name__} must be a subclass of' f' {plugins.BasePlugin.__module__}.{plugins.BasePlugin.__name__}' ) - plugin = Plugin() + plugin = plugin_cls() if hasattr(plugin, 'on_startup') or hasattr(plugin, 'on_shutdown'): self.plugin_cache[name] = plugin From 9ada9bf96918153fd6e28eee5292e63040f57d23 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 8 Oct 2022 14:28:09 +0200 Subject: [PATCH 5/7] Also tighten warnings of plugin configs --- mkdocs/config/config_options.py | 4 +- mkdocs/tests/config/config_options_tests.py | 59 +++++++++++++++++++-- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 0c3fc5c286..bc2d37f65c 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -946,7 +946,7 @@ def run_validation(self, value: object) -> plugins.PluginCollection: self.plugins[item] = self.load_plugin(item, cfg) return self.plugins - def load_plugin(self, name, config): + def load_plugin(self, name: str, config) -> plugins.BasePlugin: if not isinstance(name, str): raise ValidationError(f"'{name}' is not a valid plugin name.") if name not in self.installed_plugins: @@ -972,7 +972,7 @@ def load_plugin(self, name, config): self.plugin_cache[name] = plugin errors, warnings = plugin.load_config(config, self.config_file_path) - self.warnings.extend(warnings) + self.warnings.extend(f"Plugin '{name}' value: '{x}'. Warning: {y}" for x, y in warnings) errors_message = '\n'.join(f"Plugin '{name}' value: '{x}'. Error: {y}" for x, y in errors) if errors_message: raise ValidationError(errors_message) diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index ce598fd030..f33194251d 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1630,16 +1630,30 @@ class FakePlugin(BasePlugin[_FakePluginConfig]): pass +class _FakePlugin2Config(_FakePluginConfig): + depr = c.Deprecated() + + +class FakePlugin2(BasePlugin[_FakePlugin2Config]): + pass + + class FakeEntryPoint: - @property - def name(self): - return 'sample' + def __init__(self, name, cls): + self.name = name + self.cls = cls def load(self): - return FakePlugin + return self.cls -@patch('mkdocs.plugins.entry_points', return_value=[FakeEntryPoint()]) +@patch( + 'mkdocs.plugins.entry_points', + return_value=[ + FakeEntryPoint('sample', FakePlugin), + FakeEntryPoint('sample2', FakePlugin2), + ], +) class PluginsTest(TestCase): def test_plugin_config_without_options(self, mock_class) -> None: class Schema(Config): @@ -1818,6 +1832,41 @@ class Schema(Config): with self.expect_error(plugins="Invalid config options for the 'sample' plugin."): self.get_config(Schema, cfg) + def test_plugin_config_sub_error(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins(default=['sample']) + + cfg = { + 'plugins': { + 'sample': {'bar': 'not an int'}, + } + } + with self.expect_error( + plugins="Plugin 'sample' value: 'bar'. Error: Expected type: but received: " + ): + self.get_config(Schema, cfg) + + def test_plugin_config_sub_warning(self, mock_class) -> None: + class Schema(Config): + plugins = c.Plugins() + + cfg = { + 'plugins': { + 'sample2': {'depr': 'deprecated value'}, + } + } + conf = self.get_config( + Schema, + cfg, + warnings=dict( + plugins="Plugin 'sample2' value: 'depr'. Warning: The configuration option " + "'depr' has been deprecated and will be removed in a future release of MkDocs." + ), + ) + + self.assertIsInstance(conf.plugins, PluginCollection) + self.assertIn('sample2', conf.plugins) + class HooksTest(TestCase): class Schema(Config): From b964e05372b04a557663f5a963c1c772f8b0f4c6 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 8 Oct 2022 14:45:07 +0200 Subject: [PATCH 6/7] Refactor `load_plugin` usage --- mkdocs/config/config_options.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index bc2d37f65c..18a5ddab3b 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -15,6 +15,7 @@ Collection, Dict, Generic, + Iterator, List, Mapping, NamedTuple, @@ -931,24 +932,31 @@ def run_validation(self, value: object) -> plugins.PluginCollection: if not isinstance(value, (list, tuple, dict)): raise ValidationError('Invalid Plugins configuration. Expected a list or dict.') self.plugins = plugins.PluginCollection() + for name, cfg in self._parse_configs(value): + self.plugins[name] = self.load_plugin(name, cfg) + return self.plugins + + @classmethod + def _parse_configs(cls, value: Union[list, tuple, dict]) -> Iterator[Tuple[str, dict]]: if isinstance(value, dict): for name, cfg in value.items(): - self.plugins[name] = self.load_plugin(name, cfg) + if not isinstance(name, str): + raise ValidationError(f"'{name}' is not a valid plugin name.") + yield name, cfg else: for item in value: if isinstance(item, dict): if len(item) > 1: raise ValidationError('Invalid Plugins configuration') name, cfg = item.popitem() - item = name else: + name = item cfg = {} - self.plugins[item] = self.load_plugin(item, cfg) - return self.plugins + if not isinstance(name, str): + raise ValidationError(f"'{name}' is not a valid plugin name.") + yield name, cfg def load_plugin(self, name: str, config) -> plugins.BasePlugin: - if not isinstance(name, str): - raise ValidationError(f"'{name}' is not a valid plugin name.") if name not in self.installed_plugins: raise ValidationError(f'The "{name}" plugin is not installed') From aaf819f183fec9f2bda14449ad9fc875ad830e45 Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 8 Oct 2022 15:07:34 +0200 Subject: [PATCH 7/7] Better guard an edge case in plugin config --- mkdocs/config/config_options.py | 2 +- mkdocs/tests/config/config_options_tests.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py index 18a5ddab3b..689092ea18 100644 --- a/mkdocs/config/config_options.py +++ b/mkdocs/config/config_options.py @@ -946,7 +946,7 @@ def _parse_configs(cls, value: Union[list, tuple, dict]) -> Iterator[Tuple[str, else: for item in value: if isinstance(item, dict): - if len(item) > 1: + if len(item) != 1: raise ValidationError('Invalid Plugins configuration') name, cfg = item.popitem() else: diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index f33194251d..34c09673f3 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1812,6 +1812,14 @@ class Schema(Config): with self.expect_error(plugins="Invalid Plugins configuration"): self.get_config(Schema, cfg) + cfg = { + 'plugins': [ + {}, + ], + } + with self.expect_error(plugins="Invalid Plugins configuration"): + self.get_config(Schema, cfg) + def test_plugin_config_not_string_or_dict(self, mock_class) -> None: class Schema(Config): plugins = c.Plugins()