From 5edca97a2ef091d969fcd550aa436bb7fedad71a Mon Sep 17 00:00:00 2001
From: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Date: Tue, 16 Jul 2024 20:48:49 +0100
Subject: [PATCH] Warn when ``html_sidebars`` values are strings (#12600)

---
 CHANGES.rst                            |  4 ++++
 sphinx/builders/html/__init__.py       | 29 +++++++++++++++++++++-----
 tests/test_builders/test_build_html.py | 28 ++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index a1d722d9de5..007108b33f6 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,10 @@ Release 7.4.5 (in development)
 Bugs fixed
 ----------
 
+* #12593, #12600: Revert coercing the type of selected :confval:`html_sidebars`
+  values to a list.
+  Log an error message when string values are detected.
+  Patch by Adam Turner.
 
 Release 7.4.4 (released Jul 15, 2024)
 =====================================
diff --git a/sphinx/builders/html/__init__.py b/sphinx/builders/html/__init__.py
index 2ea8cd4f8ff..989079f3a82 100644
--- a/sphinx/builders/html/__init__.py
+++ b/sphinx/builders/html/__init__.py
@@ -982,11 +982,10 @@ def has_wildcard(pattern: str) -> bool:
                 matched = pattern
                 sidebars = pat_sidebars
 
-        if len(sidebars) == 0:
-            # keep defaults
-            pass
-
-        ctx['sidebars'] = list(sidebars)
+        # See error_on_html_sidebars_string_values.
+        # Replace with simple list coercion in Sphinx 8.0
+        # xref: RemovedInSphinx80Warning
+        ctx['sidebars'] = sidebars
 
     # --------- these are overwritten by the serialization builder
 
@@ -1287,6 +1286,25 @@ def validate_html_favicon(app: Sphinx, config: Config) -> None:
         config.html_favicon = None
 
 
+def error_on_html_sidebars_string_values(app: Sphinx, config: Config) -> None:
+    """Support removed in Sphinx 2."""
+    errors = {}
+    for pattern, pat_sidebars in config.html_sidebars.items():
+        if isinstance(pat_sidebars, str):
+            errors[pattern] = [pat_sidebars]
+    if not errors:
+        return
+    msg = __("Values in 'html_sidebars' must be a list of strings. "
+             "At least one pattern has a string value: %s. "
+             "Change to `html_sidebars = %r`.")
+    bad_patterns = ', '.join(map(repr, errors))
+    fixed = config.html_sidebars | errors
+    logger.error(msg, bad_patterns, fixed)
+    # Enable hard error in next major version.
+    # xref: RemovedInSphinx80Warning
+    # raise ConfigError(msg % (bad_patterns, fixed))
+
+
 def error_on_html_4(_app: Sphinx, config: Config) -> None:
     """Error on HTML 4."""
     if config.html4_writer:
@@ -1357,6 +1375,7 @@ def setup(app: Sphinx) -> ExtensionMetadata:
     app.connect('config-inited', validate_html_static_path, priority=800)
     app.connect('config-inited', validate_html_logo, priority=800)
     app.connect('config-inited', validate_html_favicon, priority=800)
+    app.connect('config-inited', error_on_html_sidebars_string_values, priority=800)
     app.connect('config-inited', error_on_html_4, priority=800)
     app.connect('builder-inited', validate_math_renderer)
     app.connect('html-page-context', setup_resource_paths)
diff --git a/tests/test_builders/test_build_html.py b/tests/test_builders/test_build_html.py
index eb32474456b..8db0790d577 100644
--- a/tests/test_builders/test_build_html.py
+++ b/tests/test_builders/test_build_html.py
@@ -1,5 +1,6 @@
 """Test the HTML builder and check output against XPath."""
 
+import contextlib
 import os
 import posixpath
 import re
@@ -8,7 +9,7 @@
 
 from sphinx.builders.html import validate_html_extra_path, validate_html_static_path
 from sphinx.deprecation import RemovedInSphinx80Warning
-from sphinx.errors import ConfigError
+from sphinx.errors import ConfigError, ThemeError
 from sphinx.util.console import strip_colors
 from sphinx.util.inventory import InventoryFile
 
@@ -16,6 +17,31 @@
 from tests.test_builders.xpath_util import check_xpath
 
 
+def test_html_sidebars_error(make_app, tmp_path):
+    (tmp_path / 'conf.py').touch()
+    (tmp_path / 'index.rst').touch()
+    app = make_app(
+        buildername='html',
+        srcdir=tmp_path,
+        confoverrides={'html_sidebars': {'index': 'searchbox.html'}},
+    )
+
+    # Test that the error is logged
+    warnings = app.warning.getvalue()
+    assert ("ERROR: Values in 'html_sidebars' must be a list of strings. "
+            "At least one pattern has a string value: 'index'. "
+            "Change to `html_sidebars = {'index': ['searchbox.html']}`.") in warnings
+
+    # But that the value is unchanged.
+    # (Remove this bit of the test in Sphinx 8)
+    def _html_context_hook(app, pagename, templatename, context, doctree):
+        assert context["sidebars"] == 'searchbox.html'
+    app.connect('html-page-context', _html_context_hook)
+    with contextlib.suppress(ThemeError):
+        # ignore template rendering issues (ThemeError).
+        app.build()
+
+
 def test_html4_error(make_app, tmp_path):
     (tmp_path / 'conf.py').write_text('', encoding='utf-8')
     with pytest.raises(