Merged in lehmannro/sphinx-warnconfig (pull request #314)

Check configuration values for their types

4 files changed, 70 insertions, 3 deletions

diff --git a/CHANGES b/CHANGES
index 56cca7e2..46fb581b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,9 @@ Features added
 
 * #1597: Added possibility to return a new template name from
   `html-page-context`.
+* PR#314, #1150: Configuration values are now checked for their type.  A
+  warning is raised if the configured and the default value do not have the
+  same type and do not share a common non-trivial base class.
 
 Bugs fixed
 ----------
diff --git a/sphinx/application.py b/sphinx/application.py
index 630bff36..5f6b92f5 100644
--- a/sphinx/application.py
+++ b/sphinx/application.py
@@ -124,6 +124,7 @@ class Sphinx(object):
         self.config = Config(confdir, CONFIG_FILENAME,
                              confoverrides or {}, self.tags)
         self.config.check_unicode(self.warn)
+        # defer checking types until i18n has been initialized
 
         # set confdir to srcdir if -C given (!= no confdir); a few pieces
         # of code expect a confdir to be set
@@ -172,6 +173,8 @@ class Sphinx(object):
 
         # set up translation infrastructure
         self._init_i18n()
+        # check all configuration values for permissible types
+        self.config.check_types(self.warn)
         # set up the build environment
         self._init_env(freshenv)
         # set up the builder
diff --git a/sphinx/config.py b/sphinx/config.py
index cf6bed08..b8a3d664 100644
--- a/sphinx/config.py
+++ b/sphinx/config.py
@@ -224,7 +224,7 @@ class Config(object):
         self.overrides = overrides
         self.values = Config.config_values.copy()
         config = {}
-        if 'extensions' in overrides:
+        if 'extensions' in overrides: #XXX do we need this?
             if isinstance(overrides['extensions'], string_types):
                 config['extensions'] = overrides.pop('extensions').split(',')
             else:
@@ -249,6 +249,30 @@ class Config(object):
         self.setup = config.get('setup', None)
         self.extensions = config.get('extensions', [])
 
+    def check_types(self, warn):
+        # check all values for deviation from the default value's type, since
+        # that can result in TypeErrors all over the place
+        # NB. since config values might use l_() we have to wait with calling
+        # this method until i18n is initialized
+        for name in self._raw_config:
+            if name not in Config.config_values:
+                continue  # we don't know a default value
+            default, dummy_rebuild = Config.config_values[name]
+            if hasattr(default, '__call__'):
+                default = default(self)  # could invoke l_()
+            if default is None:
+                continue
+            current = self[name]
+            if type(current) is type(default):
+                continue
+            common_bases = (set(type(current).__bases__ + (type(current),))
+                          & set(type(default).__bases__))
+            common_bases.discard(object)
+            if common_bases:
+                continue  # at least we share a non-trivial base class
+            warn("the config value %r has type `%s', defaults to `%s.'"
+                    % (name, type(current).__name__, type(default).__name__))
+
     def check_unicode(self, warn):
         # check all string values for non-ASCII characters in bytestrings,
         # since that can result in UnicodeErrors all over the place
@@ -296,7 +320,6 @@ class Config(object):
         for name in config:
             if name in self.values:
                 self.__dict__[name] = config[name]
-        del self._raw_config
 
     def __getattr__(self, name):
         if name.startswith('_'):
diff --git a/tests/test_config.py b/tests/test_config.py
index 0dcf3fa3..e48079e3 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -9,7 +9,7 @@
     :copyright: Copyright 2007-2014 by the Sphinx team, see AUTHORS.
     :license: BSD, see LICENSE for details.
 """
-from six import PY3
+from six import PY2, PY3, StringIO
 
 from util import TestApp, with_app, with_tempdir, raises, raises_msg
 
@@ -133,3 +133,41 @@ def test_config_eol(tmpdir):
         cfg = Config(tmpdir, 'conf.py', {}, None)
         cfg.init_values(lambda warning: 1/0)
         assert cfg.project == u'spam'
+
+
+TYPECHECK_OVERRIDES = [
+    # configuration key, override value, should warn, default type
+    ('master_doc', 123, True, str),
+    ('man_pages', 123, True, list),  # lambda
+    ('man_pages', [], False, list),
+    ('epub_tocdepth', True, True, int),  # child type
+    ('nitpicky', 3, False, bool),  # parent type
+    ('templates_path', (), True, list),  # other sequence, also raises
+]
+if PY2:
+    # Run a check for proper sibling detection in Python 2.  Under py3k, the
+    # default types do not have any siblings.
+    TYPECHECK_OVERRIDES.append(
+            ('html_add_permalinks', 'bar', False, unicode))
+
+def test_gen_check_types():
+    for key, value, should, deftype in TYPECHECK_OVERRIDES:
+        warning = StringIO()
+        try:
+            app = TestApp(confoverrides={key: value}, warning=warning)
+        except:
+            pass
+        else:
+            app.cleanup()
+
+        real = type(value).__name__
+        msg = ("WARNING: the config value %r has type `%s',"
+               " defaults to `%s.'\n" % (key, real, deftype.__name__))
+        def test():
+            assert (msg in warning.buflist) == should, \
+                    "Setting %s to %r should%s raise: %s" % \
+                    (key, value, " not" if should else "", msg)
+        test.description = "test_check_type_%s_on_%s" % \
+                (real, type(Config.config_values[key][0]).__name__)
+
+        yield test