From 64b4d7c686e58dfc369a89285718ac988698f34b Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 2 Oct 2017 19:24:13 +0100 Subject: [PATCH 1/3] utils: Use better variable names in check_fileheader We're going to be doing some surgery on this function, so first clean it up before we do anything else. Signed-off-by: Stephen Finucane --- utils/check_sources.py | 47 +++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/utils/check_sources.py b/utils/check_sources.py index 59b380e45..2e8105487 100755 --- a/utils/check_sources.py +++ b/utils/check_sources.py @@ -98,47 +98,46 @@ def check_style(fn, lines): @checker('.py', only_pkg=True) def check_fileheader(fn, lines): # line number correction - c = 1 + offset = 1 if lines[0:1] == ['#!/usr/bin/env python\n']: lines = lines[1:] - c = 2 + offset = 2 llist = [] - docopen = False - for lno, l in enumerate(lines): - llist.append(l) + doc_open = False + for lno, line in enumerate(lines): + llist.append(line) if lno == 0: - if l != '# -*- coding: utf-8 -*-\n': + if line != '# -*- coding: utf-8 -*-\n': yield 1, "missing coding declaration" elif lno == 1: - if l != '"""\n' and l != 'r"""\n': + if line != '"""\n' and line != 'r"""\n': yield 2, 'missing docstring begin (""")' else: - docopen = True - elif docopen: - if l == '"""\n': + doc_open = True + elif doc_open: + if line == '"""\n': # end of docstring if lno <= 4: - yield lno + c, "missing module name in docstring" + yield lno + offset, "missing module name in docstring" break - if l != '\n' and l[:4] != ' ' and docopen: - yield lno + c, "missing correct docstring indentation" + if line != '\n' and line[:4] != ' ' and doc_open: + yield lno + offset, "missing correct docstring indentation" if lno == 2: # if not in package, don't check the module name - modname = fn[:-3].replace('/', '.').replace('.__init__', '') - while modname: - if l.lower()[4:-1] == modname: + mod_name = fn[:-3].replace('/', '.').replace('.__init__', '') + while mod_name: + if line.lower()[4:-1] == mod_name: break - modname = '.'.join(modname.split('.')[1:]) + mod_name = '.'.join(mod_name.split('.')[1:]) else: yield 3, "wrong module name in docstring heading" - modnamelen = len(l.strip()) + mod_name_len = len(line.strip()) elif lno == 3: - if l.strip() != modnamelen * '~': + if line.strip() != mod_name_len * '~': yield 4, "wrong module name underline, should be ~~~...~" - else: yield 0, "missing end and/or start of docstring..." @@ -147,11 +146,11 @@ def check_fileheader(fn, lines): if not license or not license_re.match(license[0]): yield 0, "no correct license info" - ci = -3 - copyright = llist[ci:ci + 1] + offset = -3 + copyright = llist[offset:offset + 1] while copyright and copyright_2_re.match(copyright[0]): - ci -= 1 - copyright = llist[ci:ci + 1] + offset -= 1 + copyright = llist[offset:offset + 1] if not copyright or not copyright_re.match(copyright[0]): yield 0, "no correct copyright info" From f5c0d646589ba0da03c4afc1487b0d28751fe774 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 1 Oct 2017 21:57:23 +0100 Subject: [PATCH 2/3] utils: Move "header check" to a flake8 plugin If we want to check style, we run 'tox -e flake8': it shouldn't be necessary to run some obscure 'make' command too. Make this possible by moving the sole useful test from the target of this make command to a flake8 plugin. This includes a fix for a header that was previously excluded from checks, but is now included. Signed-off-by: Stephen Finucane --- setup.cfg | 2 +- setup.py | 7 +++ utils/__init__.py | 0 utils/check_sources.py | 67 ------------------------- utils/checks.py | 111 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 68 deletions(-) create mode 100644 utils/__init__.py create mode 100644 utils/checks.py diff --git a/setup.cfg b/setup.cfg index 517e00702..48045a292 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,7 +26,7 @@ universal = 1 [flake8] max-line-length = 95 ignore = E116,E241,E251 -exclude = .git,.tox,.venv,tests/*,build/*,sphinx/search/*,sphinx/pycode/pgen2/*,doc/ext/example*.py +exclude = .git,.tox,.venv,tests/*,build/*,doc/_build/*,sphinx/search/*,sphinx/pycode/pgen2/*,doc/ext/example*.py [mypy] python_version = 2.7 diff --git a/setup.py b/setup.py index c8c300667..af21f3938 100644 --- a/setup.py +++ b/setup.py @@ -247,6 +247,13 @@ setup( 'distutils.commands': [ 'build_sphinx = sphinx.setup_command:BuildDoc', ], + # consider moving this to 'flake8:local-plugins' once flake8 3.5.0 is + # in the wild: + # http://flake8.pycqa.org/en/latest/user/configuration.html\ + # #using-local-plugins + 'flake8.extension': [ + 'X101 = utils.checks:sphinx_has_header', + ], }, install_requires=requires, extras_require=extras_require, diff --git a/utils/__init__.py b/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/utils/check_sources.py b/utils/check_sources.py index 2e8105487..ace691449 100755 --- a/utils/check_sources.py +++ b/utils/check_sources.py @@ -37,13 +37,6 @@ def checker(*suffixes, **kwds): coding_re = re.compile(br'coding[:=]\s*([-\w.]+)') uni_coding_re = re.compile(r'^#.*coding[:=]\s*([-\w.]+).*') -name_mail_re = r'[\w ]+(<.*?>)?' -copyright_re = re.compile(r'^ :copyright: Copyright 200\d(-20\d\d)? ' - r'by %s(, %s)*[,.]$' % - (name_mail_re, name_mail_re)) -license_re = re.compile(r" :license: (.*?).\n") -copyright_2_re = re.compile(r'^ %s(, %s)*[,.]$' % - (name_mail_re, name_mail_re)) not_ix_re = re.compile(r'\bnot\s+\S+?\s+i[sn]\s\S+') is_const_re = re.compile(r'if.*?==\s+(None|False|True)\b') noqa_re = re.compile(r'#\s+NOQA\s*$', re.I) @@ -95,66 +88,6 @@ def check_style(fn, lines): yield lno + 1, 'using == None/True/False' -@checker('.py', only_pkg=True) -def check_fileheader(fn, lines): - # line number correction - offset = 1 - if lines[0:1] == ['#!/usr/bin/env python\n']: - lines = lines[1:] - offset = 2 - - llist = [] - doc_open = False - for lno, line in enumerate(lines): - llist.append(line) - if lno == 0: - if line != '# -*- coding: utf-8 -*-\n': - yield 1, "missing coding declaration" - elif lno == 1: - if line != '"""\n' and line != 'r"""\n': - yield 2, 'missing docstring begin (""")' - else: - doc_open = True - elif doc_open: - if line == '"""\n': - # end of docstring - if lno <= 4: - yield lno + offset, "missing module name in docstring" - break - - if line != '\n' and line[:4] != ' ' and doc_open: - yield lno + offset, "missing correct docstring indentation" - - if lno == 2: - # if not in package, don't check the module name - mod_name = fn[:-3].replace('/', '.').replace('.__init__', '') - while mod_name: - if line.lower()[4:-1] == mod_name: - break - mod_name = '.'.join(mod_name.split('.')[1:]) - else: - yield 3, "wrong module name in docstring heading" - mod_name_len = len(line.strip()) - elif lno == 3: - if line.strip() != mod_name_len * '~': - yield 4, "wrong module name underline, should be ~~~...~" - else: - yield 0, "missing end and/or start of docstring..." - - # check for copyright and license fields - license = llist[-2:-1] - if not license or not license_re.match(license[0]): - yield 0, "no correct license info" - - offset = -3 - copyright = llist[offset:offset + 1] - while copyright and copyright_2_re.match(copyright[0]): - offset -= 1 - copyright = llist[offset:offset + 1] - if not copyright or not copyright_re.match(copyright[0]): - yield 0, "no correct copyright info" - - @checker('.py', '.html', '.rst') def check_whitespace_and_spelling(fn, lines): for lno, line in enumerate(lines): diff --git a/utils/checks.py b/utils/checks.py new file mode 100644 index 000000000..03104d78a --- /dev/null +++ b/utils/checks.py @@ -0,0 +1,111 @@ +# -*- coding: utf-8 -*- +""" + utils.checks + ~~~~~~~~~~~~ + + Custom, Sphinx-only flake8 plugins. + + :copyright: Copyright 2007-2017 by the Sphinx team, see AUTHORS. + :license: BSD, see LICENSE for details. +""" + +import os +import re +import sphinx + +name_mail_re = r'[\w ]+(<.*?>)?' +copyright_re = re.compile(r'^ :copyright: Copyright 200\d(-20\d\d)? ' + r'by %s(, %s)*[,.]$' % (name_mail_re, name_mail_re)) +copyright_2_re = re.compile(r'^ %s(, %s)*[,.]$' % + (name_mail_re, name_mail_re)) +license_re = re.compile(r' :license: (.*?).\n') + + +def flake8ext(_func): + """Decorate flake8_asserts functions""" + _func.name = _func.__name__ + _func.version = sphinx.__version__ + _func.code = _func.__name__.upper() + + return _func + + +@flake8ext +def sphinx_has_header(physical_line, filename, lines, line_number): + """Check for correct headers. + + Make sure each Python file has a correct file header including + copyright and license information. + + X101 invalid header found + """ + # we have a state machine of sorts so we need to start on line 1. Also, + # there's no point checking really short files + if line_number != 1 or len(lines) < 10: + return + + # this file uses a funky license but unfortunately it's not possible to + # ignore specific errors on a file-level basis yet [1]. Simply skip it. + # + # [1] https://gitlab.com/pycqa/flake8/issues/347 + if os.path.samefile(filename, './sphinx/util/smartypants.py'): + return + + # if the top-level package or not inside the package, ignore + mod_name = os.path.splitext(filename)[0].strip('./\\').replace( + '/', '.').replace('.__init__', '') + if mod_name == 'sphinx' or not mod_name.startswith('sphinx.'): + return + + # line number correction + offset = 1 + if lines[0:1] == ['#!/usr/bin/env python\n']: + lines = lines[1:] + offset = 2 + + llist = [] + doc_open = False + + for lno, line in enumerate(lines): + llist.append(line) + if lno == 0: + if line != '# -*- coding: utf-8 -*-\n': + return 0, 'X101 missing coding declaration' + elif lno == 1: + if line != '"""\n' and line != 'r"""\n': + return 0, 'X101 missing docstring begin (""")' + else: + doc_open = True + elif doc_open: + if line == '"""\n': + # end of docstring + if lno <= 4: + return 0, 'X101 missing module name in docstring' + break + + if line != '\n' and line[:4] != ' ' and doc_open: + return 0, 'X101 missing correct docstring indentation' + + if lno == 2: + mod_name_len = len(line.strip()) + if line.strip() != mod_name: + return 4, 'X101 wrong module name in docstring heading' + elif lno == 3: + if line.strip() != mod_name_len * '~': + return (4, 'X101 wrong module name underline, should be ' + '~~~...~') + else: + return 0, 'X101 missing end and/or start of docstring...' + + # check for copyright and license fields + license = llist[-2:-1] + if not license or not license_re.match(license[0]): + return 0, 'X101 no correct license info' + + offset = -3 + copyright = llist[offset:offset + 1] + while copyright and copyright_2_re.match(copyright[0]): + offset -= 1 + copyright = llist[offset:offset + 1] + if not copyright or not copyright_re.match(copyright[0]): + return 0, 'X101 no correct copyright info' From 160e27e20f059a8dffad9e6454ea6763a51feb70 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Sun, 1 Oct 2017 16:26:40 +0100 Subject: [PATCH 3/3] utils: Remove 'check_sources' There are still a couple of checks here but all of them can be removed now: - Check if using valid Python syntax - Check if line length too long - Check if using 'x == None/True/False' - Check if using old HTML 3 tags The first three are already handled by default by the 'flake8' tool. The last one isn't replaced by anything, but it really isn't worth worrying about now because the tags it checks for have been dead for a really long time and would not be used by anyone writing HTML in the last 10 years. Combined, it means we can remove the entire file. The 'style-check' target is updated to simply alias 'flake8'. It can be removed in a future release. This allows us to stop using this target in the Travis jobs, seeing as we already run flake8. Signed-off-by: Stephen Finucane --- .travis.yml | 2 +- Makefile | 34 +------- utils/check_sources.py | 191 ----------------------------------------- 3 files changed, 2 insertions(+), 225 deletions(-) delete mode 100755 utils/check_sources.py diff --git a/.travis.yml b/.travis.yml index f271fd4a0..6df4f7cfb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -44,5 +44,5 @@ install: - if [[ $TRAVIS_PYTHON_VERSION == '3.6' ]]; then python3.6 -m pip install mypy typed-ast; fi script: - flake8 - - if [[ $TRAVIS_PYTHON_VERSION == '3.6' ]]; then make style-check type-check test-async; fi + - if [[ $TRAVIS_PYTHON_VERSION == '3.6' ]]; then make type-check test-async; fi - if [[ $TRAVIS_PYTHON_VERSION != '3.6' ]]; then make test; fi diff --git a/Makefile b/Makefile index a20df8f39..ce5bf096a 100644 --- a/Makefile +++ b/Makefile @@ -3,42 +3,10 @@ PYTHON ?= python .PHONY: all style-check type-check clean clean-pyc clean-patchfiles clean-backupfiles \ clean-generated pylint reindent test covertest build -DONT_CHECK = -i .ropeproject \ - -i .tox \ - -i build \ - -i dist \ - -i doc/_build \ - -i sphinx/pycode/pgen2 \ - -i sphinx/search/da.py \ - -i sphinx/search/de.py \ - -i sphinx/search/en.py \ - -i sphinx/search/es.py \ - -i sphinx/search/fi.py \ - -i sphinx/search/fr.py \ - -i sphinx/search/hu.py \ - -i sphinx/search/it.py \ - -i sphinx/search/ja.py \ - -i sphinx/search/nl.py \ - -i sphinx/search/no.py \ - -i sphinx/search/pt.py \ - -i sphinx/search/ro.py \ - -i sphinx/search/ru.py \ - -i sphinx/search/sv.py \ - -i sphinx/search/tr.py \ - -i sphinx/style/jquery.js \ - -i sphinx/util/smartypants.py \ - -i tests/build \ - -i tests/path.py \ - -i tests/roots/test-directive-code/target.py \ - -i tests/roots/test-warnings/undecodable.rst \ - -i tests/test_autodoc_py35.py \ - -i tests/typing_test_data.py \ - -i utils/convert.py - all: clean-pyc clean-backupfiles style-check type-check test style-check: - @PYTHONWARNINGS=all $(PYTHON) utils/check_sources.py $(DONT_CHECK) . + @flake8 type-check: mypy sphinx/ diff --git a/utils/check_sources.py b/utils/check_sources.py deleted file mode 100755 index ace691449..000000000 --- a/utils/check_sources.py +++ /dev/null @@ -1,191 +0,0 @@ -#!/usr/bin/env python -# -*- coding: utf-8 -*- -""" - Checker for file headers - ~~~~~~~~~~~~~~~~~~~~~~~~ - - Make sure each Python file has a correct file header - including copyright and license information. - - :copyright: Copyright 2007-2017 by the Sphinx team, see AUTHORS. - :license: BSD, see LICENSE for details. -""" -from __future__ import print_function - -import os -import re -import sys -from optparse import OptionParser -from os.path import join, splitext, abspath - - -checkers = {} - - -def checker(*suffixes, **kwds): - only_pkg = kwds.pop('only_pkg', False) - - def deco(func): - for suffix in suffixes: - checkers.setdefault(suffix, []).append(func) - func.only_pkg = only_pkg - return func - return deco - - -# this one is a byte regex since it is applied before decoding -coding_re = re.compile(br'coding[:=]\s*([-\w.]+)') - -uni_coding_re = re.compile(r'^#.*coding[:=]\s*([-\w.]+).*') -not_ix_re = re.compile(r'\bnot\s+\S+?\s+i[sn]\s\S+') -is_const_re = re.compile(r'if.*?==\s+(None|False|True)\b') -noqa_re = re.compile(r'#\s+NOQA\s*$', re.I) - -misspellings = ["developement", "adress", # ALLOW-MISSPELLING - "verificate", "informations"] # ALLOW-MISSPELLING - - -def decode_source(fn, lines): - encoding = 'ascii' if fn.endswith('.py') else 'utf-8' - decoded_lines = [] - for lno, line in enumerate(lines): - if lno < 2: - co = coding_re.search(line) - if co: - encoding = co.group(1).decode() - try: - decoded_lines.append(line.decode(encoding)) - except UnicodeDecodeError as err: - raise UnicodeError("%s:%d: not decodable: %s\n Line: %r" % - (fn, lno + 1, err, line)) - except LookupError as err: - raise LookupError("unknown encoding: %s" % encoding) - return decoded_lines - - -@checker('.py') -def check_syntax(fn, lines): - lines = [uni_coding_re.sub('', line) for line in lines] - try: - compile(''.join(lines), fn, "exec") - except SyntaxError as err: - yield 0, "not compilable: %s" % err - - -@checker('.py') -def check_style(fn, lines): - for lno, line in enumerate(lines): - if noqa_re.search(line): - continue - if len(line.rstrip('\n')) > 95: - yield lno + 1, "line too long" - if line.strip().startswith('#'): - continue - # m = not_ix_re.search(line) - # if m: - # yield lno+1, '"' + m.group() + '"' - if is_const_re.search(line): - yield lno + 1, 'using == None/True/False' - - -@checker('.py', '.html', '.rst') -def check_whitespace_and_spelling(fn, lines): - for lno, line in enumerate(lines): - if '\t' in line: - yield lno + 1, "OMG TABS!!!1 " - if line[:-1].rstrip(' \t') != line[:-1]: - yield lno + 1, "trailing whitespace" - for word in misspellings: - if word in line and 'ALLOW-MISSPELLING' not in line: - yield lno + 1, '"%s" used' % word - - -bad_tags = ['', '', '', '
', ' 1 and "s" or "")) - return int(num > 0) - - -if __name__ == '__main__': - sys.exit(main(sys.argv[1:]))