zulip/docs/testing/linters.md

# Linters

## Overview

Zulip does extensive linting of much of its source code, including
Python/JavaScript files, HTML templates (Django/handlebars), CSS files,
JSON fixtures, Markdown documents, puppet manifests, and shell scripts.

For some files we simply check for small things like trailing whitespace,
but for other files, we are quite thorough about checking semantic
correctness.

Obviously, a large reason for linting code is to enforce the [Zulip
coding standards](../contributing/code-style.html).  But we also use the linters to
prevent common coding errors.

We borrow some open source tools for much of our linting, and the links
below will direct you to the official documentation for these projects.

- [eslint](http://eslint.org)
- [mypy](http://mypy-lang.org/)
- [puppet](https://puppet.com/) (puppet provides its own mechanism for
  validating manifests)
- [pyflakes](https://pypi.python.org/pypi/pyflakes)
- [stylelint](https://github.com/stylelint/stylelint)

Zulip also uses some home-grown code to perform tasks like validating
indentation in template files, enforcing coding standards that are unique
to Zulip, allowing certain errors from third party linters to pass through,
and exempting legacy files from lint checks.

## Running the linters

If you run `./tools/test-all`, it will automatically run the linters.
You can also run them individually or pass specific files:

    ./tools/lint
    ./tools/lint static/js/compose.js
    ./tools/lint static/js/
    ./tools/run-mypy

Finally, you can rely on our Travis CI setup to run linters for you, but
it is good practice to run lint checks locally.

**Important:** We provide a
  [Git pre-commit hook](http://localhost:9991/docs/git/zulip-tools.html#set-up-git-repo-script)
  that can automatically run `tools/lint` on just the files that
  changed (in a few 100ms) whenever you make a commit.  This can save
  you a lot of time, by automatically detecting linter errors as you
  make them.

**Note:** The linters only check files that git tracks. Remember to `git add`
new files before running lint checks.

Our linting tools generally support the ability to lint files
individually--with some caveats--and those options will be described
later in this document.

We may eventually bundle `run-mypy` into `lint`, but mypy is pretty
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
the other lint checks.

## General considerations

Once you have read the [Zulip coding guidelines](../contributing/code-style.html), you can
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.
And, of course, for minor oversights, `lint` is your friend, not your foe.

Occasionally, our linters will complain about things that are more of
an artifact of the linter limitations than any actual problem with your
code.  There is usually a mechanism where you can bypass the linter in
extreme cases, but often it can be a simple matter of writing your code
in a slightly different style to appease the linter.  If you have
problems getting something to lint, you can submit an unfinished PR
and ask the reviewer to help you work through the lint problem, or you
can find other people in the [Zulip Community](../contributing/chat-zulip-org.html)
to help you.

Also, bear in mind that 100% of the lint code is open source, so if you
find limitations in either the Zulip home-grown stuff or our third party
tools, feedback will be highly appreciated.

Finally, one way to clean up your code is to thoroughly exercise it
with tests.  The [Zulip test documentation](../testing/testing.html)
describes our test system in detail.

## Lint checks

Most of our lint checks get performed by `./tools/lint`.  These include the
following checks:

- Check Python code with pyflakes.
- Check JavaScript code with eslint.
- Check Python code for custom Zulip rules.
- Check non-Python code for custom Zulip rules.
- Check puppet manifests with the puppet validator.
- Check HTML templates for matching tags and indentations.
- Check CSS for parsability and formatting.
- Check JavaScript code for addClass calls.
- Running `mypy` to check static types in Python code.  Our
  [documentation on using mypy](../testing/mypy.html) covers mypy in
  more detail.

The rest of this document pertains to the checks that occur in `./tools/lint`.

## lint

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
dispatches the heavy lifting to other components such as pyflakes,
eslint, and other home grown tools.

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`
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
gist of how it works.

Note that our project does custom regex-based checks on the code, and we
also customize how we call pyflakes and pycodestyle (pep8).  The code for these
types of checks mostly lives [here](https://github.com/zulip/zulip/blob/master/tools/linter_lib).

### Special options

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
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.
Generally, a good workflow is to run with `--modified` when you are iterating on
the code, and then run without that option right before committing new code.

If you need to troubleshoot the linters, there is a `--verbose` option that
can give you clues about which linters may be running slow, for example.

### Lint checks

The next part of this document describes the lint checks that we apply to
various file types.

#### Generic source code checks

We check almost our entire codebase for trailing whitespace.  Also, we
disallow tab (\t) characters in all but two files.

We also have custom regex-based checks that apply to specific file types.
For relatively minor files like Markdown files and JSON fixtures, this
is the extent of our checking.

Finally, we're checking line length in Python code (and hope to extend
this to other parts of the codebase soon).  You can use
`#ignorelinelength` for special cases where a very long line makes
sense (e.g. a link in a comment to an extremely long URL).

#### Python code

The bulk of our Python linting gets outsourced to the "pyflakes" tool.  We
call "pyflakes" in a fairly vanilla fashion, and then we post-process its
output to exclude certain types of errors that Zulip is comfortable
ignoring.  (One notable class of error that Zulip currently tolerates is
unused imports--because of the way mypy type annotations work in Python 2,
it would be inconvenient to enforce this too strictly.)

Zulip also has custom regex-based rules that it applies to Python code.
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.
Often, it is simply the case that our regex approach is too crude to
correctly exonerate certain valid constructs.  In other cases, the code
that we exempt may be deemed not worthwhile to fix.

#### JavaScript code

We check our JavaScript code in a few different ways:
- We run eslint.
- We perform custom Zulip regex checks on the code.
- We verify that all addClass calls, with a few exceptions, explicitly
  contain a CSS class.

The last check happens via a call to `./tools/find-add-class`.  This
particular check is a work in progress, as we are trying to evolve a
more rigorous system for weeding out legacy CSS styles, and the ability
to quickly introspect our JS code for `addClass` calls is part of our
vision.

#### Puppet manifests

We use Puppet as our tool to manage configuration files, using
puppet "manifests."  To lint puppet manifests, we use the "parser validate"
option of puppet.

#### HTML Templates

Zulip uses two HTML templating systems:

- [Django templates](https://docs.djangoproject.com/en/1.10/topics/templates/)
- [handlebars](http://handlebarsjs.com/)

Zulip has an internal tool that validates both types of templates for
correct indentation and matching tags.  You can find the code here:

- driver: [check-templates](https://github.com/zulip/zulip/blob/master/tools/check-templates)
- engine: [lib/template_parser.py](https://github.com/zulip/zulip/blob/master/tools/lib/template_parser.py)

We exempt some legacy files from indentation checks, but we are hoping to
clean those files up eventually.

#### CSS

Zulip uses [stylelint](https://github.com/stylelint/stylelint) to lint
its CSS; see our
[configuration](https://github.com/zulip/zulip/blob/master/.stylelintrc)
for the rules we currently enforce.

#### Shell scripts

Zulip uses [shellcheck](https://github.com/koalaman/shellcheck) to
lint our shell scripts.  We recommend the
[shellcheck gallery of bad code][shellcheck-bad-code] as a resource on
how to not write bad shell.

[shellcheck-bad-code]: https://github.com/koalaman/shellcheck/blob/master/README.md#user-content-gallery-of-bad-code

#### Markdown, shell scripts, JSON fixtures

We mostly validate miscellaneous source files like `.json`, and `.md`
files for whitespace issues.

## Philosophy

If you want to help improve Zulip's system for linting, here are some
considerations.

#### Speed

We want our linters to be fast enough that most developers
will feel comfortable running them in a pre-commit hook, so we run
our linters in parallel and support incremental checks.

#### Accuracy

We try to catch as many common mistakes as possible, either via a
linter or an automated test.

#### Completeness

Our goal is to have most common style issues by caught by the linters, so new
contributors to the codebase can efficiently fix produce code with correct
style without needing to go back-and-forth with a reviewer.