Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

lint: Combine functions in custom_rules into RuleList class. 

This makes linting rules in zulint more general. Make necessary
changes in tools/lint and tools/custom_check.py to run with the new
RuleList class.

Modify tests for `RuleList` class. Tests only include minor changes to
test with the new class.
This commit is contained in:
Aman Agrawal 2019-05-30 21:11:23 +05:30 committed by Tim Abbott
parent fd25ced43c
commit db25c0c2ca
4 changed files with 507 additions and 493 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
tools/lint
View File

@ -10,6 +10,7 @@ import argparse
from lib import sanity_check from lib import sanity_check
sanity_check.check_venv(__file__) sanity_check.check_venv(__file__)
from linter_lib.custom_check import python_rules, non_py_rules
from zulint import lister from zulint import lister
from zulint.command import add_default_linter_arguments, LinterConfig from zulint.command import add_default_linter_arguments, LinterConfig
from typing import cast, Dict, List from typing import cast, Dict, List
@ -44,7 +45,6 @@ def run():
root_dir = os.path.dirname(tools_dir) root_dir = os.path.dirname(tools_dir)
sys.path.insert(0, root_dir) sys.path.insert(0, root_dir)
from tools.linter_lib.custom_check import build_custom_checkers
from tools.linter_lib.exclude import EXCLUDED_FILES, PUPPET_CHECK_RULES_TO_EXCLUDE from tools.linter_lib.exclude import EXCLUDED_FILES, PUPPET_CHECK_RULES_TO_EXCLUDE
from tools.linter_lib.pyflakes import check_pyflakes from tools.linter_lib.pyflakes import check_pyflakes
from tools.linter_lib.pep8 import check_pep8 from tools.linter_lib.pep8 import check_pep8
@ -85,8 +85,6 @@ def run():
else: else:
logger.setLevel(logging.WARNING) logger.setLevel(logging.WARNING)
check_custom_checks_py, check_custom_checks_nonpy = build_custom_checkers(by_lang)
linter_config = LinterConfig(by_lang) linter_config = LinterConfig(by_lang)
linter_config.external_linter('add_class', ['tools/find-add-class'], ['js']) linter_config.external_linter('add_class', ['tools/find-add-class'], ['js'])
linter_config.external_linter('css', ['node', 'node_modules/.bin/stylelint'], ['css', 'scss']) linter_config.external_linter('css', ['node', 'node_modules/.bin/stylelint'], ['css', 'scss'])
@ -111,13 +109,15 @@ def run():
@linter_config.lint @linter_config.lint
def custom_py(): def custom_py():
# type: () -> int # type: () -> int
failed = check_custom_checks_py() failed = python_rules.check(by_lang)
return 1 if failed else 0 return 1 if failed else 0
@linter_config.lint @linter_config.lint
def custom_nonpy(): def custom_nonpy():
# type: () -> int # type: () -> int
failed = check_custom_checks_nonpy() failed = False
for rule in non_py_rules:
failed = failed or rule.check(by_lang)
return 1 if failed else 0 return 1 if failed else 0
@linter_config.lint @linter_config.lint

279
tools/linter_lib/custom_check.py
View File

