From 73e8fef5068d47ab7bdc4c49bc4abcc74434b57e Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Sat, 20 Aug 2022 21:31:46 +0200 Subject: [PATCH] Rework ConfigOption schemas as class-based This is NOT a breaking change, the old style keeps working. Now developers can make a subclass of Config, declare the schema of the config as fields of the class, and instances of this class will hold the processed config. This better represents the relationship between what a config definition and a config instance is, now you think of configs definitions as classes and parsed configs as instances. We also can write these fields as descriptors and enable safe attribute-based access. Static analysis will be able to see when a missing fields is accessed. And in followup changes I plan to add type annotations which will make even type checking fully sound. --- mkdocs/config/base.py | 78 +++++++++++++++------ mkdocs/config/defaults.py | 8 ++- mkdocs/contrib/search/__init__.py | 4 +- mkdocs/plugins.py | 19 ++++- mkdocs/tests/base.py | 8 +-- mkdocs/tests/config/base_tests.py | 18 ++--- mkdocs/tests/config/config_options_tests.py | 20 +++++- mkdocs/tests/config/config_tests.py | 8 +-- mkdocs/tests/plugin_tests.py | 4 +- 9 files changed, 115 insertions(+), 52 deletions(-) diff --git a/mkdocs/config/base.py b/mkdocs/config/base.py index b5d4b1fc..19becabf 100644 --- a/mkdocs/config/base.py +++ b/mkdocs/config/base.py @@ -61,6 +61,21 @@ class BaseConfigOption: The post-validation process method should be implemented by subclasses. """ + def __set_name__(self, owner, name): + self._name = name + + def __get__(self, obj, type=None): + if not isinstance(obj, Config): + return self + return obj[self._name] + + def __set__(self, obj, value): + if not isinstance(obj, Config): + raise AttributeError( + f"can't set attribute ({self._name}) because the parent is a {type(obj)} not a {Config}" + ) + obj[self._name] = value + class ValidationError(Exception): """Raised during the validation process of the config on errors.""" @@ -78,20 +93,35 @@ ConfigWarnings = List[Tuple[str, str]] class Config(UserDict): """ - MkDocs Configuration dict + Base class for MkDocs configuration, plugin configuration (and sub-configuration) objects. - This is a fairly simple extension of a standard dictionary. It adds methods - for running validation on the structure and contents. + It should be subclassed and have `ConfigOption`s defined as attributes. + For examples, see mkdocs/contrib/search/__init__.py and mkdocs/config/defaults.py. + + Behavior as it was prior to MkDocs 1.4 is now handled by LegacyConfig. """ - def __init__( - self, schema: PlainConfigSchema, config_file_path: Optional[Union[str, bytes]] = None - ) -> None: - """ - The schema is a Python dict which maps the config name to a validator. - """ - self._schema = schema - self._schema_keys = set(dict(schema).keys()) + _schema: PlainConfigSchema + + def __init_subclass__(cls): + schema = dict(getattr(cls, '_schema', ())) + for attr_name, attr in cls.__dict__.items(): + if isinstance(attr, BaseConfigOption): + schema[attr_name] = attr + cls._schema = tuple(schema.items()) + + def __new__(cls, *args, **kwargs) -> Config: + """Compatibility: allow referring to `LegacyConfig(...)` constructor as `Config(...)`.""" + if cls is Config: + return LegacyConfig(*args, **kwargs) + return super().__new__(cls) + + def __init__(self, config_file_path: Optional[Union[str, bytes]] = None): + super().__init__() + self.user_configs: List[dict] = [] + self.set_defaults() + + self._schema_keys = {k for k, v in self._schema} # Ensure config_file_path is a Unicode string if config_file_path is not None and not isinstance(config_file_path, str): try: @@ -100,10 +130,6 @@ class Config(UserDict): except UnicodeDecodeError: raise ValidationError("config_file_path is not a Unicode string.") self.config_file_path = config_file_path - self.data = {} - - self.user_configs: List[dict] = [] - self.set_defaults() def set_defaults(self) -> None: """ @@ -187,7 +213,7 @@ class Config(UserDict): ) self.user_configs.append(patch) - self.data.update(patch) + self.update(patch) def load_file(self, config_file: IO) -> None: """Load config options from the open file descriptor of a YAML file.""" @@ -204,12 +230,22 @@ class Config(UserDict): def get_schema(cls: type) -> PlainConfigSchema: """ Extract ConfigOptions defined in a class (used just as a container) and put them into a schema tuple. - - See mkdocs/config/defaults.py for an example. """ + if issubclass(cls, Config): + return cls._schema return tuple((k, v) for k, v in cls.__dict__.items() if isinstance(v, BaseConfigOption)) +class LegacyConfig(Config): + """ + A configuration object for plugins, as just a dict without type-safe attribute access. + """ + + def __init__(self, schema: PlainConfigSchema, config_file_path: Optional[str] = None): + self._schema = schema + super().__init__(config_file_path) + + @contextmanager def _open_config_file(config_file: Optional[Union[str, IO]]) -> Iterator[IO]: """ @@ -277,12 +313,10 @@ def load_config(config_file: Optional[Union[str, IO]] = None, **kwargs) -> Confi options.pop(key) with _open_config_file(config_file) as fd: - options['config_file_path'] = getattr(fd, 'name', '') - # Initialize the config with the default schema. - from mkdocs.config import defaults + from mkdocs.config.defaults import MkDocsConfig - cfg = Config(schema=defaults.get_schema(), config_file_path=options['config_file_path']) + cfg = MkDocsConfig(config_file_path=getattr(fd, 'name', '')) # load the config file cfg.load_file(fd) diff --git a/mkdocs/config/defaults.py b/mkdocs/config/defaults.py index a7529d97..8c59c6a5 100644 --- a/mkdocs/config/defaults.py +++ b/mkdocs/config/defaults.py @@ -5,14 +5,16 @@ from mkdocs.config import config_options as c def get_schema() -> base.PlainConfigSchema: - return base.get_schema(_MkDocsConfig) + return MkDocsConfig._schema # NOTE: The order here is important. During validation some config options # depend on others. So, if config option A depends on B, then A should be # listed higher in the schema. -class _MkDocsConfig: - config_file_path = c.Type(str) +class MkDocsConfig(base.Config): + """The configuration of MkDocs itself (the root object of mkdocs.yml).""" + + config_file_path = c.Type(str) # type: ignore[assignment] """Reserved for internal use, stores the mkdocs.yml config file.""" site_name = c.Type(str, required=True) diff --git a/mkdocs/contrib/search/__init__.py b/mkdocs/contrib/search/__init__.py index 6dd7ba07..2873d82c 100644 --- a/mkdocs/contrib/search/__init__.py +++ b/mkdocs/contrib/search/__init__.py @@ -46,7 +46,7 @@ class LangOption(c.OptionallyRequired): return value -class _PluginConfig: +class _PluginConfig(base.Config): lang = LangOption() separator = c.Type(str, default=r'[\s\-]+') min_search_length = c.Type(int, default=3) @@ -57,7 +57,7 @@ class _PluginConfig: class SearchPlugin(BasePlugin): """Add a search feature to MkDocs.""" - config_scheme = base.get_schema(_PluginConfig) + config_class = _PluginConfig def on_config(self, config: Config, **kwargs) -> Config: "Add plugin templates and scripts to config." diff --git a/mkdocs/plugins.py b/mkdocs/plugins.py index 25d63f7a..eb707820 100644 --- a/mkdocs/plugins.py +++ b/mkdocs/plugins.py @@ -7,7 +7,7 @@ from __future__ import annotations import logging import sys from collections import OrderedDict -from typing import Any, Callable, Dict, List, Optional, Tuple, TypeVar, overload +from typing import Any, Callable, Dict, List, Optional, Tuple, Type, TypeVar, overload if sys.version_info >= (3, 10): from importlib.metadata import EntryPoint, entry_points @@ -22,7 +22,7 @@ else: import jinja2.environment from mkdocs import utils -from mkdocs.config.base import Config, ConfigErrors, ConfigWarnings, PlainConfigSchema +from mkdocs.config.base import Config, ConfigErrors, ConfigWarnings, LegacyConfig, PlainConfigSchema from mkdocs.livereload import LiveReloadServer from mkdocs.structure.files import Files from mkdocs.structure.nav import Navigation @@ -54,15 +54,28 @@ class BasePlugin: All plugins should subclass this class. """ + config_class: Type[Config] = LegacyConfig config_scheme: PlainConfigSchema = () config: Config = {} # type: ignore[assignment] + def __init_subclass__(cls): + if not issubclass(cls.config_class, Config): + raise TypeError( + f"config_class {cls.config_class} must be a subclass of `mkdocs.config.base.Config`" + ) + if cls.config_class is not LegacyConfig: + cls.config_scheme = cls.config_class._schema # For compatibility. + def load_config( self, options: Dict[str, Any], config_file_path: Optional[str] = None ) -> Tuple[ConfigErrors, ConfigWarnings]: """Load config from a dict of options. Returns a tuple of (errors, warnings).""" - self.config = Config(schema=self.config_scheme, config_file_path=config_file_path) + if self.config_class is LegacyConfig: + self.config = LegacyConfig(self.config_scheme, config_file_path=config_file_path) + else: + self.config = self.config_class(config_file_path=config_file_path) + self.config.load_dict(options) return self.config.validate() diff --git a/mkdocs/tests/base.py b/mkdocs/tests/base.py index b15de7f5..b6eee458 100644 --- a/mkdocs/tests/base.py +++ b/mkdocs/tests/base.py @@ -6,8 +6,8 @@ from tempfile import TemporaryDirectory import markdown -from mkdocs import config, utils -from mkdocs.config import defaults as config_defaults +from mkdocs import utils +from mkdocs.config.defaults import MkDocsConfig def dedent(text): @@ -32,9 +32,7 @@ def load_config(**cfg): if 'docs_dir' not in cfg: # Point to an actual dir to avoid a 'does not exist' error on validation. cfg['docs_dir'] = os.path.join(path_base, 'docs') - conf = config.Config( - schema=config_defaults.get_schema(), config_file_path=cfg['config_file_path'] - ) + conf = MkDocsConfig(config_file_path=cfg['config_file_path']) conf.load_dict(cfg) errors_warnings = conf.validate() diff --git a/mkdocs/tests/config/base_tests.py b/mkdocs/tests/config/base_tests.py index b87dc420..70f96787 100644 --- a/mkdocs/tests/config/base_tests.py +++ b/mkdocs/tests/config/base_tests.py @@ -11,7 +11,7 @@ from mkdocs.tests.base import change_dir, tempdir class ConfigBaseTests(unittest.TestCase): def test_unrecognised_keys(self): - conf = base.Config(schema=defaults.get_schema()) + conf = defaults.MkDocsConfig() conf.load_dict( { 'not_a_valid_config_option': "test", @@ -31,7 +31,7 @@ class ConfigBaseTests(unittest.TestCase): ) def test_missing_required(self): - conf = base.Config(schema=defaults.get_schema()) + conf = defaults.MkDocsConfig() errors, warnings = conf.validate() @@ -51,7 +51,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_dir, 'docs')) cfg = base.load_config(config_file=config_file.name) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') @tempdir() @@ -64,7 +64,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_dir, 'docs')) with change_dir(temp_dir): cfg = base.load_config(config_file=None) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') @tempdir @@ -77,7 +77,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_dir, 'docs')) with change_dir(temp_dir): cfg = base.load_config(config_file=None) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') @tempdir() @@ -93,7 +93,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_dir, 'docs')) with change_dir(temp_dir): cfg = base.load_config(config_file=None) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test1') def test_load_from_missing_file(self): @@ -114,7 +114,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_path, 'docs')) cfg = base.load_config(config_file=config_file) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') # load_config will always close the file self.assertTrue(config_file.closed) @@ -130,7 +130,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(os.path.join(temp_dir, 'docs')) cfg = base.load_config(config_file=config_file) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') @tempdir @@ -264,7 +264,7 @@ class ConfigBaseTests(unittest.TestCase): os.mkdir(docs_dir) cfg = base.load_config(config_file=config_file) - self.assertTrue(isinstance(cfg, base.Config)) + self.assertTrue(isinstance(cfg, defaults.MkDocsConfig)) self.assertEqual(cfg['site_name'], 'MkDocs Test') self.assertEqual(cfg['docs_dir'], docs_dir) self.assertEqual(cfg.config_file_path, config_fname) diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py index 1fb70156..0f795d4c 100644 --- a/mkdocs/tests/config/config_options_tests.py +++ b/mkdocs/tests/config/config_options_tests.py @@ -1,4 +1,5 @@ import contextlib +import copy import io import os import re @@ -37,7 +38,7 @@ class TestCase(unittest.TestCase): warnings={}, config_file_path=None, ): - config = base.Config(base.get_schema(schema), config_file_path=config_file_path) + config = base.LegacyConfig(base.get_schema(schema), config_file_path=config_file_path) config.load_dict(cfg) actual_errors, actual_warnings = config.validate() if actual_errors: @@ -1612,3 +1613,20 @@ class TestHooks(TestCase): ) self.assertEqual(hook.on_page_markdown('foo foo'), 'zoo zoo') self.assertFalse(hasattr(hook, 'on_nav')) + + +class SchemaTest(TestCase): + def test_copy(self): + copy.deepcopy( + base.LegacyConfig( + (('foo', c.MarkdownExtensions()),), + ), + ) + + copy.deepcopy(self.get_config(IpAddressTest.Schema, {'option': '1.2.3.4:5678'})) + copy.deepcopy(IpAddressTest.Schema) + copy.deepcopy(base.get_schema(IpAddressTest.Schema)) + + copy.deepcopy(self.get_config(EditURITest.Schema, {})) + copy.deepcopy(EditURITest.Schema) + copy.deepcopy(base.get_schema(EditURITest.Schema)) diff --git a/mkdocs/tests/config/config_tests.py b/mkdocs/tests/config/config_tests.py index 4b3c0dd5..7144e75f 100644 --- a/mkdocs/tests/config/config_tests.py +++ b/mkdocs/tests/config/config_tests.py @@ -12,8 +12,6 @@ from mkdocs.exceptions import ConfigurationError from mkdocs.localization import parse_locale from mkdocs.tests.base import dedent, tempdir -DEFAULT_SCHEMA = defaults.get_schema() - class ConfigTests(unittest.TestCase): def test_missing_config_file(self): @@ -21,7 +19,7 @@ class ConfigTests(unittest.TestCase): config.load_config(config_file='bad_filename.yaml') def test_missing_site_name(self): - conf = config.Config(schema=DEFAULT_SCHEMA) + conf = defaults.MkDocsConfig() conf.load_dict({}) errors, warnings = conf.validate() self.assertEqual( @@ -216,7 +214,7 @@ class ConfigTests(unittest.TestCase): self.assertEqual({k: conf['theme'][k] for k in iter(conf['theme'])}, result['vars']) def test_empty_nav(self): - conf = config.Config(schema=DEFAULT_SCHEMA) + conf = defaults.MkDocsConfig() conf.load_dict( { 'site_name': 'Example', @@ -227,7 +225,7 @@ class ConfigTests(unittest.TestCase): self.assertEqual(conf['nav'], None) def test_error_on_pages(self): - conf = config.Config(schema=DEFAULT_SCHEMA) + conf = defaults.MkDocsConfig() conf.load_dict( { 'site_name': 'Example', diff --git a/mkdocs/tests/plugin_tests.py b/mkdocs/tests/plugin_tests.py index 9fa8cf16..1d8aef69 100644 --- a/mkdocs/tests/plugin_tests.py +++ b/mkdocs/tests/plugin_tests.py @@ -14,14 +14,14 @@ from mkdocs.exceptions import Abort, BuildError, PluginError from mkdocs.tests.base import load_config -class _DummyPluginConfig: +class _DummyPluginConfig(base.Config): foo = c.Type(str, default='default foo') bar = c.Type(int, default=0) dir = c.Dir(exists=False) class DummyPlugin(plugins.BasePlugin): - config_scheme = base.get_schema(_DummyPluginConfig) + config_class = _DummyPluginConfig def on_pre_page(self, content, **kwargs): """modify page content by prepending `foo` config value."""