Repository: mkdocs/mkdocs
Pull request #2937 (2022-08-16 17:16:31)

Revert to the explicit way of registering plugins' methods
See prior: https://github.com/mkdocs/mkdocs/commit/f79b34d174e41084391868e7b503f5c61b8b1bdf#diff-da2086192f0b49a8ae9a67eba6ec5e7572aaa0c0db458b1d9deea21005fc7655L84

But now we need to also skip non-overridden methods. That's also the reason why I needed to use `staticmethod`, so these are not concealed by `bound method` functionality.



Patch:
diff --git a/mkdocs/config/config_options.py b/mkdocs/config/config_options.py
index 9f0108f5..6c5c315d 100644
--- a/mkdocs/config/config_options.py
+++ b/mkdocs/config/config_options.py
@@ -51,15 +51,34 @@ class BaseConfigOption:
 
 
 class SubConfig(BaseConfigOption):
-    def __init__(self, *config_options):
+    """
+    Subconfig Config Option
+
+    A set of `config_options` grouped under a single config option.
+    By default, validation errors and warnings resulting from validating
+    `config_options` are ignored (`validate=False`). Users should typically
+    enable validation with `validate=True`.
+    """
+
+    def __init__(self, *config_options, validate=False):
         super().__init__()
         self.default = {}
         self.config_options = config_options
+        self._do_validation = validate
 
     def run_validation(self, value):
         config = Config(self.config_options)
         config.load_dict(value)
-        config.validate()
+        failed, warnings = config.validate()
+
+        if self._do_validation:
+            # Capture errors and warnings
+            self.warnings = warnings
+            if failed:
+                # Get the first failing one
+                key, err = failed[0]
+                raise ValidationError(f"Sub-option {key!r} configuration error: {err}")
+
         return config
 
 
diff --git a/mkdocs/plugins.py b/mkdocs/plugins.py
index f66c6021..5778fbcb 100644
--- a/mkdocs/plugins.py
+++ b/mkdocs/plugins.py
@@ -20,27 +20,6 @@ from mkdocs.structure.pages import Page
 log = logging.getLogger('mkdocs.plugins')
 
 
-EVENTS = (
-    'config',
-    'pre_build',
-    'files',
-    'nav',
-    'env',
-    'pre_template',
-    'template_context',
-    'post_template',
-    'pre_page',
-    'page_read_source',
-    'page_markdown',
-    'page_content',
-    'page_context',
-    'post_page',
-    'post_build',
-    'serve',
-    'build_error',
-)
-
-
 def get_plugins():
     """Return a dict of all installed Plugins as {name: EntryPoint}."""
 
@@ -77,10 +56,13 @@ class BasePlugin:
 
         return self.config.validate()
 
+    # (Note that event implementations shouldn't actually be static methods in subclasses)
+
     # Global events
 
+    @staticmethod
     def on_serve(
-        self, server: LiveReloadServer, config: Config, builder: Callable
+        server: LiveReloadServer, config: Config, builder: Callable
     ) -> Optional[LiveReloadServer]:
         """
         The `serve` event is only called when the `serve` command is used during
@@ -98,7 +80,8 @@ class BasePlugin:
         """
         return server
 
-    def on_config(self, config: Config) -> Optional[Config]:
+    @staticmethod
+    def on_config(config: Config) -> Optional[Config]:
         """
         The `config` event is the first event called on build and is run immediately
         after the user configuration is loaded and validated. Any alterations to the
@@ -112,7 +95,8 @@ class BasePlugin:
         """
         return config
 
-    def on_pre_build(self, config: Config) -> None:
+    @staticmethod
+    def on_pre_build(config: Config) -> None:
         """
         The `pre_build` event does not alter any variables. Use this event to call
         pre-build scripts.
@@ -121,7 +105,8 @@ class BasePlugin:
             config: global configuration object
         """
 
-    def on_files(self, files: Files, config: Config) -> Optional[Files]:
+    @staticmethod
+    def on_files(files: Files, config: Config) -> Optional[Files]:
         """
         The `files` event is called after the files collection is populated from the
         `docs_dir`. Use this event to add, remove, or alter files in the
@@ -138,7 +123,8 @@ class BasePlugin:
         """
         return files
 
-    def on_nav(self, nav: Navigation, config: Config, files: Files) -> Optional[Navigation]:
+    @staticmethod
+    def on_nav(nav: Navigation, config: Config, files: Files) -> Optional[Navigation]:
         """
         The `nav` event is called after the site navigation is created and can
         be used to alter the site navigation.
@@ -153,8 +139,9 @@ class BasePlugin:
         """
         return nav
 
