From 8b00f57f4d2ba427a2a320b5b31493c7fe73ad73 Mon Sep 17 00:00:00 2001 From: Robert Lehmann Date: Fri, 11 Sep 2015 09:35:46 +0200 Subject: [PATCH 1/5] Fixed #1786: Add configurable type hints. This adds the option of giving, in addition to the type of the default value, hints about permissible types for configuration values. --- sphinx/application.py | 7 +-- sphinx/config.py | 82 +++++++++++++++++---------------- tests/roots/test-config/conf.py | 32 +++++++++++++ tests/test_config.py | 61 +++++++++++------------- tests/util.py | 17 +++---- 5 files changed, 116 insertions(+), 83 deletions(-) create mode 100644 tests/roots/test-config/conf.py diff --git a/sphinx/application.py b/sphinx/application.py index db456e3f7..9ae7c8acd 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -535,13 +535,14 @@ class Sphinx(object): builder.name, self.builderclasses[builder.name].__module__)) self.builderclasses[builder.name] = builder - def add_config_value(self, name, default, rebuild): - self.debug('[app] adding config value: %r', (name, default, rebuild)) + def add_config_value(self, name, default, rebuild, types=()): + self.debug('[app] adding config value: %r', + (name, default, rebuild) + ((types,) if types else ())) if name in self.config.values: raise ExtensionError('Config value %r already present' % name) if rebuild in (False, True): rebuild = rebuild and 'env' or '' - self.config.values[name] = (default, rebuild) + self.config.values[name] = (default, rebuild, types) def add_event(self, name): self.debug('[app] adding event: %r', name) diff --git a/sphinx/config.py b/sphinx/config.py index 090875a79..4219b768f 100644 --- a/sphinx/config.py +++ b/sphinx/config.py @@ -27,10 +27,8 @@ if PY3: CONFIG_SYNTAX_ERROR += "\nDid you change the syntax from 2.x to 3.x?" CONFIG_EXIT_ERROR = "The configuration file (or one of the modules it imports) " \ "called sys.exit()" - -IGNORE_CONFIG_TYPE_CHECKS = ( - 'html_domain_indices', 'latex_domain_indices', 'texinfo_domain_indices' -) +CONFIG_TYPE_WARNING = "The config value `{name}' has type `{current.__name__}', " \ + "defaults to `{default.__name__}.'" class Config(object): @@ -50,9 +48,10 @@ class Config(object): version = ('', 'env'), release = ('', 'env'), today = ('', 'env'), - today_fmt = (None, 'env'), # the real default is locale-dependent + # the real default is locale-dependent + today_fmt = (None, 'env', [str]), - language = (None, 'env'), + language = (None, 'env', [str]), locale_dirs = ([], 'env'), master_doc = ('contents', 'env'), @@ -60,23 +59,23 @@ class Config(object): source_encoding = ('utf-8-sig', 'env'), source_parsers = ({}, 'env'), exclude_patterns = ([], 'env'), - default_role = (None, 'env'), + default_role = (None, 'env', [str]), add_function_parentheses = (True, 'env'), add_module_names = (True, 'env'), trim_footnote_reference_space = (False, 'env'), show_authors = (False, 'env'), - pygments_style = (None, 'html'), + pygments_style = (None, 'html', [str]), highlight_language = ('python', 'env'), highlight_options = ({}, 'env'), templates_path = ([], 'html'), - template_bridge = (None, 'html'), + template_bridge = (None, 'html', [str]), keep_warnings = (False, 'env'), modindex_common_prefix = ([], 'html'), - rst_epilog = (None, 'env'), - rst_prolog = (None, 'env'), + rst_epilog = (None, 'env', [str]), + rst_prolog = (None, 'env', [str]), trim_doctest_flags = (True, 'env'), primary_domain = ('py', 'env'), - needs_sphinx = (None, None), + needs_sphinx = (None, None, [str]), needs_extensions = ({}, None), nitpicky = (False, 'env'), nitpick_ignore = ([], 'html'), @@ -95,34 +94,34 @@ class Config(object): (self.project, self.release), 'html'), html_short_title = (lambda self: self.html_title, 'html'), - html_style = (None, 'html'), - html_logo = (None, 'html'), - html_favicon = (None, 'html'), + html_style = (None, 'html', [str]), + html_logo = (None, 'html', [str]), + html_favicon = (None, 'html', [str]), html_static_path = ([], 'html'), html_extra_path = ([], 'html'), # the real default is locale-dependent - html_last_updated_fmt = (None, 'html'), + html_last_updated_fmt = (None, 'html', [str]), html_use_smartypants = (True, 'html'), - html_translator_class = (None, 'html'), + html_translator_class = (None, 'html', [str]), html_sidebars = ({}, 'html'), html_additional_pages = ({}, 'html'), html_use_modindex = (True, 'html'), # deprecated - html_domain_indices = (True, 'html'), + html_domain_indices = (True, 'html', [list]), html_add_permalinks = (u'\u00B6', 'html'), html_use_index = (True, 'html'), html_split_index = (False, 'html'), html_copy_source = (True, 'html'), html_show_sourcelink = (True, 'html'), html_use_opensearch = ('', 'html'), - html_file_suffix = (None, 'html'), - html_link_suffix = (None, 'html'), + html_file_suffix = (None, 'html', [str]), + html_link_suffix = (None, 'html', [str]), html_show_copyright = (True, 'html'), html_show_sphinx = (True, 'html'), html_context = ({}, 'html'), html_output_encoding = ('utf-8', 'html'), html_compact_lists = (True, 'html'), html_secnumber_suffix = ('. ', 'html'), - html_search_language = (None, 'html'), + html_search_language = (None, 'html', [str]), html_search_options = ({}, 'html'), html_search_scorer = ('', None), html_scaled_image_link = (True, 'html'), @@ -139,17 +138,17 @@ class Config(object): # Apple help options applehelp_bundle_name = (lambda self: make_filename(self.project), 'applehelp'), - applehelp_bundle_id = (None, 'applehelp'), + applehelp_bundle_id = (None, 'applehelp', [str]), applehelp_dev_region = ('en-us', 'applehelp'), applehelp_bundle_version = ('1', 'applehelp'), - applehelp_icon = (None, 'applehelp'), + applehelp_icon = (None, 'applehelp', [str]), applehelp_kb_product = (lambda self: '%s-%s' % (make_filename(self.project), self.release), 'applehelp'), - applehelp_kb_url = (None, 'applehelp'), - applehelp_remote_url = (None, 'applehelp'), - applehelp_index_anchors = (False, 'applehelp'), - applehelp_min_term_length = (None, 'applehelp'), + applehelp_kb_url = (None, 'applehelp', [str]), + applehelp_remote_url = (None, 'applehelp', [str]), + applehelp_index_anchors = (False, 'applehelp', [str]), + applehelp_min_term_length = (None, 'applehelp', [str]), applehelp_stopwords = (lambda self: self.language or 'en', 'applehelp'), applehelp_locale = (lambda self: self.language or 'en', 'applehelp'), applehelp_title = (lambda self: self.project + ' Help', 'applehelp'), @@ -196,11 +195,11 @@ class Config(object): self.project, '', 'manual')], None), - latex_logo = (None, None), + latex_logo = (None, None, [str]), latex_appendices = ([], None), latex_use_parts = (False, None), latex_use_modindex = (True, None), # deprecated - latex_domain_indices = (True, None), + latex_domain_indices = (True, None, [list]), latex_show_urls = ('no', None), latex_show_pagerefs = (False, None), # paper_size and font_size are still separate values @@ -236,13 +235,13 @@ class Config(object): None), texinfo_appendices = ([], None), texinfo_elements = ({}, None), - texinfo_domain_indices = (True, None), + texinfo_domain_indices = (True, None, [list]), texinfo_show_urls = ('footnote', None), texinfo_no_detailmenu = (False, None), # linkcheck options linkcheck_ignore = ([], None), - linkcheck_timeout = (None, None), + linkcheck_timeout = (None, None, [int]), linkcheck_workers = (5, None), linkcheck_anchors = (True, None), @@ -292,25 +291,30 @@ class Config(object): # 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 in IGNORE_CONFIG_TYPE_CHECKS: - continue # for a while, ignore multiple types config value. see #1781 - if name not in Config.config_values: + if name not in self.values: continue # we don't know a default value - default, dummy_rebuild = Config.config_values[name] + settings = self.values[name] + default, dummy_rebuild = settings[:2] + permitted = settings[2] if len(settings) == 3 else () + if hasattr(default, '__call__'): default = default(self) # could invoke l_() - if default is None: - continue + if default is None and not permitted: + continue # neither inferrable nor expliclitly permitted types current = self[name] if type(current) is type(default): continue + if type(current) in permitted: + 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__)) + + warn(CONFIG_TYPE_WARNING.format( + name=name, current=type(current), default=type(default))) def check_unicode(self, warn): # check all string values for non-ASCII characters in bytestrings, diff --git a/tests/roots/test-config/conf.py b/tests/roots/test-config/conf.py new file mode 100644 index 000000000..34e89a52d --- /dev/null +++ b/tests/roots/test-config/conf.py @@ -0,0 +1,32 @@ +value1 = 123 # wrong type +value2 = 123 # lambda with wrong type +value3 = [] # lambda with correct type +value4 = True # child type +value5 = 3 # parent type +value6 = () # other sequence type, also raises +value7 = ['foo'] # explicitly permitted + +class A(object): + pass +class B(A): + pass +class C(A): + pass + +value8 = C() # sibling type + +# both have no default or permissible types +value9 = 'foo' +value10 = 123 + +def setup(app): + app.add_config_value('value1', 'string', False) + app.add_config_value('value2', lambda conf: [], False) + app.add_config_value('value3', [], False) + app.add_config_value('value4', 100, False) + app.add_config_value('value5', False, False) + app.add_config_value('value6', [], False) + app.add_config_value('value7', 'string', False, [list]) + app.add_config_value('value8', B(), False) + app.add_config_value('value9', None, False) + app.add_config_value('value10', None, False) diff --git a/tests/test_config.py b/tests/test_config.py index 5bf208288..1c745db70 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -9,9 +9,10 @@ :copyright: Copyright 2007-2015 by the Sphinx team, see AUTHORS. :license: BSD, see LICENSE for details. """ -from six import PY2, PY3, StringIO +from six import PY2, PY3, StringIO, iteritems -from util import TestApp, with_app, with_tempdir, raises, raises_msg +from util import TestApp, with_app, gen_with_app, with_tempdir, \ + raises, raises_msg, assert_in, assert_not_in from sphinx.config import Config from sphinx.errors import ExtensionError, ConfigError, VersionRequirementError @@ -135,36 +136,30 @@ def test_config_eol(tmpdir): 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)) +@with_app(confoverrides={'master_doc': 123, 'language': 'foo'}) +def test_builtin_conf(app, status, warning): + assert_in('master_doc', warning.getvalue(), + 'override on builtin "master_doc" should raise a type warning') + assert_not_in('language', warning.getvalue(), 'explicitly permitted ' + 'override on builtin "language" should NOT raise a type warning') -def test_gen_check_types(): - for key, value, should, deftype in TYPECHECK_OVERRIDES: - warning = StringIO() - app = TestApp(confoverrides={key: value}, warning=warning) - 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(): - warning_list = warning.getvalue() - assert (msg in warning_list) == 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 +# See roots/test-config/conf.py. +TYPECHECK_WARNINGS = { + 'value1': True, + 'value2': True, + 'value3': False, + 'value4': True, + 'value5': False, + 'value6': True, + 'value7': False, + 'value8': False, + 'value9': False, + 'value10': False, +} +@gen_with_app(testroot='config') +def test_gen_check_types(app, status, warning): + for key, should in iteritems(TYPECHECK_WARNINGS): + yield assert_in if should else assert_not_in, key, warning.getvalue(), \ + 'override on "%s" should%s raise a type warning' % \ + (key, '' if should else ' NOT') diff --git a/tests/util.py b/tests/util.py index df952d457..246b3728e 100644 --- a/tests/util.py +++ b/tests/util.py @@ -94,14 +94,15 @@ def assert_startswith(thing, prefix): assert False, '%r does not start with %r' % (thing, prefix) -def assert_in(x, thing): - if x not in thing: - assert False, '%r is not in %r' % (x, thing) - - -def assert_not_in(x, thing): - if x in thing: - assert False, '%r is in %r' % (x, thing) +try: + from nose.tools import assert_in, assert_not_in +except ImportError: + def assert_in(x, thing, msg=''): + if x not in thing: + assert False, msg or '%r is not in %r%r' % (x, thing) + def assert_not_in(x, thing, msg=''): + if x in thing: + assert False, msg or '%r is in %r%r' % (x, thing) def skip_if(condition, msg=None): From ca655f31a8be71c163296b1363bacaee6cfbc39a Mon Sep 17 00:00:00 2001 From: Robert Lehmann Date: Fri, 11 Sep 2015 09:54:07 +0200 Subject: [PATCH 2/5] Fixes #1984: Hint None being valid for primary_domain. --- sphinx/config.py | 4 ++-- sphinx/util/pycompat.py | 2 ++ tests/test_config.py | 12 +++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sphinx/config.py b/sphinx/config.py index 4219b768f..0b7ac9cad 100644 --- a/sphinx/config.py +++ b/sphinx/config.py @@ -18,7 +18,7 @@ from six import PY3, iteritems, string_types, binary_type, integer_types from sphinx.errors import ConfigError from sphinx.locale import l_ from sphinx.util.osutil import make_filename, cd -from sphinx.util.pycompat import execfile_ +from sphinx.util.pycompat import execfile_, NoneType nonascii_re = re.compile(br'[\x80-\xff]') @@ -74,7 +74,7 @@ class Config(object): rst_epilog = (None, 'env', [str]), rst_prolog = (None, 'env', [str]), trim_doctest_flags = (True, 'env'), - primary_domain = ('py', 'env'), + primary_domain = ('py', 'env', [NoneType]), needs_sphinx = (None, None, [str]), needs_extensions = ({}, None), nitpicky = (False, 'env'), diff --git a/sphinx/util/pycompat.py b/sphinx/util/pycompat.py index 062cee739..c7ea19d75 100644 --- a/sphinx/util/pycompat.py +++ b/sphinx/util/pycompat.py @@ -20,6 +20,8 @@ from itertools import product from six import PY3, text_type, exec_ +NoneType = type(None) + # ------------------------------------------------------------------------------ # Python 2/3 compatibility diff --git a/tests/test_config.py b/tests/test_config.py index 1c745db70..ee7dc3afe 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -136,12 +136,18 @@ def test_config_eol(tmpdir): assert cfg.project == u'spam' -@with_app(confoverrides={'master_doc': 123, 'language': 'foo'}) +@with_app(confoverrides={ + 'master_doc': 123, + 'language': 'foo', + 'primary_domain': None}) def test_builtin_conf(app, status, warning): - assert_in('master_doc', warning.getvalue(), + warning = warning.getvalue() + assert_in('master_doc', warnings, 'override on builtin "master_doc" should raise a type warning') - assert_not_in('language', warning.getvalue(), 'explicitly permitted ' + assert_not_in('language', warnings, 'explicitly permitted ' 'override on builtin "language" should NOT raise a type warning') + assert_not_in('primary_domain', warnings, 'override to None on builtin ' + '"primary_domain" should NOT raise a type warning') # See roots/test-config/conf.py. From 6552e97425bb8c9bebab46d4670084814f6bc085 Mon Sep 17 00:00:00 2001 From: Robert Lehmann Date: Fri, 11 Sep 2015 09:55:29 +0200 Subject: [PATCH 3/5] Hint True/False being valid for autosummary_generate. --- sphinx/ext/autosummary/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/autosummary/__init__.py b/sphinx/ext/autosummary/__init__.py index a53b125d5..359eb9673 100644 --- a/sphinx/ext/autosummary/__init__.py +++ b/sphinx/ext/autosummary/__init__.py @@ -583,5 +583,5 @@ def setup(app): app.add_role('autolink', autolink_role) app.connect('doctree-read', process_autosummary_toc) app.connect('builder-inited', process_generate_options) - app.add_config_value('autosummary_generate', [], True) + app.add_config_value('autosummary_generate', [], True, [bool]) return {'version': sphinx.__display_version__, 'parallel_read_safe': True} From fff61dd1f8167f73fc1811848d5f5c1e60bcabfa Mon Sep 17 00:00:00 2001 From: Robert Lehmann Date: Fri, 11 Sep 2015 17:10:04 +0200 Subject: [PATCH 4/5] Fix PEP8 complaint pyflake8: E128 continuation line under-indented for visual indent. --- sphinx/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/application.py b/sphinx/application.py index 9ae7c8acd..1e3435c5a 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -537,7 +537,7 @@ class Sphinx(object): def add_config_value(self, name, default, rebuild, types=()): self.debug('[app] adding config value: %r', - (name, default, rebuild) + ((types,) if types else ())) + (name, default, rebuild) + ((types,) if types else ())) if name in self.config.values: raise ExtensionError('Config value %r already present' % name) if rebuild in (False, True): From c5f412f491ba65a47901cf8fc57a400a46be29ef Mon Sep 17 00:00:00 2001 From: Robert Lehmann Date: Fri, 9 Oct 2015 08:24:17 +0200 Subject: [PATCH 5/5] Fix typo. --- tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index ee7dc3afe..803cf09f0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -141,7 +141,7 @@ def test_config_eol(tmpdir): 'language': 'foo', 'primary_domain': None}) def test_builtin_conf(app, status, warning): - warning = warning.getvalue() + warnings = warning.getvalue() assert_in('master_doc', warnings, 'override on builtin "master_doc" should raise a type warning') assert_not_in('language', warnings, 'explicitly permitted '