diff --git a/tools/lint b/tools/lint index 0e19dc7ed8..1f4db9ac67 100755 --- a/tools/lint +++ b/tools/lint @@ -10,6 +10,7 @@ import argparse from lib import sanity_check sanity_check.check_venv(__file__) +from linter_lib.custom_check import python_rules, non_py_rules from zulint import lister from zulint.command import add_default_linter_arguments, LinterConfig from typing import cast, Dict, List @@ -44,7 +45,6 @@ def run(): root_dir = os.path.dirname(tools_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.pyflakes import check_pyflakes from tools.linter_lib.pep8 import check_pep8 @@ -85,8 +85,6 @@ def run(): else: logger.setLevel(logging.WARNING) - check_custom_checks_py, check_custom_checks_nonpy = build_custom_checkers(by_lang) - linter_config = LinterConfig(by_lang) linter_config.external_linter('add_class', ['tools/find-add-class'], ['js']) linter_config.external_linter('css', ['node', 'node_modules/.bin/stylelint'], ['css', 'scss']) @@ -111,13 +109,15 @@ def run(): @linter_config.lint def custom_py(): # type: () -> int - failed = check_custom_checks_py() + failed = python_rules.check(by_lang) return 1 if failed else 0 @linter_config.lint def custom_nonpy(): # 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 @linter_config.lint diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py index 726fec631f..f69415e3ce 100644 --- a/tools/linter_lib/custom_check.py +++ b/tools/linter_lib/custom_check.py @@ -3,17 +3,31 @@ from __future__ import print_function from __future__ import absolute_import -from zulint.printer import colors -from zulint.custom_rules import ( - custom_check_file, -) +from zulint.custom_rules import RuleList -from typing import cast, Any, Callable, Dict, List, Tuple +from typing import cast, Any, Dict, List, Tuple -Rule = Dict[str, Any] -RuleList = List[Dict[str, Any]] +Rule = 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([, ...])' - if is a filename, excludes that file. +# if is a directory, excludes all files directly below the directory . +# 'exclude_line': 'set([(, ), ...])' - excludes all lines matching in the file from linting. +# 'include_only': 'set([, ...])' - includes only those files where is a substring of the filepath. 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 = { # This basically requires a big DB migration: 'zerver/lib/topic.py', @@ -40,63 +54,47 @@ FILES_WITH_LEGACY_SUBJECT = { '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) +trailing_whitespace_rule = { + 'pattern': r'\s+$', + 'strip': '\n', + 'description': 'Fix trailing whitespace' +} +whitespace_rules = [ + # This linter should be first since bash_rules depends on it. + trailing_whitespace_rule, + {'pattern': 'http://zulip.readthedocs.io', + 'description': 'Use HTTPS when linking to ReadTheDocs', + }, + {'pattern': '\t', + 'strip': '\n', + 'exclude': set(['tools/ci/success-http-headers.txt']), + 'description': 'Fix tab-based whitespace'}, +] # type: Rule +comma_whitespace_rule = [ + {'pattern': ', {2,}[^#/ ]', + 'exclude': set(['zerver/tests', 'frontend_tests/node_tests', 'corporate/tests']), + 'description': "Remove multiple whitespaces after ','", + 'good_lines': ['foo(1, 2, 3)', 'foo = bar # some inline comment'], + 'bad_lines': ['foo(1, 2, 3)', 'foo(1, 2, 3)']}, +] # type: Rule +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. + # This rule finds one space trailing a non-space, three or more trailing spaces, and + # spaces on an empty line. + {'pattern': r'((? 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([, ...])' - if is a filename, excludes that file. - # if is a directory, excludes all files directly below the directory . - # 'exclude_line': 'set([(, ), ...])' - excludes all lines matching in the file from linting. - # 'include_only': 'set([, ...])' - includes only those files where is a substring of the filepath. - trailing_whitespace_rule = { - 'pattern': r'\s+$', - 'strip': '\n', - 'description': 'Fix trailing whitespace' - } - whitespace_rules = [ - # This linter should be first since bash_rules depends on it. - trailing_whitespace_rule, - {'pattern': 'http://zulip.readthedocs.io', - 'description': 'Use HTTPS when linking to ReadTheDocs', - }, - {'pattern': '\t', - 'strip': '\n', - 'exclude': set(['tools/ci/success-http-headers.txt']), - 'description': 'Fix tab-based whitespace'}, - ] # type: RuleList - comma_whitespace_rule = [ - {'pattern': ', {2,}[^#/ ]', - 'exclude': set(['zerver/tests', 'frontend_tests/node_tests', 'corporate/tests']), - 'description': "Remove multiple whitespaces after ','", - 'good_lines': ['foo(1, 2, 3)', 'foo = bar # some inline comment'], - 'bad_lines': ['foo(1, 2, 3)', 'foo(1, 2, 3)']}, - ] # type: RuleList - 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. - # This rule finds one space trailing a non-space, three or more trailing spaces, and - # spaces on an empty line. - {'pattern': r'((?Foo

', 'style = "color: blue;"']}, - ]) + whitespace_rules + comma_whitespace_rule - python_rules = cast(RuleList, [ + ]) + whitespace_rules + comma_whitespace_rule, +) + +python_rules = RuleList( + langs=['py'], + rules=cast(Rule, [ {'pattern': 'subject|SUBJECT', 'exclude_pattern': 'subject to the|email|outbox', 'description': 'avoid subject as a var', @@ -501,8 +503,13 @@ def build_custom_checkers(by_lang): 'include_only': set(["/management/commands/"]), 'description': 'Raise CommandError to exit with failure in management commands', }, - ]) + whitespace_rules + comma_whitespace_rule - bash_rules = cast(RuleList, [ + ]) + whitespace_rules + comma_whitespace_rule, + max_length=110, +) + +bash_rules = RuleList( + langs=['bash'], + rules=cast(Rule, [ {'pattern': '#!.*sh [-xe]', 'description': 'Fix shebang line with proper call to /usr/bin/env for Bash path, change -x|-e switches' ' to set -x|set -e'}, @@ -513,8 +520,12 @@ def build_custom_checkers(by_lang): 'scripts/lib/install', 'scripts/setup/configure-rabbitmq' ]), }, - ]) + whitespace_rules[0:1] - css_rules = cast(RuleList, [ + ]) + whitespace_rules[0:1], +) + +css_rules = RuleList( + langs=['css', 'scss'], + rules=cast(Rule, [ {'pattern': r'calc\([^+]+\+[^+]+\)', 'description': "Avoid using calc with '+' operator. See #8403 : in CSS.", 'good_lines': ["width: calc(20% - -14px);"], @@ -553,146 +564,150 @@ def build_custom_checkers(by_lang): 'good_lines': ["border-width: 5px;"], 'bad_lines': ["border-width: thick;", "border: thick solid black;"]}, ]) + whitespace_rules + comma_whitespace_rule - prose_style_rules = cast(RuleList, [ - {'pattern': r'[^\/\#\-"]([jJ]avascript)', # exclude usage in hrefs/divs - 'description': "javascript should be spelled JavaScript"}, - {'pattern': r'''[^\/\-\."'\_\=\>]([gG]ithub)[^\.\-\_"\<]''', # exclude usage in hrefs/divs - 'description': "github should be spelled GitHub"}, - {'pattern': '[oO]rganisation', # exclude usage in hrefs/divs - 'description': "Organization is spelled with a z", - 'exclude_line': [('docs/translating/french.md', '* organization - **organisation**')]}, - {'pattern': '!!! warning', - 'description': "!!! warning is invalid; it's spelled '!!! warn'"}, - {'pattern': 'Terms of service', - 'description': "The S in Terms of Service is capitalized"}, - {'pattern': '[^-_]botserver(?!rc)|bot server', - 'description': "Use Botserver instead of botserver or bot server."}, - ]) + comma_whitespace_rule - html_rules = whitespace_rules + prose_style_rules + [ - {'pattern': 'subject|SUBJECT', - 'exclude': set(['templates/zerver/email.html']), - 'exclude_pattern': 'email subject', - 'description': 'avoid subject in templates', - 'good_lines': ['topic_name'], - 'bad_lines': ['subject="foo"', ' MAX_SUBJECT_LEN']}, - {'pattern': r'placeholder="[^{#](?:(?!\.com).)+$', - 'description': "`placeholder` value should be translatable.", - 'exclude_line': [('templates/zerver/register.html', 'placeholder="acme"'), - ('templates/zerver/register.html', 'placeholder="Acme or Aκμή"')], - 'exclude': set(["templates/analytics/support.html"]), - 'good_lines': [''], - 'bad_lines': ['']}, - {'pattern': "placeholder='[^{]", - 'description': "`placeholder` value should be translatable.", - 'good_lines': [''], - 'bad_lines': [""]}, - {'pattern': "aria-label='[^{]", - 'description': "`aria-label` value should be translatable.", - 'good_lines': ['"]}, - {'pattern': 'aria-label="[^{]', - 'description': "`aria-label` value should be translatable.", - 'good_lines': ['']}, - {'pattern': 'script src="http', - 'description': "Don't directly load dependencies from CDNs. See docs/subsystems/front-end-build-process.md", - 'exclude': set(["templates/corporate/billing.html", "templates/zerver/hello.html", - "templates/corporate/upgrade.html"]), - 'good_lines': ["{{ render_bundle('landing-page') }}"], - 'bad_lines': ['']}, - {'pattern': "title='[^{]", - 'description': "`title` value should be translatable.", - 'good_lines': [''], - 'bad_lines': ["

"], - }, - {'pattern': r'title="[^{\:]', - 'exclude_line': set([ - ('templates/zerver/app/markdown_help.html', - ':heart:') - ]), - 'exclude': set(["templates/zerver/emails", "templates/analytics/support.html"]), - 'description': "`title` value should be translatable."}, - {'pattern': r'''\Walt=["'][^{"']''', - 'description': "alt argument should be enclosed by _() or it should be an empty string.", - 'exclude': set(['static/templates/settings/display-settings.handlebars', - 'templates/zerver/app/keyboard_shortcuts.html', - 'templates/zerver/app/markdown_help.html']), - 'good_lines': ['{{ _(name) }}', ''], - 'bad_lines': ['Foo Image']}, - {'pattern': r'''\Walt=["']{{ ?["']''', - 'description': "alt argument should be enclosed by _().", - 'good_lines': ['{{ _(name) }}'], - 'bad_lines': ['{{ ']}, - {'pattern': r'\bon\w+ ?=', - 'description': "Don't use inline event handlers (onclick=, etc. attributes) in HTML. Instead," - "attach a jQuery event handler ($('#foo').on('click', function () {...})) when " - "the DOM is ready (inside a $(function () {...}) block).", - 'exclude': set(['templates/zerver/dev_login.html', 'templates/corporate/upgrade.html']), - 'good_lines': ["($('#foo').on('click', function () {}"], - 'bad_lines': ["", ""]}, - {'pattern': 'style ?=', - 'description': "Avoid using the `style=` attribute; we prefer styling in CSS files", - 'exclude_pattern': r'.*style ?=["' + "'" + '](display: ?none|background: {{|color: {{|background-color: {{).*', - 'exclude': set([ - # KaTeX output uses style attribute - 'templates/zerver/app/markdown_help.html', - # 5xx page doesn't have external CSS - 'static/html/5xx.html', - # Group PMs color is dynamically calculated - 'static/templates/group_pms.handlebars', +) - # exclude_pattern above handles color, but have other issues: - 'static/templates/draft.handlebars', - 'static/templates/subscription.handlebars', - 'static/templates/single_message.handlebars', +prose_style_rules = cast(Rule, [ + {'pattern': r'[^\/\#\-"]([jJ]avascript)', # exclude usage in hrefs/divs + 'description': "javascript should be spelled JavaScript"}, + {'pattern': r'''[^\/\-\."'\_\=\>]([gG]ithub)[^\.\-\_"\<]''', # exclude usage in hrefs/divs + 'description': "github should be spelled GitHub"}, + {'pattern': '[oO]rganisation', # exclude usage in hrefs/divs + 'description': "Organization is spelled with a z", + 'exclude_line': [('docs/translating/french.md', '* organization - **organisation**')]}, + {'pattern': '!!! warning', + 'description': "!!! warning is invalid; it's spelled '!!! warn'"}, + {'pattern': 'Terms of service', + 'description': "The S in Terms of Service is capitalized"}, + {'pattern': '[^-_]botserver(?!rc)|bot server', + 'description': "Use Botserver instead of botserver or bot server."}, +]) + comma_whitespace_rule +html_rules = whitespace_rules + prose_style_rules + cast(Rule, [ + {'pattern': 'subject|SUBJECT', + 'exclude': set(['templates/zerver/email.html']), + 'exclude_pattern': 'email subject', + 'description': 'avoid subject in templates', + 'good_lines': ['topic_name'], + 'bad_lines': ['subject="foo"', ' MAX_SUBJECT_LEN']}, + {'pattern': r'placeholder="[^{#](?:(?!\.com).)+$', + 'description': "`placeholder` value should be translatable.", + 'exclude_line': [('templates/zerver/register.html', 'placeholder="acme"'), + ('templates/zerver/register.html', 'placeholder="Acme or Aκμή"')], + 'exclude': set(["templates/analytics/support.html"]), + 'good_lines': [''], + 'bad_lines': ['']}, + {'pattern': "placeholder='[^{]", + 'description': "`placeholder` value should be translatable.", + 'good_lines': [''], + 'bad_lines': [""]}, + {'pattern': "aria-label='[^{]", + 'description': "`aria-label` value should be translatable.", + 'good_lines': ['"]}, + {'pattern': 'aria-label="[^{]', + 'description': "`aria-label` value should be translatable.", + 'good_lines': ['']}, + {'pattern': 'script src="http', + 'description': "Don't directly load dependencies from CDNs. See docs/subsystems/front-end-build-process.md", + 'exclude': set(["templates/corporate/billing.html", "templates/zerver/hello.html", + "templates/corporate/upgrade.html"]), + 'good_lines': ["{{ render_bundle('landing-page') }}"], + 'bad_lines': ['']}, + {'pattern': "title='[^{]", + 'description': "`title` value should be translatable.", + 'good_lines': [''], + 'bad_lines': ["

"]}, + {'pattern': r'title="[^{\:]', + 'exclude_line': set([ + ('templates/zerver/app/markdown_help.html', + ':heart:') + ]), + 'exclude': set(["templates/zerver/emails", "templates/analytics/support.html"]), + 'description': "`title` value should be translatable."}, + {'pattern': r'''\Walt=["'][^{"']''', + 'description': "alt argument should be enclosed by _() or it should be an empty string.", + 'exclude': set(['static/templates/settings/display-settings.handlebars', + 'templates/zerver/app/keyboard_shortcuts.html', + 'templates/zerver/app/markdown_help.html']), + 'good_lines': ['{{ _(name) }}', ''], + 'bad_lines': ['Foo Image']}, + {'pattern': r'''\Walt=["']{{ ?["']''', + 'description': "alt argument should be enclosed by _().", + 'good_lines': ['{{ _(name) }}'], + 'bad_lines': ['{{ ']}, + {'pattern': r'\bon\w+ ?=', + 'description': "Don't use inline event handlers (onclick=, etc. attributes) in HTML. Instead," + "attach a jQuery event handler ($('#foo').on('click', function () {...})) when " + "the DOM is ready (inside a $(function () {...}) block).", + 'exclude': set(['templates/zerver/dev_login.html', 'templates/corporate/upgrade.html']), + 'good_lines': ["($('#foo').on('click', function () {}"], + 'bad_lines': ["", ""]}, + {'pattern': 'style ?=', + 'description': "Avoid using the `style=` attribute; we prefer styling in CSS files", + 'exclude_pattern': r'.*style ?=["' + "'" + '](display: ?none|background: {{|color: {{|background-color: {{).*', + 'exclude': set([ + # KaTeX output uses style attribute + 'templates/zerver/app/markdown_help.html', + # 5xx page doesn't have external CSS + 'static/html/5xx.html', + # Group PMs color is dynamically calculated + 'static/templates/group_pms.handlebars', - # Old-style email templates need to use inline style - # attributes; it should be possible to clean these up - # when we convert these templates to use premailer. - 'templates/zerver/emails/email_base_messages.html', + # exclude_pattern above handles color, but have other issues: + 'static/templates/draft.handlebars', + 'static/templates/subscription.handlebars', + 'static/templates/single_message.handlebars', - # Email log templates; should clean up. - 'templates/zerver/email.html', - 'templates/zerver/email_log.html', + # Old-style email templates need to use inline style + # attributes; it should be possible to clean these up + # when we convert these templates to use premailer. + 'templates/zerver/emails/email_base_messages.html', - # Probably just needs to be changed to display: none so the exclude works - 'templates/zerver/app/navbar.html', + # Email log templates; should clean up. + 'templates/zerver/email.html', + 'templates/zerver/email_log.html', - # Needs the width cleaned up; display: none is fine - 'static/templates/settings/account-settings.handlebars', + # Probably just needs to be changed to display: none so the exclude works + 'templates/zerver/app/navbar.html', - # background image property is dynamically generated - 'static/templates/user_profile_modal.handlebars', - 'static/templates/sidebar_private_message_list.handlebars', + # Needs the width cleaned up; display: none is fine + 'static/templates/settings/account-settings.handlebars', - # Inline styling for an svg; could be moved to CSS files? - 'templates/zerver/landing_nav.html', - 'templates/zerver/billing_nav.html', - 'templates/zerver/app/home.html', - 'templates/zerver/features.html', - 'templates/zerver/portico-header.html', - 'templates/corporate/billing.html', - 'templates/corporate/upgrade.html', + # background image property is dynamically generated + 'static/templates/user_profile_modal.handlebars', + 'static/templates/sidebar_private_message_list.handlebars', - # Miscellaneous violations to be cleaned up - 'static/templates/user_info_popover_title.handlebars', - 'static/templates/subscription_invites_warning_modal.handlebars', - 'templates/zerver/reset_confirm.html', - 'templates/zerver/config_error.html', - 'templates/zerver/dev_env_email_access_details.html', - 'templates/zerver/confirm_continue_registration.html', - 'templates/zerver/register.html', - 'templates/zerver/accounts_send_confirm.html', - 'templates/zerver/integrations/index.html', - 'templates/zerver/documentation_main.html', - 'templates/analytics/realm_summary_table.html', - 'templates/corporate/zephyr.html', - 'templates/corporate/zephyr-mirror.html', - ]), - 'good_lines': ['#my-style {color: blue;}', 'style="display: none"', "style='display: none"], - 'bad_lines': ['

Foo

', 'style = "color: blue;"']}, - ] # type: RuleList - handlebars_rules = html_rules + [ + # Inline styling for an svg; could be moved to CSS files? + 'templates/zerver/landing_nav.html', + 'templates/zerver/billing_nav.html', + 'templates/zerver/app/home.html', + 'templates/zerver/features.html', + 'templates/zerver/portico-header.html', + 'templates/corporate/billing.html', + 'templates/corporate/upgrade.html', + + # Miscellaneous violations to be cleaned up + 'static/templates/user_info_popover_title.handlebars', + 'static/templates/subscription_invites_warning_modal.handlebars', + 'templates/zerver/reset_confirm.html', + 'templates/zerver/config_error.html', + 'templates/zerver/dev_env_email_access_details.html', + 'templates/zerver/confirm_continue_registration.html', + 'templates/zerver/register.html', + 'templates/zerver/accounts_send_confirm.html', + 'templates/zerver/integrations/index.html', + 'templates/zerver/documentation_main.html', + 'templates/analytics/realm_summary_table.html', + 'templates/corporate/zephyr.html', + 'templates/corporate/zephyr-mirror.html', + ]), + 'good_lines': ['#my-style {color: blue;}', 'style="display: none"', "style='display: none"], + 'bad_lines': ['

Foo

', 'style = "color: blue;"']}, +]) + +handlebars_rules = RuleList( + langs=['handlebars'], + rules=html_rules + cast(Rule, [ {'pattern': "[<]script", 'description': "Do not use inline