@ -3,17 +3,31 @@
from __future__ import print_function from __future__ import print_function
from __future__ import absolute_import from __future__ import absolute_import
from zulint.printer import colors from zulint.custom_rules import RuleList
from zulint.custom_rules import (
custom_check_file,
)
from typing import cast, Any, Callable, Dict, List, Tuple from typing import cast, Any, Dict, List, Tuple
Rule = Dict[str, Any] Rule = List[Dict[str, Any]]
RuleList = List[Dict[str, Any]] # Rule help:
# By default, a rule applies to all files within the extension for which it is specified (e.g. all .py files)
# There are three operators we can use to manually include or exclude files from linting for a rule:
# 'exclude': 'set([<path>, ...])' - if <path> is a filename, excludes that file.
# if <path> is a directory, excludes all files directly below the directory <path>.
# 'exclude_line': 'set([(<path>, <line>), ...])' - excludes all lines matching <line> in the file <path> from linting.
# 'include_only': 'set([<path>, ...])' - includes only those files where <path> is a substring of the filepath.
LineTup = Tuple[int, str, str, str] LineTup = Tuple[int, str, str, str]
PYDELIMS = r'''"'()\[\]{}#\\'''
PYREG = r"[^{}]".format(PYDELIMS)
PYSQ = r'"(?:[^"\\]|\\.)*"'
PYDQ = r"'(?:[^'\\]|\\.)*'"
PYLEFT = r"[(\[{]"
PYRIGHT = r"[)\]}]"
PYCODE = PYREG
for depth in range(5):
PYGROUP = r"""(?:{}|{}|{}{}*{})""".format(PYSQ, PYDQ, PYLEFT, PYCODE, PYRIGHT)
PYCODE = r"""(?:{}|{})""".format(PYREG, PYGROUP)
FILES_WITH_LEGACY_SUBJECT = { FILES_WITH_LEGACY_SUBJECT = {
# This basically requires a big DB migration: # This basically requires a big DB migration:
'zerver/lib/topic.py', 'zerver/lib/topic.py',
@ -40,26 +54,6 @@ FILES_WITH_LEGACY_SUBJECT = {
'zerver/tests/test_narrow.py', 'zerver/tests/test_narrow.py',
} }
PYDELIMS = r'''"'()\[\]{}#\\'''
PYREG = r"[^{}]".format(PYDELIMS)
PYSQ = r'"(?:[^"\\]|\\.)*"'
PYDQ = r"'(?:[^'\\]|\\.)*'"
PYLEFT = r"[(\[{]"
PYRIGHT = r"[)\]}]"
PYCODE = PYREG
for depth in range(5):
PYGROUP = r"""(?:{}|{}|{}{}*{})""".format(PYSQ, PYDQ, PYLEFT, PYCODE, PYRIGHT)
PYCODE = r"""(?:{}|{})""".format(PYREG, PYGROUP)
def build_custom_checkers(by_lang):
# type: (Dict[str, List[str]]) -> Tuple[Callable[[], bool], Callable[[], bool]]
# By default, a rule applies to all files within the extension for which it is specified (e.g. all .py files)
# There are three operators we can use to manually include or exclude files from linting for a rule:
# 'exclude': 'set([<path>, ...])' - if <path> is a filename, excludes that file.
# if <path> is a directory, excludes all files directly below the directory <path>.
# 'exclude_line': 'set([(<path>, <line>), ...])' - excludes all lines matching <line> in the file <path> from linting.
# 'include_only': 'set([<path>, ...])' - includes only those files where <path> is a substring of the filepath.
trailing_whitespace_rule = { trailing_whitespace_rule = {
'pattern': r'\s+$', 'pattern': r'\s+$',
'strip': '\n', 'strip': '\n',
@ -75,14 +69,14 @@ def build_custom_checkers(by_lang):
'strip': '\n', 'strip': '\n',
'exclude': set(['tools/ci/success-http-headers.txt']), 'exclude': set(['tools/ci/success-http-headers.txt']),
'description': 'Fix tab-based whitespace'}, 'description': 'Fix tab-based whitespace'},
] # type: RuleList ] # type: Rule
comma_whitespace_rule = [ comma_whitespace_rule = [
{'pattern': ', {2,}[^#/ ]', {'pattern': ', {2,}[^#/ ]',
'exclude': set(['zerver/tests', 'frontend_tests/node_tests', 'corporate/tests']), 'exclude': set(['zerver/tests', 'frontend_tests/node_tests', 'corporate/tests']),
'description': "Remove multiple whitespaces after ','", 'description': "Remove multiple whitespaces after ','",
'good_lines': ['foo(1, 2, 3)', 'foo = bar # some inline comment'], 'good_lines': ['foo(1, 2, 3)', 'foo = bar # some inline comment'],
'bad_lines': ['foo(1, 2, 3)', 'foo(1, 2, 3)']}, 'bad_lines': ['foo(1, 2, 3)', 'foo(1, 2, 3)']},
] # type: RuleList ] # type: Rule
markdown_whitespace_rules = list([rule for rule in whitespace_rules if rule['pattern'] != r'\s+$']) + [ markdown_whitespace_rules = list([rule for rule in whitespace_rules if rule['pattern'] != r'\s+$']) + [
# Two spaces trailing a line with other content is okay--it's a markdown line break. # Two spaces trailing a line with other content is okay--it's a markdown line break.
# This rule finds one space trailing a non-space, three or more trailing spaces, and # This rule finds one space trailing a non-space, three or more trailing spaces, and
@ -95,8 +89,12 @@ def build_custom_checkers(by_lang):
'description': 'Missing space after # in heading', 'description': 'Missing space after # in heading',
'good_lines': ['### some heading', '# another heading'], 'good_lines': ['### some heading', '# another heading'],
'bad_lines': ['###some heading', '#another heading']}, 'bad_lines': ['###some heading', '#another heading']},
] # type: RuleList ] # type: Rule
js_rules = cast(RuleList, [
js_rules = RuleList(
langs=['js'],
rules=cast(Rule, [
{'pattern': 'subject|SUBJECT', {'pattern': 'subject|SUBJECT',
'exclude': set(['static/js/util.js', 'exclude': set(['static/js/util.js',
'frontend_tests/']), 'frontend_tests/']),
@ -202,8 +200,12 @@ def build_custom_checkers(by_lang):
]), ]),
'good_lines': ['#my-style {color: blue;}'], 'good_lines': ['#my-style {color: blue;}'],
'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']}, 'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']},
]) + whitespace_rules + comma_whitespace_rule ]) + whitespace_rules + comma_whitespace_rule,
python_rules = cast(RuleList, [ )
python_rules = RuleList(
langs=['py'],
rules=cast(Rule, [
{'pattern': 'subject|SUBJECT', {'pattern': 'subject|SUBJECT',
'exclude_pattern': 'subject to the|email|outbox', 'exclude_pattern': 'subject to the|email|outbox',
'description': 'avoid subject as a var', 'description': 'avoid subject as a var',
@ -501,8 +503,13 @@ def build_custom_checkers(by_lang):
'include_only': set(["/management/commands/"]), 'include_only': set(["/management/commands/"]),
'description': 'Raise CommandError to exit with failure in management commands', 'description': 'Raise CommandError to exit with failure in management commands',
}, },
]) + whitespace_rules + comma_whitespace_rule ]) + whitespace_rules + comma_whitespace_rule,
bash_rules = cast(RuleList, [ max_length=110,
)
bash_rules = RuleList(
langs=['bash'],
rules=cast(Rule, [
{'pattern': '#!.*sh [-xe]', {'pattern': '#!.*sh [-xe]',
'description': 'Fix shebang line with proper call to /usr/bin/env for Bash path, change -x|-e switches' 'description': 'Fix shebang line with proper call to /usr/bin/env for Bash path, change -x|-e switches'
' to set -x|set -e'}, ' to set -x|set -e'},
@ -513,8 +520,12 @@ def build_custom_checkers(by_lang):
'scripts/lib/install', 'scripts/lib/install',
'scripts/setup/configure-rabbitmq' 'scripts/setup/configure-rabbitmq'
]), }, ]), },
]) + whitespace_rules[0:1] ]) + whitespace_rules[0:1],
css_rules = cast(RuleList, [ )
css_rules = RuleList(
langs=['css', 'scss'],
rules=cast(Rule, [
{'pattern': r'calc\([^+]+\+[^+]+\)', {'pattern': r'calc\([^+]+\+[^+]+\)',
'description': "Avoid using calc with '+' operator. See #8403 : in CSS.", 'description': "Avoid using calc with '+' operator. See #8403 : in CSS.",
'good_lines': ["width: calc(20% - -14px);"], 'good_lines': ["width: calc(20% - -14px);"],
@ -553,7 +564,9 @@ def build_custom_checkers(by_lang):
'good_lines': ["border-width: 5px;"], 'good_lines': ["border-width: 5px;"],
'bad_lines': ["border-width: thick;", "border: thick solid black;"]}, 'bad_lines': ["border-width: thick;", "border: thick solid black;"]},
]) + whitespace_rules + comma_whitespace_rule ]) + whitespace_rules + comma_whitespace_rule
prose_style_rules = cast(RuleList, [ )
prose_style_rules = cast(Rule, [
{'pattern': r'[^\/\#\-"]([jJ]avascript)', # exclude usage in hrefs/divs {'pattern': r'[^\/\#\-"]([jJ]avascript)', # exclude usage in hrefs/divs
'description': "javascript should be spelled JavaScript"}, 'description': "javascript should be spelled JavaScript"},
{'pattern': r'''[^\/\-\."'\_\=\>]([gG]ithub)[^\.\-\_"\<]''', # exclude usage in hrefs/divs {'pattern': r'''[^\/\-\."'\_\=\>]([gG]ithub)[^\.\-\_"\<]''', # exclude usage in hrefs/divs
@ -568,7 +581,7 @@ def build_custom_checkers(by_lang):
{'pattern': '[^-_]botserver(?!rc)|bot server', {'pattern': '[^-_]botserver(?!rc)|bot server',
'description': "Use Botserver instead of botserver or bot server."}, 'description': "Use Botserver instead of botserver or bot server."},
]) + comma_whitespace_rule ]) + comma_whitespace_rule
html_rules = whitespace_rules + prose_style_rules + [ html_rules = whitespace_rules + prose_style_rules + cast(Rule, [
{'pattern': 'subject|SUBJECT', {'pattern': 'subject|SUBJECT',
'exclude': set(['templates/zerver/email.html']), 'exclude': set(['templates/zerver/email.html']),
'exclude_pattern': 'email subject', 'exclude_pattern': 'email subject',
@ -603,8 +616,7 @@ def build_custom_checkers(by_lang):
{'pattern': "title='[^{]", {'pattern': "title='[^{]",
'description': "`title` value should be translatable.", 'description': "`title` value should be translatable.",
'good_lines': ['<link rel="author" title="{{ _(\'About these documents\') }}" />'], 'good_lines': ['<link rel="author" title="{{ _(\'About these documents\') }}" />'],
'bad_lines': ["<p title='foo'></p>"], 'bad_lines': ["<p title='foo'></p>"]},
},
{'pattern': r'title="[^{\:]', {'pattern': r'title="[^{\:]',
'exclude_line': set([ 'exclude_line': set([
('templates/zerver/app/markdown_help.html', ('templates/zerver/app/markdown_help.html',
@ -691,8 +703,11 @@ def build_custom_checkers(by_lang):
]), ]),
'good_lines': ['#my-style {color: blue;}', 'style="display: none"', "style='display: none"], 'good_lines': ['#my-style {color: blue;}', 'style="display: none"', "style='display: none"],
'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']}, 'bad_lines': ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"']},
] # type: RuleList ])
handlebars_rules = html_rules + [
handlebars_rules = RuleList(
langs=['handlebars'],
rules=html_rules + cast(Rule, [
{'pattern': "[<]script", {'pattern': "[<]script",
'description': "Do not use inline <script> tags here; put JavaScript in static/js instead."}, 'description': "Do not use inline <script> tags here; put JavaScript in static/js instead."},
{'pattern': '{{ t ("|\')', {'pattern': '{{ t ("|\')',
@ -709,14 +724,22 @@ def build_custom_checkers(by_lang):
'description': 'Translatable strings should not have trailing spaces.'}, 'description': 'Translatable strings should not have trailing spaces.'},
{'pattern': '{{t "[^"]+ " }}', {'pattern': '{{t "[^"]+ " }}',
'description': 'Translatable strings should not have trailing spaces.'}, 'description': 'Translatable strings should not have trailing spaces.'},
] ]),
jinja2_rules = html_rules + [ )
jinja2_rules = RuleList(
langs=['html'],
rules=html_rules + cast(Rule, [
{'pattern': r"{% endtrans %}[\.\?!]", {'pattern': r"{% endtrans %}[\.\?!]",
'description': "Period should be part of the translatable string."}, 'description': "Period should be part of the translatable string."},
{'pattern': r"{{ _(.+) }}[\.\?!]", {'pattern': r"{{ _(.+) }}[\.\?!]",
'description': "Period should be part of the translatable string."}, 'description': "Period should be part of the translatable string."},
] ]),
json_rules = [ )
json_rules = RuleList(
langs=['json'],
rules=cast(Rule, [
# Here, we don't use `whitespace_rules`, because the tab-based # Here, we don't use `whitespace_rules`, because the tab-based
# whitespace rule flags a lot of third-party JSON fixtures # whitespace rule flags a lot of third-party JSON fixtures
# under zerver/webhooks that we want preserved verbatim. So # under zerver/webhooks that we want preserved verbatim. So
@ -732,85 +755,9 @@ def build_custom_checkers(by_lang):
{'pattern': r'":["\[\{]', {'pattern': r'":["\[\{]',
'exclude': set(['zerver/webhooks/', 'zerver/tests/fixtures/']), 'exclude': set(['zerver/webhooks/', 'zerver/tests/fixtures/']),
'description': 'Require space after : in JSON'}, 'description': 'Require space after : in JSON'},
] # type: RuleList ])
markdown_rules = markdown_whitespace_rules + prose_style_rules + [ )
{'pattern': r'\[(?P<url>[^\]]+)\]\((?P=url)\)',
'description': 'Linkified markdown URLs should use cleaner <http://example.com> syntax.'},
{'pattern': 'https://zulip.readthedocs.io/en/latest/[a-zA-Z0-9]',
'exclude': ['docs/overview/contributing.md', 'docs/overview/readme.md', 'docs/README.md'],
'exclude_line': set([
('docs/testing/mypy.md',
'# See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts')
]),
'include_only': set(['docs/']),
'description': "Use relative links (../foo/bar.html) to other documents in docs/",
},
{'pattern': "su zulip -c [^']",
'include_only': set(['docs/']),
'description': "Always quote arguments using `su zulip -c '` to avoid confusion about how su works.",
},
{'pattern': r'\][(][^#h]',
'include_only': set(['README.md', 'CONTRIBUTING.md']),
'description': "Use absolute links from docs served by GitHub",
},
]
help_markdown_rules = markdown_rules + [
{'pattern': '[a-z][.][A-Z]',
'description': "Likely missing space after end of sentence"},
{'pattern': r'\b[rR]ealm[s]?\b',
'good_lines': ['Organization', 'deactivate_realm', 'realm_filter'],
'bad_lines': ['Users are in a realm', 'Realm is the best model'],
'description': "Realms are referred to as Organizations in user-facing docs."},
]
txt_rules = whitespace_rules
def check_custom_checks_py():
# type: () -> bool
failed = False
color = next(colors)
for fn in by_lang['py']:
if 'custom_check.py' in fn:
continue
if custom_check_file(fn, 'py', python_rules, color, max_length=110):
failed = True
return failed
def check_custom_checks_nonpy():
# type: () -> bool
failed = False
color = next(colors)
for fn in by_lang['js']:
if custom_check_file(fn, 'js', js_rules, color):
failed = True
color = next(colors)
for fn in by_lang['sh']:
if custom_check_file(fn, 'sh', bash_rules, color):
failed = True
color = next(colors)
for fn in by_lang['scss']:
if custom_check_file(fn, 'scss', css_rules, color):
failed = True
color = next(colors)
for fn in by_lang['handlebars']:
if custom_check_file(fn, 'handlebars', handlebars_rules, color):
failed = True
color = next(colors)
for fn in by_lang['html']:
if custom_check_file(fn, 'html', jinja2_rules, color):
failed = True
color = next(colors)
for fn in by_lang['json']:
if custom_check_file(fn, 'json', json_rules, color):
failed = True
color = next(colors)
markdown_docs_length_exclude = { markdown_docs_length_exclude = {
# Has some example Vagrant output that's very long # Has some example Vagrant output that's very long
"docs/development/setup-vagrant.md", "docs/development/setup-vagrant.md",
@ -830,31 +777,63 @@ def build_custom_checkers(by_lang):
"README.md", "README.md",
"docs/overview/readme.md", "docs/overview/readme.md",
} }
for fn in by_lang['md']:
max_length = None
if fn not in markdown_docs_length_exclude:
max_length = 120
rules = markdown_rules
if fn.startswith("templates/zerver/help"):
rules = help_markdown_rules
if custom_check_file(fn, 'md', rules, color, max_length=max_length):
failed = True
color = next(colors) markdown_rules = RuleList(
for fn in by_lang['txt'] + by_lang['text']: langs=['md'],
if custom_check_file(fn, 'txt', txt_rules, color): rules=markdown_whitespace_rules + prose_style_rules + cast(Rule, [
failed = True {'pattern': r'\[(?P<url>[^\]]+)\]\((?P=url)\)',
'description': 'Linkified markdown URLs should use cleaner <http://example.com> syntax.'},
{'pattern': 'https://zulip.readthedocs.io/en/latest/[a-zA-Z0-9]',
'exclude': ['docs/overview/contributing.md', 'docs/overview/readme.md', 'docs/README.md'],
'exclude_line': set([
('docs/testing/mypy.md',
'# See https://zulip.readthedocs.io/en/latest/testing/mypy.html#mypy-in-production-scripts')
]),
'include_only': set(['docs/']),
'description': "Use relative links (../foo/bar.html) to other documents in docs/",
},
{'pattern': "su zulip -c [^']",
'include_only': set(['docs/']),
'description': "Always quote arguments using `su zulip -c '` to avoid confusion about how su works.",
},
{'pattern': r'\][(][^#h]',
'include_only': set(['README.md', 'CONTRIBUTING.md']),
'description': "Use absolute links from docs served by GitHub",
},
]),
max_length=120,
length_exclude=markdown_docs_length_exclude,
exclude_files_in='templates/zerver/help/'
)
color = next(colors) help_markdown_rules = RuleList(
for fn in by_lang['rst']: langs=['md'],
if custom_check_file(fn, 'rst', txt_rules, color): rules=markdown_rules.rules + cast(Rule, [
failed = True {'pattern': '[a-z][.][A-Z]',
'description': "Likely missing space after end of sentence",
'include_only': set(['templates/zerver/help/']),
},
{'pattern': r'\b[rR]ealm[s]?\b',
'include_only': set(['templates/zerver/help/']),
'good_lines': ['Organization', 'deactivate_realm', 'realm_filter'],
'bad_lines': ['Users are in a realm', 'Realm is the best model'],
'description': "Realms are referred to as Organizations in user-facing docs."},
]),
length_exclude=markdown_docs_length_exclude,
)
color = next(colors) txt_rules = RuleList(
for fn in by_lang['yaml']: langs=['txt', 'text', 'yaml', 'rst'],
if custom_check_file(fn, 'yaml', txt_rules, color): rules=whitespace_rules,
failed = True )
non_py_rules = [
return failed handlebars_rules,
jinja2_rules,
return (check_custom_checks_py, check_custom_checks_nonpy) css_rules,
js_rules,
json_rules,
markdown_rules,
help_markdown_rules,
bash_rules,
txt_rules,
]

28
tools/tests/test_linter_custom_check.py → tools/tests/test_zulint_custom_rules.py
View File

@ -3,28 +3,18 @@ import os
from mock import patch from mock import patch
from unittest import TestCase from unittest import TestCase
from typing import Any, Dict, List from zulint.custom_rules import RuleList
from linter_lib.custom_check import python_rules, non_py_rules
from tools.linter_lib.custom_check import build_custom_checkers
from tools.linter_lib.custom_check import custom_check_file
ROOT_DIR = os.path.abspath(os.path.join(__file__, '..', '..', '..')) ROOT_DIR = os.path.abspath(os.path.join(__file__, '..', '..', '..'))
CHECK_MESSAGE = "Fix the corresponding rule in `tools/linter_lib/custom_check.py`." CHECK_MESSAGE = "Fix the corresponding rule in `tools/zulint/custom_rules.py`."
class TestCustomRules(TestCase): class TestRuleList(TestCase):
def setUp(self) -> None: def setUp(self) -> None:
self.all_rules = [] # type: List[Dict[str, Any]] self.all_rules = python_rules.rules
with patch('tools.linter_lib.custom_check.custom_check_file', return_value=False) as mock_custom_check_file: for rule in non_py_rules:
by_lang = dict.fromkeys(['py', 'js', 'sh', 'scss', 'handlebars', 'html', self.all_rules.extend(rule.rules)
'json', 'md', 'txt', 'text', 'yaml', 'rst'],
['foo/bar.baz'])
check_custom_checks_py, check_custom_checks_nonpy = build_custom_checkers(by_lang)
check_custom_checks_py()
check_custom_checks_nonpy()
for call_args in mock_custom_check_file.call_args_list:
rule_set = call_args[0][2]
self.all_rules.extend(rule_set)
def test_paths_in_rules(self) -> None: def test_paths_in_rules(self) -> None:
"""Verifies that the paths mentioned in linter rules actually exist""" """Verifies that the paths mentioned in linter rules actually exist"""
@ -53,7 +43,7 @@ class TestCustomRules(TestCase):
for line in rule.get('good_lines', []): for line in rule.get('good_lines', []):
# create=True is superfluous when mocking built-ins in Python >= 3.5 # create=True is superfluous when mocking built-ins in Python >= 3.5
with patch('builtins.open', return_value=iter((line+'\n\n').splitlines()), create=True, autospec=True): with patch('builtins.open', return_value=iter((line+'\n\n').splitlines()), create=True, autospec=True):
self.assertFalse(custom_check_file('foo.bar', 'baz', [rule], ''), self.assertFalse(RuleList([], [rule]).custom_check_file('foo.bar', 'baz', ''),
"The pattern '{}' matched the line '{}' while it shouldn't.".format(pattern, line)) "The pattern '{}' matched the line '{}' while it shouldn't.".format(pattern, line))
for line in rule.get('bad_lines', []): for line in rule.get('bad_lines', []):
@ -61,5 +51,5 @@ class TestCustomRules(TestCase):
with patch('builtins.open', with patch('builtins.open',
return_value=iter((line+'\n\n').splitlines()), create=True, autospec=True), patch('builtins.print'): return_value=iter((line+'\n\n').splitlines()), create=True, autospec=True), patch('builtins.print'):
filename = list(rule.get('include_only', {'foo.bar'}))[0] filename = list(rule.get('include_only', {'foo.bar'}))[0]
self.assertTrue(custom_check_file(filename, 'baz', [rule], ''), self.assertTrue(RuleList([], [rule]).custom_check_file(filename, 'baz', ''),
"The pattern '{}' didn't match the line '{}' while it should.".format(pattern, line)) "The pattern '{}' didn't match the line '{}' while it should.".format(pattern, line))

83
tools/zulint/custom_rules.py
View File

@ -6,16 +6,28 @@ from __future__ import absolute_import
import re import re
import traceback import traceback
from zulint.printer import print_err from zulint.printer import print_err, colors
if False: if False:
from typing import Any, Dict, List, Optional, Tuple, Iterable from typing import Any, Dict, List, Optional, Tuple, Iterable
Rule = Dict[str, Any] Rule = List[Dict[str, Any]]
RuleList = List[Dict[str, Any]]
LineTup = Tuple[int, str, str, str] LineTup = Tuple[int, str, str, str]
def get_line_info_from_file(fn):
class RuleList:
"""Defines and runs custom linting rules for the specified language."""
def __init__(self, langs, rules, max_length=None, length_exclude=[], exclude_files_in=None):
# type: (List[str], Rule, Optional[int], List[str], Rule, Optional[str]) -> None
self.langs = langs
self.rules = rules
self.max_length = max_length
self.length_exclude = length_exclude
# Exclude the files in this folder from rules
self.exclude_files_in = "\\"
def get_line_info_from_file(self, fn):
# type: (str) -> List[LineTup] # type: (str) -> List[LineTup]
line_tups = [] line_tups = []
for i, line in enumerate(open(fn)): for i, line in enumerate(open(fn)):
@ -27,8 +39,8 @@ def get_line_info_from_file(fn):
line_tups.append(tup) line_tups.append(tup)
return line_tups return line_tups
def get_rules_applying_to_fn(fn, rules): def get_rules_applying_to_fn(self, fn, rules):
# type: (str, RuleList) -> RuleList # type: (str, Rule) -> Rule
rules_to_apply = [] rules_to_apply = []
for rule in rules: for rule in rules:
excluded = False excluded = False
@ -49,12 +61,13 @@ def get_rules_applying_to_fn(fn, rules):
return rules_to_apply return rules_to_apply
def check_file_for_pattern(fn, def check_file_for_pattern(self,
fn,
line_tups, line_tups,
identifier, identifier,
color, color,
rule): rule):
# type: (str, List[LineTup], str, Optional[Iterable[str]], Rule) -> bool # type: (str, List[LineTup], str, Optional[Iterable[str]], Dict[str, Any]) -> bool
''' '''
DO NOT MODIFY THIS FUNCTION WITHOUT PROFILING. DO NOT MODIFY THIS FUNCTION WITHOUT PROFILING.
@ -107,7 +120,8 @@ def check_file_for_pattern(fn,
return ok return ok
def check_file_for_long_lines(fn, def check_file_for_long_lines(self,
fn,
max_length, max_length,
line_tups): line_tups):
# type: (str, int, List[LineTup]) -> bool # type: (str, int, List[LineTup]) -> bool
@ -129,20 +143,20 @@ def check_file_for_long_lines(fn,
ok = False ok = False
return ok return ok
def custom_check_file(fn, def custom_check_file(self,
fn,
identifier, identifier,
rules,
color, color,
max_length=None): max_length=None):
# type: (str, str, RuleList, Optional[Iterable[str]], Optional[int]) -> bool # type: (str, str, Optional[Iterable[str]], Optional[int]) -> bool
failed = False failed = False
line_tups = get_line_info_from_file(fn=fn) line_tups = self.get_line_info_from_file(fn=fn)
rules_to_apply = get_rules_applying_to_fn(fn=fn, rules=rules) rules_to_apply = self.get_rules_applying_to_fn(fn=fn, rules=self.rules)
for rule in rules_to_apply: for rule in rules_to_apply:
ok = check_file_for_pattern( ok = self.check_file_for_pattern(
fn=fn, fn=fn,
line_tups=line_tups, line_tups=line_tups,
identifier=identifier, identifier=identifier,
@ -156,11 +170,12 @@ def custom_check_file(fn,
firstline = None firstline = None
lastLine = None lastLine = None
if line_tups: if line_tups:
firstline = line_tups[0][3] # line_fully_stripped for the first line. # line_fully_stripped for the first line.
firstline = line_tups[0][3]
lastLine = line_tups[-1][1] lastLine = line_tups[-1][1]
if max_length is not None: if max_length is not None:
ok = check_file_for_long_lines( ok = self.check_file_for_long_lines(
fn=fn, fn=fn,
max_length=max_length, max_length=max_length,
line_tups=line_tups, line_tups=line_tups,
@ -181,8 +196,8 @@ def custom_check_file(fn,
" for interpreters other than sh."}, " for interpreters other than sh."},
{'pattern': '^#!/usr/bin/env python$', {'pattern': '^#!/usr/bin/env python$',
'description': "Use `#!/usr/bin/env python3` instead of `#!/usr/bin/env python`."} 'description': "Use `#!/usr/bin/env python3` instead of `#!/usr/bin/env python`."}
] # type: RuleList ] # type: Rule
shebang_rules_to_apply = get_rules_applying_to_fn(fn=fn, rules=shebang_rules) shebang_rules_to_apply = self.get_rules_applying_to_fn(fn=fn, rules=shebang_rules)
for rule in shebang_rules_to_apply: for rule in shebang_rules_to_apply:
if re.search(rule['pattern'], firstline): if re.search(rule['pattern'], firstline):
print_err(identifier, color, print_err(identifier, color,
@ -195,3 +210,33 @@ def custom_check_file(fn,
failed = True failed = True
return failed return failed
def check(self, by_lang):
# type: (Dict[str, List[str]]) -> bool
# By default, a rule applies to all files within the extension for
# which it is specified (e.g. all .py files)
# There are three operators we can use to manually include or exclude files from linting for a rule:
# 'exclude': 'set([<path>, ...])' - if <path> is a filename, excludes that file.
# if <path> is a directory, excludes all files
# directly below the directory <path>.
# 'exclude_line': 'set([(<path>, <line>), ...])' - excludes all lines matching <line>
# in the file <path> from linting.
# 'include_only': 'set([<path>, ...])' - includes only those files where <path> is a
# substring of the filepath.
failed = False
for lang in self.langs:
color = next(colors)
for fn in by_lang[lang]:
if fn.startswith(self.exclude_files_in) or ('custom_check.py' in fn):
# This is a bit of a hack, but it generally really doesn't
# work to check the file that defines all the things to check for.
#
# TODO: Migrate this to looking at __module__ type attributes.
continue
max_length = None
if fn not in self.length_exclude:
max_length = self.max_length
if self.custom_check_file(fn, lang, color, max_length=max_length):
failed = True
return failed