mirror of https://github.com/zulip/zulip.git
parent
f4a4c23380
commit
64984f5bf4
|
@ -13,7 +13,7 @@ question.
|
||||||
|
|
||||||
You can run them all at once with
|
You can run them all at once with
|
||||||
|
|
||||||
./tools/lint-all
|
./tools/lint
|
||||||
|
|
||||||
You can set this up as a local Git commit hook with
|
You can set this up as a local Git commit hook with
|
||||||
|
|
||||||
|
@ -21,7 +21,7 @@ You can set this up as a local Git commit hook with
|
||||||
|
|
||||||
The Vagrant setup process runs this for you.
|
The Vagrant setup process runs this for you.
|
||||||
|
|
||||||
`lint-all` runs many lint checks in parallel, including
|
`lint` runs many lint checks in parallel, including
|
||||||
|
|
||||||
- JavaScript ([ESLint](http://eslint.org/))
|
- JavaScript ([ESLint](http://eslint.org/))
|
||||||
- Python ([Pyflakes](http://pypi.python.org/pypi/pyflakes))
|
- Python ([Pyflakes](http://pypi.python.org/pypi/pyflakes))
|
||||||
|
|
|
@ -35,7 +35,7 @@ one small exception: it does not run mypy against scripts).
|
||||||
|
|
||||||
You can also run them individually:
|
You can also run them individually:
|
||||||
|
|
||||||
./tools/lint-all
|
./tools/lint
|
||||||
./tools/run-mypy
|
./tools/run-mypy
|
||||||
./tools/run-mypy --scripts-only
|
./tools/run-mypy --scripts-only
|
||||||
|
|
||||||
|
@ -49,7 +49,7 @@ Our linting tools generally support the ability to lint files
|
||||||
individually--with some caveats--and those options will be described
|
individually--with some caveats--and those options will be described
|
||||||
later in this document.
|
later in this document.
|
||||||
|
|
||||||
We may eventually bundle `run-mypy` into `lint-all`, but mypy is pretty
|
We may eventually bundle `run-mypy` into `lint`, but mypy is pretty
|
||||||
resource intensive compared to the rest of the linters, because it does
|
resource intensive compared to the rest of the linters, because it does
|
||||||
static code analysis. So we keep mypy separate to allow folks to quickly run
|
static code analysis. So we keep mypy separate to allow folks to quickly run
|
||||||
the other lint checks.
|
the other lint checks.
|
||||||
|
@ -59,7 +59,7 @@ the other lint checks.
|
||||||
Once you have read the [Zulip coding guidelines](code-style.html), you can
|
Once you have read the [Zulip coding guidelines](code-style.html), you can
|
||||||
be pretty confident that 99% of the code that you write will pass through
|
be pretty confident that 99% of the code that you write will pass through
|
||||||
the linters fine, as long as you are thorough about keeping your code clean.
|
the linters fine, as long as you are thorough about keeping your code clean.
|
||||||
And, of course, for minor oversights, `lint-all` is your friend, not your foe.
|
And, of course, for minor oversights, `lint` is your friend, not your foe.
|
||||||
|
|
||||||
Occasionally, our linters will complain about things that are more of
|
Occasionally, our linters will complain about things that are more of
|
||||||
an artifact of the linter limitations than any actual problem with your
|
an artifact of the linter limitations than any actual problem with your
|
||||||
|
@ -81,7 +81,7 @@ describes our test system in detail.
|
||||||
|
|
||||||
## Lint checks
|
## Lint checks
|
||||||
|
|
||||||
Most of our lint checks get performed by `./tools/lint-all`. These include the
|
Most of our lint checks get performed by `./tools/lint`. These include the
|
||||||
following checks:
|
following checks:
|
||||||
|
|
||||||
- Check Python code with pyflakes.
|
- Check Python code with pyflakes.
|
||||||
|
@ -99,25 +99,25 @@ code analysis of Python type annotations throughout our Python codebase.
|
||||||
|
|
||||||
Our [documentation on using mypy](mypy.html) covers mypy in more detail.
|
Our [documentation on using mypy](mypy.html) covers mypy in more detail.
|
||||||
|
|
||||||
The rest of this document pertains to the checks that occur in `./tools/lint-all`.
|
The rest of this document pertains to the checks that occur in `./tools/lint`.
|
||||||
|
|
||||||
## lint-all
|
## lint
|
||||||
|
|
||||||
Zulip has a script called `lint-all` that lives in our "tools" directory.
|
Zulip has a script called `lint` that lives in our "tools" directory.
|
||||||
It is the workhorse of our linting system, although in some cases it
|
It is the workhorse of our linting system, although in some cases it
|
||||||
dispatches the heavy lifting to other components such as pyflakes,
|
dispatches the heavy lifting to other components such as pyflakes,
|
||||||
eslint, and other home grown tools.
|
eslint, and other home grown tools.
|
||||||
|
|
||||||
You can find the source code [here](https://github.com/zulip/zulip/blob/master/tools/lint-all).
|
You can find the source code [here](https://github.com/zulip/zulip/blob/master/tools/lint).
|
||||||
|
|
||||||
In order for our entire lint suite to run in a timely fashion, the `lint-all`
|
In order for our entire lint suite to run in a timely fashion, the `lint`
|
||||||
script performs several lint checks in parallel by forking out subprocesses. This mechanism
|
script performs several lint checks in parallel by forking out subprocesses. This mechanism
|
||||||
is still evolving, but you can look at the method `run_parallel` to get the
|
is still evolving, but you can look at the method `run_parallel` to get the
|
||||||
gist of how it works.
|
gist of how it works.
|
||||||
|
|
||||||
### Special options
|
### Special options
|
||||||
|
|
||||||
You can use the `-h` option for `lint-all` to see its usage. One particular
|
You can use the `-h` option for `lint` to see its usage. One particular
|
||||||
flag to take note of is the `--modified` flag, which enables you to only run
|
flag to take note of is the `--modified` flag, which enables you to only run
|
||||||
lint checks against files that are modified in your git repo. Most of the
|
lint checks against files that are modified in your git repo. Most of the
|
||||||
"sub-linters" respect this flag, but some will continue to process all the files.
|
"sub-linters" respect this flag, but some will continue to process all the files.
|
||||||
|
@ -156,7 +156,7 @@ unused imports--because of the way mypy type annotations work in Python 2,
|
||||||
it would be inconvenient to enforce this too strictly.)
|
it would be inconvenient to enforce this too strictly.)
|
||||||
|
|
||||||
Zulip also has custom regex-based rules that it applies to Python code.
|
Zulip also has custom regex-based rules that it applies to Python code.
|
||||||
Look for `python_rules` in the source code for `lint-all`. Note that we
|
Look for `python_rules` in the source code for `lint`. Note that we
|
||||||
provide a mechanism to exclude certain lines of codes from these checks.
|
provide a mechanism to exclude certain lines of codes from these checks.
|
||||||
Often, it is simply the case that our regex approach is too crude to
|
Often, it is simply the case that our regex approach is too crude to
|
||||||
correctly exonerate certain valid constructs. In other cases, the code
|
correctly exonerate certain valid constructs. In other cases, the code
|
||||||
|
|
|
@ -32,13 +32,13 @@ Then, to run the full Zulip test suite, do this:
|
||||||
./tools/test-all
|
./tools/test-all
|
||||||
```
|
```
|
||||||
|
|
||||||
This runs the linter (`tools/lint-all`) plus all of our test suites;
|
This runs the linter (`tools/lint`) plus all of our test suites;
|
||||||
they can all be run separately (just read `tools/test-all` to see
|
they can all be run separately (just read `tools/test-all` to see
|
||||||
them). You can also run individual tests which can save you a lot of
|
them). You can also run individual tests which can save you a lot of
|
||||||
time debugging a test failure, e.g.:
|
time debugging a test failure, e.g.:
|
||||||
|
|
||||||
```
|
```
|
||||||
./tools/lint-all # Runs all the linters in parallel
|
./tools/lint # Runs all the linters in parallel
|
||||||
./tools/test-backend zerver.tests.test_bugdown.BugdownTest.test_inline_youtube
|
./tools/test-backend zerver.tests.test_bugdown.BugdownTest.test_inline_youtube
|
||||||
./tools/test-backend BugdownTest # Run `test-backend --help` for more options
|
./tools/test-backend BugdownTest # Run `test-backend --help` for more options
|
||||||
./tools/test-js-with-casper 09-navigation.js
|
./tools/test-js-with-casper 09-navigation.js
|
||||||
|
|
|
@ -254,7 +254,7 @@ JsonableError(_('English Text'))
|
||||||
```
|
```
|
||||||
|
|
||||||
To ensure we always internationalize our JSON errors messages, the
|
To ensure we always internationalize our JSON errors messages, the
|
||||||
Zulip linter (`tools/lint-all`) checks for correct usage.
|
Zulip linter (`tools/lint`) checks for correct usage.
|
||||||
|
|
||||||
## Frontend translations
|
## Frontend translations
|
||||||
|
|
||||||
|
|
|
@ -119,7 +119,7 @@ UBUNTU_COMMON_APT_DEPENDENCIES = [
|
||||||
"yui-compressor",
|
"yui-compressor",
|
||||||
"wget",
|
"wget",
|
||||||
"ca-certificates", # Explicit dependency in case e.g. wget is already installed
|
"ca-certificates", # Explicit dependency in case e.g. wget is already installed
|
||||||
"puppet", # Used by lint-all
|
"puppet", # Used by lint
|
||||||
"gettext", # Used by makemessages i18n
|
"gettext", # Used by makemessages i18n
|
||||||
"curl", # Used for fetching PhantomJS as wget occasionally fails on redirects
|
"curl", # Used for fetching PhantomJS as wget occasionally fails on redirects
|
||||||
"netcat", # Used for flushing memcached
|
"netcat", # Used for flushing memcached
|
||||||
|
|
|
@ -288,21 +288,21 @@ def build_custom_checkers(by_lang):
|
||||||
{'pattern': '[.]text\(["\'][a-zA-Z]',
|
{'pattern': '[.]text\(["\'][a-zA-Z]',
|
||||||
'description': 'Strings passed to $().text should be wrapped in i18n.t() for internationalization'},
|
'description': 'Strings passed to $().text should be wrapped in i18n.t() for internationalization'},
|
||||||
{'pattern': 'compose_error\(["\']',
|
{'pattern': 'compose_error\(["\']',
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
'description': 'Argument to compose_error should be a literal string enclosed '
|
'description': 'Argument to compose_error should be a literal string enclosed '
|
||||||
'by i18n.t()'},
|
'by i18n.t()'},
|
||||||
{'pattern': 'ui.report_success\(',
|
{'pattern': 'ui.report_success\(',
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
'description': 'Deprecated function, use ui_report.success.'},
|
'description': 'Deprecated function, use ui_report.success.'},
|
||||||
{'pattern': 'report.success\(["\']',
|
{'pattern': 'report.success\(["\']',
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
'description': 'Argument to report_success should be a literal string enclosed '
|
'description': 'Argument to report_success should be a literal string enclosed '
|
||||||
'by i18n.t()'},
|
'by i18n.t()'},
|
||||||
{'pattern': 'ui.report_error\(',
|
{'pattern': 'ui.report_error\(',
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
'description': 'Deprecated function, use ui_report.error.'},
|
'description': 'Deprecated function, use ui_report.error.'},
|
||||||
{'pattern': 'report.error\(["\']',
|
{'pattern': 'report.error\(["\']',
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
'description': 'Argument to report_error should be a literal string enclosed '
|
'description': 'Argument to report_error should be a literal string enclosed '
|
||||||
'by i18n.t()'},
|
'by i18n.t()'},
|
||||||
]) + whitespace_rules
|
]) + whitespace_rules
|
||||||
|
@ -312,7 +312,7 @@ def build_custom_checkers(by_lang):
|
||||||
{'pattern': '".*"%\([a-z_].*\)?$',
|
{'pattern': '".*"%\([a-z_].*\)?$',
|
||||||
'description': 'Missing space around "%"'},
|
'description': 'Missing space around "%"'},
|
||||||
{'pattern': "'.*'%\([a-z_].*\)?$",
|
{'pattern': "'.*'%\([a-z_].*\)?$",
|
||||||
'exclude': set(['tools/lint-all',
|
'exclude': set(['tools/lint',
|
||||||
'analytics/lib/counts.py',
|
'analytics/lib/counts.py',
|
||||||
'analytics/tests/test_counts.py',
|
'analytics/tests/test_counts.py',
|
||||||
]),
|
]),
|
||||||
|
@ -336,14 +336,14 @@ def build_custom_checkers(by_lang):
|
||||||
{'pattern': "assertEquals[(]",
|
{'pattern': "assertEquals[(]",
|
||||||
'description': 'Use assertEqual, not assertEquals (which is deprecated).'},
|
'description': 'Use assertEqual, not assertEquals (which is deprecated).'},
|
||||||
{'pattern': "== None",
|
{'pattern': "== None",
|
||||||
'exclude': 'tools/lint-all',
|
'exclude': 'tools/lint',
|
||||||
'description': 'Use `is None` to check whether something is None'},
|
'description': 'Use `is None` to check whether something is None'},
|
||||||
{'pattern': "type:[(]",
|
{'pattern': "type:[(]",
|
||||||
'description': 'Missing whitespace after ":" in type annotation'},
|
'description': 'Missing whitespace after ":" in type annotation'},
|
||||||
{'pattern': "# type [(]",
|
{'pattern': "# type [(]",
|
||||||
'description': 'Missing : after type in type annotation'},
|
'description': 'Missing : after type in type annotation'},
|
||||||
{'pattern': "#type",
|
{'pattern': "#type",
|
||||||
'exclude': 'tools/lint-all',
|
'exclude': 'tools/lint',
|
||||||
'description': 'Missing whitespace after "#" in type annotation'},
|
'description': 'Missing whitespace after "#" in type annotation'},
|
||||||
{'pattern': 'if[(]',
|
{'pattern': 'if[(]',
|
||||||
'description': 'Missing space between if and ('},
|
'description': 'Missing space between if and ('},
|
||||||
|
@ -368,10 +368,10 @@ def build_custom_checkers(by_lang):
|
||||||
'description': 'Use json_success() to return nothing'},
|
'description': 'Use json_success() to return nothing'},
|
||||||
# To avoid json_error(_variable) and json_error(_(variable))
|
# To avoid json_error(_variable) and json_error(_(variable))
|
||||||
{'pattern': '\Wjson_error\(_\(?\w+\)',
|
{'pattern': '\Wjson_error\(_\(?\w+\)',
|
||||||
'exclude': set(['tools/lint-all', 'zerver/tests']),
|
'exclude': set(['tools/lint', 'zerver/tests']),
|
||||||
'description': 'Argument to json_error should be a literal string enclosed by _()'},
|
'description': 'Argument to json_error should be a literal string enclosed by _()'},
|
||||||
{'pattern': '\Wjson_error\([\'"].+[),]$',
|
{'pattern': '\Wjson_error\([\'"].+[),]$',
|
||||||
'exclude': set(['tools/lint-all', 'zerver/tests']),
|
'exclude': set(['tools/lint', 'zerver/tests']),
|
||||||
'exclude_line': set([
|
'exclude_line': set([
|
||||||
# We don't want this string tagged for translation.
|
# We don't want this string tagged for translation.
|
||||||
('zerver/views/compatibility.py', 'return json_error("Client is too old")'),
|
('zerver/views/compatibility.py', 'return json_error("Client is too old")'),
|
||||||
|
@ -379,10 +379,10 @@ def build_custom_checkers(by_lang):
|
||||||
'description': 'Argument to json_error should a literal string enclosed by _()'},
|
'description': 'Argument to json_error should a literal string enclosed by _()'},
|
||||||
# To avoid JsonableError(_variable) and JsonableError(_(variable))
|
# To avoid JsonableError(_variable) and JsonableError(_(variable))
|
||||||
{'pattern': '\WJsonableError\(_\(?\w.+\)',
|
{'pattern': '\WJsonableError\(_\(?\w.+\)',
|
||||||
'exclude': set(['tools/lint-all', 'zerver/tests']),
|
'exclude': set(['tools/lint', 'zerver/tests']),
|
||||||
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
||||||
{'pattern': '\WJsonableError\(["\'].+\)',
|
{'pattern': '\WJsonableError\(["\'].+\)',
|
||||||
'exclude': set(['tools/lint-all', 'zerver/tests']),
|
'exclude': set(['tools/lint', 'zerver/tests']),
|
||||||
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
'description': 'Argument to JsonableError should be a literal string enclosed by _()'},
|
||||||
{'pattern': '([a-zA-Z0-9_]+)=REQ\([\'"]\\1[\'"]',
|
{'pattern': '([a-zA-Z0-9_]+)=REQ\([\'"]\\1[\'"]',
|
||||||
'description': 'REQ\'s first argument already defaults to parameter name'},
|
'description': 'REQ\'s first argument already defaults to parameter name'},
|
||||||
|
@ -434,7 +434,7 @@ def build_custom_checkers(by_lang):
|
||||||
},
|
},
|
||||||
{'pattern': 'render_to_response\(',
|
{'pattern': 'render_to_response\(',
|
||||||
'description': "Use render() instead of render_to_response().",
|
'description': "Use render() instead of render_to_response().",
|
||||||
'exclude': set(['tools/lint-all']),
|
'exclude': set(['tools/lint']),
|
||||||
},
|
},
|
||||||
# This rule might give false positives in virtualenv setup files which should be excluded,
|
# This rule might give false positives in virtualenv setup files which should be excluded,
|
||||||
# and comments which should be rewritten to avoid use of "python2", "python3", etc.
|
# and comments which should be rewritten to avoid use of "python2", "python3", etc.
|
||||||
|
@ -442,7 +442,7 @@ def build_custom_checkers(by_lang):
|
||||||
'exclude': set(['tools/lib/provision.py',
|
'exclude': set(['tools/lib/provision.py',
|
||||||
'tools/setup/setup_venvs.py',
|
'tools/setup/setup_venvs.py',
|
||||||
'scripts/lib/setup_venv.py',
|
'scripts/lib/setup_venv.py',
|
||||||
'tools/lint-all']),
|
'tools/lint']),
|
||||||
'description': 'Explicit python invocations should not include a version'}
|
'description': 'Explicit python invocations should not include a version'}
|
||||||
]) + whitespace_rules
|
]) + whitespace_rules
|
||||||
bash_rules = [
|
bash_rules = [
|
|
@ -1,6 +1,6 @@
|
||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
# This hook runs the Zulip code linter ./tools/lint-all and returns true
|
# This hook runs the Zulip code linter ./tools/lint and returns true
|
||||||
# regardless of linter results so that your commit may continue.
|
# regardless of linter results so that your commit may continue.
|
||||||
|
|
||||||
# Messages from the linter will be printed out to the screen.
|
# Messages from the linter will be printed out to the screen.
|
||||||
|
@ -16,10 +16,10 @@ if [ -z "$changed_files" ]; then
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [ -z "$VIRTUAL_ENV" ] && `which vagrant > /dev/null` && [ -e .vagrant ]; then
|
if [ -z "$VIRTUAL_ENV" ] && `which vagrant > /dev/null` && [ -e .vagrant ]; then
|
||||||
vcmd="/srv/zulip/tools/lint-all --pep8 --no-gitlint --force $changed_files || true"
|
vcmd="/srv/zulip/tools/lint --pep8 --no-gitlint --force $changed_files || true"
|
||||||
echo "Running lint-all using vagrant..."
|
echo "Running lint using vagrant..."
|
||||||
vagrant ssh -c "$vcmd"
|
vagrant ssh -c "$vcmd"
|
||||||
else
|
else
|
||||||
./tools/lint-all --pep8 --no-gitlint --force $changed_files || true
|
./tools/lint --pep8 --no-gitlint --force $changed_files || true
|
||||||
fi
|
fi
|
||||||
exit 0
|
exit 0
|
||||||
|
|
|
@ -33,7 +33,7 @@ function run {
|
||||||
run ./tools/check-provision $FORCEARG
|
run ./tools/check-provision $FORCEARG
|
||||||
run ./tools/clean-repo
|
run ./tools/clean-repo
|
||||||
run ./tools/test-tools
|
run ./tools/test-tools
|
||||||
run ./tools/lint-all --pep8 $FORCEARG
|
run ./tools/lint --pep8 $FORCEARG
|
||||||
run ./tools/test-migrations
|
run ./tools/test-migrations
|
||||||
run ./tools/test-js-with-node
|
run ./tools/test-js-with-node
|
||||||
run ./tools/run-mypy
|
run ./tools/run-mypy
|
||||||
|
|
|
@ -5,7 +5,7 @@ source tools/travis/activate-venv
|
||||||
set -e
|
set -e
|
||||||
set -x
|
set -x
|
||||||
|
|
||||||
./tools/lint-all --pep8 # Include the slow and thus non-default pep8 linter check
|
./tools/lint --pep8 # Include the slow and thus non-default pep8 linter check
|
||||||
./tools/test-tools
|
./tools/test-tools
|
||||||
./tools/test-backend --coverage
|
./tools/test-backend --coverage
|
||||||
./tools/test-migrations
|
./tools/test-migrations
|
||||||
|
|
Loading…
Reference in New Issue