+    @staticmethod
     def on_env(
-        self, env: jinja2.Environment, config: Config, files: Files
+        env: jinja2.Environment, config: Config, files: Files
     ) -> Optional[jinja2.Environment]:
         """
         The `env` event is called after the Jinja template environment is created
@@ -171,7 +158,8 @@ class BasePlugin:
         """
         return env
 
-    def on_post_build(self, config: Config) -> None:
+    @staticmethod
+    def on_post_build(config: Config) -> None:
         """
         The `post_build` event does not alter any variables. Use this event to call
         post-build scripts.
@@ -180,7 +168,8 @@ class BasePlugin:
             config: global configuration object
         """
 
-    def on_build_error(self, error: Exception) -> None:
+    @staticmethod
+    def on_build_error(error: Exception) -> None:
         """
         The `build_error` event is called after an exception of any kind
         is caught by MkDocs during the build process.
@@ -194,8 +183,9 @@ class BasePlugin:
 
     # Template events
 
+    @staticmethod
     def on_pre_template(
-        self, template: jinja2.Template, template_name: str, config: Config
+        template: jinja2.Template, template_name: str, config: Config
     ) -> Optional[jinja2.Template]:
         """
         The `pre_template` event is called immediately after the subject template is
@@ -211,8 +201,9 @@ class BasePlugin:
         """
         return template
 
+    @staticmethod
     def on_template_context(
-        self, context: Dict[str, Any], template_name: str, config: Config
+        context: Dict[str, Any], template_name: str, config: Config
     ) -> Optional[Dict[str, Any]]:
         """
         The `template_context` event is called immediately after the context is created
@@ -229,7 +220,8 @@ class BasePlugin:
         """
         return context
 
-    def on_post_template(self, output_content: str, template_name: str, config: Config) -> str:
+    @staticmethod
+    def on_post_template(output_content: str, template_name: str, config: Config) -> str:
         """
         The `post_template` event is called after the template is rendered, but before
         it is written to disc and can be used to alter the output of the template.
@@ -248,7 +240,8 @@ class BasePlugin:
 
     # Page events
 
-    def on_pre_page(self, page: Page, config: Config, files: Files) -> Optional[Page]:
+    @staticmethod
+    def on_pre_page(page: Page, config: Config, files: Files) -> Optional[Page]:
         """
         The `pre_page` event is called before any actions are taken on the subject
         page and can be used to alter the `Page` instance.
@@ -263,7 +256,8 @@ class BasePlugin:
         """
         return page
 
-    def on_page_read_source(self, page: Page, config: Config) -> Optional[str]:
+    @staticmethod
+    def on_page_read_source(page: Page, config: Config) -> Optional[str]:
         """
         The `on_page_read_source` event can replace the default mechanism to read
         the contents of a page's source from the filesystem.
@@ -278,9 +272,8 @@ class BasePlugin:
         """
         return None
 
-    def on_page_markdown(
-        self, markdown: str, page: Page, config: Config, files: Files
-    ) -> Optional[str]:
+    @staticmethod
+    def on_page_markdown(markdown: str, page: Page, config: Config, files: Files) -> Optional[str]:
         """
         The `page_markdown` event is called after the page's markdown is loaded
         from file and can be used to alter the Markdown source text. The meta-
@@ -297,7 +290,8 @@ class BasePlugin:
         """
         return markdown
 
-    def on_page_content(self, html: str, page: Page, config: Config, files: Files) -> Optional[str]:
+    @staticmethod
+    def on_page_content(html: str, page: Page, config: Config, files: Files) -> Optional[str]:
         """
         The `page_content` event is called after the Markdown text is rendered to
         HTML (but before being passed to a template) and can be used to alter the
@@ -314,8 +308,9 @@ class BasePlugin:
         """
         return html
 
+    @staticmethod
     def on_page_context(
-        self, context: Dict[str, Any], page: Page, config: Config, nav: Navigation
+        context: Dict[str, Any], page: Page, config: Config, nav: Navigation
     ) -> Optional[Dict[str, Any]]:
         """
         The `page_context` event is called after the context for a page is created
@@ -332,7 +327,8 @@ class BasePlugin:
         """
         return context
 
-    def on_post_page(self, output: str, page: Page, config: Config) -> str:
+    @staticmethod
+    def on_post_page(output: str, page: Page, config: Config) -> str:
         """
         The `post_page` event is called after the template is rendered, but
         before it is written to disc and can be used to alter the output of the
@@ -350,6 +346,8 @@ class BasePlugin:
         return output
 
 
+EVENTS = tuple(k[3:] for k in dir(BasePlugin) if k.startswith('on_'))
+
 T = TypeVar('T')
 
 
@@ -364,6 +362,11 @@ class PluginCollection(OrderedDict):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        self.events = {k: [] for k in EVENTS}
+
+    def _register_event(self, event_name, method):
+        """Register a method for an event."""
+        self.events[event_name].append(method)
 
     def __setitem__(self, key: str, value: BasePlugin, **kwargs):
         if not isinstance(value, BasePlugin):
@@ -373,6 +376,11 @@ class PluginCollection(OrderedDict):
                 ' subclasses'
             )
         super().__setitem__(key, value, **kwargs)
+        # 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)
+            if callable(method) and method is not getattr(BasePlugin, event_name):
+                self._register_event(event_name[3:], method)
 
     def run_event(self, name: str, item: T = None, **kwargs) -> T:
         """
@@ -384,11 +392,8 @@ class PluginCollection(OrderedDict):
         be modified by the event method.
         """
 
-        name = 'on_' + name
         pass_item = item is not None
-        getattr(BasePlugin, name)  # just to produce AttributeError
-        for plugin in list(self.values()):  # can change size during iteration
-            method = getattr(plugin, name)
+        for method in self.events[name]:
             if pass_item:
                 result = method(item, **kwargs)
             else:


Test patch:
diff --git a/mkdocs/tests/config/config_options_tests.py b/mkdocs/tests/config/config_options_tests.py
index c0fb9996..3b17ed7d 100644
--- a/mkdocs/tests/config/config_options_tests.py
+++ b/mkdocs/tests/config/config_options_tests.py
@@ -7,6 +7,7 @@ from unittest.mock import patch
 import mkdocs
 from mkdocs.config import config_options
 from mkdocs.config.base import Config
+from mkdocs.exceptions import ConfigurationError
 from mkdocs.tests.base import tempdir
 from mkdocs.utils import yaml_load
 
@@ -854,6 +855,66 @@ class PrivateTest(unittest.TestCase):
             option.validate('somevalue')
 
 
+class SubConfigTest(unittest.TestCase):
+    def test_subconfig_wrong_type(self):
+        # Test that an error is raised if subconfig does not receive a dict
+        option = config_options.SubConfig()
+        with self.assertRaises(ConfigurationError):
+            option.validate("not_a_dict")
+        with self.assertRaises(ConfigurationError):
+            option.validate(("not_a_dict",))
+        with self.assertRaises(ConfigurationError):
+            option.validate(["not_a_dict"])
+
+    def test_subconfig_default(self):
+        """Default behaviour of subconfig: validation is ignored"""
+
+        # Nominal
+        option = config_options.SubConfig(('c', config_options.Choice(('foo', 'bar'))))
+        res = option.validate(dict(c='foo'))
+        assert option.warnings == []
+        assert res == dict(c='foo')
+
+        # Invalid option ! No error
+        option = config_options.SubConfig(('c', config_options.Choice(('foo', 'bar'))))
+        res = option.validate(dict(c=True))
+        assert option.warnings == []
+        assert res == dict(c=True)
+
+        # Missing option ! Will be considered optional with default None
+        option = config_options.SubConfig(('c', config_options.Choice(('foo', 'bar'))))
+        res = option.validate(dict())
+        assert option.warnings == []
+        assert res == dict(c=None)
+
+        # Unknown option ! No warning
+        option = config_options.SubConfig(('c', config_options.Choice(('foo', 'bar'))))
+        res = option.validate({"unknown_key_is_ok": 0})
+        assert option.warnings == []
+        assert res == {"c": None, "unknown_key_is_ok": 0}
+
+    def test_subconfig_strict(self):
+        """Strict validation mode for subconfigs."""
+
+        # Unknown option: warning
+        option = config_options.SubConfig(validate=True)
+        res = option.validate({"unknown": 0})
+        assert option.warnings == [('unknown', 'Unrecognised configuration name: unknown')]
+        assert res == {"unknown": 0}
+
+        # Invalid option: error
+        option = config_options.SubConfig(
+            ('c', config_options.Choice(('foo', 'bar'))),
+            validate=True,
+        )
+        with self.assertRaises(config_options.ValidationError):
+            option.validate(dict(c=True))
+
+        # Nominal
+        res = option.validate(dict(c='foo'))
+        assert res == dict(c='foo')
+
+
 class MarkdownExtensionsTest(unittest.TestCase):
     @patch('markdown.Markdown')
     def test_simple_list(self, mockMd):
diff --git a/mkdocs/tests/plugin_tests.py b/mkdocs/tests/plugin_tests.py
index a8896df0..4047c175 100644
--- a/mkdocs/tests/plugin_tests.py
+++ b/mkdocs/tests/plugin_tests.py
@@ -145,7 +145,7 @@ class TestPluginCollection(unittest.TestCase):
 
     def test_run_unknown_event_on_collection(self):
         collection = plugins.PluginCollection()
-        with self.assertRaises(AttributeError):
+        with self.assertRaises(KeyError):
             collection.run_event('unknown', 'page content')
 
     def test_run_build_error_event(self):