mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-18 21:02:41 +01:00
Further modernize lint documentation
Summary: Ref T2039. Contains a small amount of wishful thinking, I'll note it inline. Test Plan: Generated and read documentation. Reviewers: btrahan, avive, joshuaspence Reviewed By: joshuaspence Subscribers: epriestley, avivey Maniphest Tasks: T2039 Differential Revision: https://secure.phabricator.com/D9061
This commit is contained in:
parent
1644ef185e
commit
94772689cd
1 changed files with 311 additions and 153 deletions
|
@ -11,7 +11,9 @@ This guide explains how lint works when configured in an `arc` project. If
|
|||
you haven't set up a project yet, do that first. For instructions, see
|
||||
@{article:Arcanist User Guide: Configuring a New Project}.
|
||||
|
||||
= Overview =
|
||||
|
||||
Overview
|
||||
========
|
||||
|
||||
"Lint" refers to a general class of programming tools which analyze source code
|
||||
and raise warnings and errors about it. For example, a linter might raise
|
||||
|
@ -35,87 +37,321 @@ There are many lint and static analysis tools available for a wide variety of
|
|||
languages. Arcanist ships with bindings for many popular tools, and you can
|
||||
write new bindings fairly easily if you have custom tools.
|
||||
|
||||
= Available Linters =
|
||||
|
||||
Arcanist ships with bindings for these linters:
|
||||
Available Linters
|
||||
=================
|
||||
|
||||
- [[http://www.jshint.com/ | JSHint]], a Javascript linter.
|
||||
See @{class@arcanist:ArcanistJSHintLinter}.
|
||||
- [[http://pypi.python.org/pypi/pep8 | PEP8]],
|
||||
[[http://pypi.python.org/pypi/pyflakes | Pyflakes]],
|
||||
[[https://flake8.readthedocs.org/ | Flake8]], and
|
||||
[[http://pypi.python.org/pypi/pylint | Pylint]] - various Python linters.
|
||||
See @{class@arcanist:ArcanistPEP8Linter},
|
||||
@{class@arcanist:ArcanistPyFlakesLinter},
|
||||
@{class@arcanist:ArcanistFlake8Linter},
|
||||
and @{class@arcanist:ArcanistPyLintLinter}.
|
||||
- [[http://pear.php.net/package/PHP_CodeSniffer | PHP CodeSniffer]], a
|
||||
PHP linter. See @{class@arcanist:ArcanistPhpcsLinter}.
|
||||
- [[http://cppcheck.sourceforge.net/ | Cppcheck]] and
|
||||
[[http://google-styleguide.googlecode.com/svn/trunk/cpplint/cpplint.py |
|
||||
cpplint.py]], two checkers for C++. See
|
||||
@{class@arcanist:ArcanistCppcheckLinter} and
|
||||
@{class@arcanist:ArcanistCpplintLinter} respectively.
|
||||
- [[https://github.com/hach-que/cstools | cslint]], a C# linting tool based on
|
||||
[[http://stylecop.codeplex.com/ | StyleCop]]. See
|
||||
@{class@arcanist:ArcanistCSharpLinter}.
|
||||
- [[http://puppet-lint.com/ | puppet-lint]], a linter for
|
||||
[[http://puppetlabs.com/ | Puppet]] manifests. See
|
||||
@{class@arcanist:ArcanistPuppetLintLinter}.
|
||||
- [[http://www.ruby-lang.org | Ruby]]'s built-in checker. See
|
||||
@{class@arcanist:ArcanistRubyLinter}.
|
||||
- [[http://www.scala-sbt.org/ | sbt]], a built tool for Scala. See
|
||||
@{class@arcanist:ArcanistScalaSBTLinter}.
|
||||
- [[http://csslint.net/ | CSSLint]], for CSS:
|
||||
@{class@arcanist:ArcanistCSSLintLinter}.
|
||||
- [[https://github.com/less/less.js | lessc]], error detection in
|
||||
[[http://lesscss.org/ | LESS]] code. @{class@arcanist:ArcanistLesscLinter}.
|
||||
- [[http://php.net/simplexml | SimpleXML]] for XML files.
|
||||
@{class@arcanist:ArcanistXMLLinter}.
|
||||
To see a list of available linters, run:
|
||||
|
||||
$ arc linters
|
||||
|
||||
Arcanist ships with bindings for a number of linters that can check for errors
|
||||
or problems in JS, CSS, PHP, Python, C, C++, C#, Less, Puppet, Ruby, JSON, XML,
|
||||
and several other languages.
|
||||
|
||||
Some general purpose linters are also available. These linters can check for
|
||||
cross-language issues like sensible filenames, trailing or mixed whitespace,
|
||||
character sets, spelling mistakes, and unresolved merge conflicts.
|
||||
|
||||
If you have a tool you'd like to use as a linter that isn't supported by
|
||||
default, you can write bindings for it. For information on writing new linter
|
||||
bindings, see @{article:Arcanist User Guide: Customizing Lint, Unit Tests and
|
||||
Workflows}.
|
||||
|
||||
|
||||
Arcanist also ships with generic bindings which can be configured to parse the
|
||||
output of a broad range of lint programs:
|
||||
Configuring Lint
|
||||
================
|
||||
|
||||
- @{class@arcanist:ArcanistScriptAndRegexLinter}, which runs a script and
|
||||
parses its output with a regular expression.
|
||||
- @{class@arcanist:ArcanistConduitLinter}, which invokes a linter over
|
||||
Conduit and can allow you to build client/server linters.
|
||||
To configure lint integration for your project, create a file called `.arclint`
|
||||
at the project root. This file should be in JSON format, and look like this:
|
||||
|
||||
Additionally, Arcanist ships with some general purpose linters:
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- @{class@arcanist:ArcanistTextLinter}, which enforces basic things like
|
||||
trailing whitespace, DOS newlines, file encoding, line widths, terminal
|
||||
newlines, and tab literals.
|
||||
- @{class@arcanist:ArcanistSpellingLinter}, which can detect common spelling
|
||||
mistakes.
|
||||
- @{class@arcanist:ArcanistFilenameLinter}, which can enforce generally
|
||||
sensible rules about not giving files nonsense names.
|
||||
- @{class@arcanist:ArcanistLicenseLinter}, which can make sure license
|
||||
headers are applied to all source files.
|
||||
- @{class@arcanist:ArcanistNoLintLinter}, which can disable lint for files
|
||||
marked unlintable.
|
||||
- @{class@arcanist:ArcanistGeneratedLinter}, which can disable lint for
|
||||
generated files.
|
||||
- @{class@arcanist:ArcanistMergeConflictLinter}, which detects unresolved
|
||||
merge conflicts.
|
||||
Here, the key ("sample") is a human-readable label identifying the linter. It
|
||||
does not affect linter behavior, so just choose something that makes sense to
|
||||
you.
|
||||
|
||||
Finally, Arcanist has special-purpose linters:
|
||||
The `type` specifies which linter to run. Use `arc linters` to find the names of
|
||||
the available linters.
|
||||
|
||||
- @{class@arcanist:ArcanistXHPASTLinter}, the PHP linter used by Phabricator
|
||||
itself. This linter is powerful, but somewhat rigid (it enforces phutil
|
||||
rules and isn't very configurable for other rulesets).
|
||||
- @{class@arcanist:ArcanistPhutilLibraryLinter}, which enforces phutil library
|
||||
layout rules.
|
||||
**Including and Excluding Files**: By default, a linter will run on every file.
|
||||
This is appropriate for some linters (like the Filename linter), but normally
|
||||
you only want to run a linter like **pep8** on Python files. To include or
|
||||
exclude files, use `include` and `exclude`:
|
||||
|
||||
You can add support for new linters in three ways:
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8",
|
||||
"include": "(\\.py$)",
|
||||
"exclude": "(^third-party/)"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- write new bindings and contribute them to the upstream;
|
||||
- write new bindings and install them alongside Arcanist; or
|
||||
- use a generic binding like @{class@arcanist:ArcanistScriptAndRegexLinter}
|
||||
and drive the integration through configuration.
|
||||
The `include` key is a regular expression (or list of regular expressions)
|
||||
identifying paths the linter should be run on, while `exclude` is a regular
|
||||
expression (or list of regular expressions) identifying paths which it should
|
||||
not run on.
|
||||
|
||||
= Using Lint to Improve Code Review =
|
||||
Thus, this configures a **pep8** linter named "sample" which will run on files
|
||||
ending in ".py", unless they are inside the "third-party/" directory.
|
||||
|
||||
In these examples, regular expressions are written in this style:
|
||||
|
||||
"(example/path)"
|
||||
|
||||
They can be specified with any delimiters, but using `(` and `)` means you don't
|
||||
have to escape slashes in the expression, so it may be more convenient to
|
||||
specify them like this. If you prefer, these are all equivalent:
|
||||
|
||||
"(example/path)i"
|
||||
"/example\\/path/i"
|
||||
"@example/path@i"
|
||||
|
||||
You can also exclude files globally, so no linters run on them at all. Do this
|
||||
by specifying `exclude` at top level:
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"exclude": "(^tests/data/)",
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8",
|
||||
"include": "(\\.py$)",
|
||||
"exclude": "(^third-party/)"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Here, the addition of a global `exclude` rule means no linter will be run on
|
||||
files in "tests/data/".
|
||||
|
||||
**Running Multiple Linters**: Often, you will want to run several different
|
||||
linters. Perhaps your project has a mixture of Python and Javascript code, or
|
||||
you have some PHP and some JSON files. To run multiple linters, just list
|
||||
them in the `linters` map:
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"jshint": {
|
||||
"type": "jshint",
|
||||
"include": "(\\.js$)"
|
||||
},
|
||||
"xml": {
|
||||
"type": "xml",
|
||||
"include": "(\\.xml$)"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This will run JSHint on `.js` files, and SimpleXML on `.xml` files.
|
||||
|
||||
**Adjusting Message Severities**: Arcanist raises lint messages at various
|
||||
severities. Each message has a different severity: for example, lint might
|
||||
find a syntax error and raise an `error` about it, and find trailing whitespace
|
||||
and raise a `warning` about it.
|
||||
|
||||
Normally, you will be presented with lint messages as you are sending code for
|
||||
review. In that context, the severities behave like this:
|
||||
|
||||
- `error` When a file contains lint errors, they are always reported. These
|
||||
are intended to be severe problems, like a syntax error. Unresoved lint
|
||||
errors require you to confirm that you want to continue.
|
||||
- `warning` When a file contains warnings, they are reported by default only
|
||||
if they appear on lines that you have changed. They are intended to be
|
||||
minor problems, like unconventional whitespace. Unresolved lint warnings
|
||||
require you to confirm that you want to continue.
|
||||
- `autofix` This level is like `warning`, but if the message includes patches
|
||||
they will be applied automatically without prompting.
|
||||
- `advice` Like warnings, these messages are only reported on changed lines.
|
||||
They are intended to be very minor issues which may merit a note, like a
|
||||
"TODO" comment. They do not require confirmation.
|
||||
- `disabled` This level suppresses messages. They are not displayed. You can
|
||||
use this to turn off a message if you don't care about the issue it
|
||||
detects.
|
||||
|
||||
By default, Arcanist tries to select reasonable severities for each message.
|
||||
However, you may want to make a message more or less severe, or disable it
|
||||
entirely.
|
||||
|
||||
For many linters, you can do this by providing a `severity` map:
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8",
|
||||
"severity": {
|
||||
"E221": "disabled",
|
||||
"E401": "warning"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Here, the lint message "E221" (which is "multiple spaces before operator") is
|
||||
disabled, so it won't be shown. The message "E401" (which is "multiple imports
|
||||
on one line") is set to "warning" severity.
|
||||
|
||||
If you want to remap a large number of messages, you can use `severity.rules`
|
||||
and specify regular expressions:
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8",
|
||||
"severity.rules": {
|
||||
"(^E)": "warning",
|
||||
"(^W)": "advice"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This adjusts the severity of all "E" codes to "warning", and all "W" codes to
|
||||
"advice".
|
||||
|
||||
**Locating Binaries and Interpreters**: Normally, Arcanist expects to find
|
||||
external linters (like `pep8`) in `$PATH`, and be able to run them without any
|
||||
special qualifiers. That is, it will run a command similar to:
|
||||
|
||||
$ pep8 example.py
|
||||
|
||||
If you want to use a different copy of a linter binary, or invoke it in an
|
||||
explicit way, you can use `interpreter` and `bin`. These accept strings (or
|
||||
lists of strings) identifying places to look for linters. For example:
|
||||
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"sample": {
|
||||
"type": "pep8",
|
||||
"interpreter": ["python2.6", "python"],
|
||||
"bin": ["/usr/local/bin/pep8-1.5.6", "/usr/local/bin/pep8"]
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
When configured like this, `arc` will walk the `interpreter` list to find an
|
||||
available interpreter, then walk the `bin` list to find an available binary.
|
||||
If it can locate an appropriate interpreter and binary, it will execute those
|
||||
instead of the defaults. For example, this might cause it to execute a command
|
||||
similar to:
|
||||
|
||||
$ python2.6 /usr/local/bin/pep8-1.5.6 example.py
|
||||
|
||||
**Additional Options**: Some linters support additional options to configure
|
||||
their behavior. You can run this command get a list of these options and
|
||||
descriptions of what they do and how to configure them:
|
||||
|
||||
$ arc linters --verbose
|
||||
|
||||
This will show the available options for each linter in detail.
|
||||
|
||||
**Running Different Rules on Different Files**: Sometimes, you may want to
|
||||
run the same linter with different rulesets on different files. To do this,
|
||||
create two copies of the linter and just give them different keys in the
|
||||
`linters` map:
|
||||
|
||||
```lang=js
|
||||
{
|
||||
"linters": {
|
||||
"pep8-relaxed": {
|
||||
"type": "pep8",
|
||||
"include": "(^legacy/.*\\.py$)",
|
||||
"severity.rules": {
|
||||
"(.*)": "advice"
|
||||
}
|
||||
},
|
||||
"pep8-normal": {
|
||||
"type": "pep8",
|
||||
"include": "(\\.py$)",
|
||||
"exclude": "(^legacy/)"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This example will run a relaxed version of the linter (which raises every
|
||||
message as advice) on Python files in "legacy/", and a normal version everywhere
|
||||
else.
|
||||
|
||||
**Example .arclint Files**: You can find a collection of example files in
|
||||
`arcanist/resources/arclint/` to use as a starting point or refer to while
|
||||
configuring your own `.arclint` file.
|
||||
|
||||
Advanced Configuration: Lint Engines
|
||||
====================================
|
||||
|
||||
If you need to specify how linters execute in greater detail than is possible
|
||||
with `.arclint`, you can write a lint engine in PHP to extend Arcanist. This is
|
||||
an uncommon, advanced use case. The remainder of this section overviews how the
|
||||
lint internals work, and discusses how to extend Arcanist with a custom lint
|
||||
engine. If your needs are met by `.arclint`, you can skip to the next section
|
||||
of this document.
|
||||
|
||||
The lint pipeline has two major components: linters and lint engines.
|
||||
|
||||
**Linters** are programs which detect problems in a source file. Usually a
|
||||
linter is an external script, which Arcanist runs and passes a path to, like
|
||||
`jshint` or `pep8`.
|
||||
|
||||
The script emits some messages, and Arcanist parses the output into structured
|
||||
errors. A piece of glue code (like @{class@arcanist:ArcanistJSHintLinter} or
|
||||
@{class@arcanist:ArcanistPEP8Linter}) handles calling the external script and
|
||||
interpreting its output.
|
||||
|
||||
**Lint engines** coordinate linters, and decide which linters should run on
|
||||
which files. For instance, you might want to run `jshint` on all your `.js`
|
||||
files, and `pep8.py` on all your `.py` files. And you might not want to lint
|
||||
anything in `externals/` or `third-party/`, and maybe there are other files
|
||||
which you want to exclude or apply special rules for.
|
||||
|
||||
By default, Arcanist uses the
|
||||
@{class@arcanist:ArcanistConfigurationDrivenLintEngine} engine if there is
|
||||
an `.arclint` file present in the working copy. This engine reads the `.arclint`
|
||||
file and uses it to decide which linters should be run on which paths. If no
|
||||
`.arclint` is present, Arcanist does not select an engine by default.
|
||||
|
||||
You can write a custom lint engine instead, which can make a more powerful set
|
||||
of decisions about which linters to run on which paths. For instructions on
|
||||
writing a custom lint engine, see @{article:Arcanist User Guide: Customizing
|
||||
Lint, Unit Tests and Workflows} and @{class@arcanist:ExampleLintEngine}.
|
||||
|
||||
To name an alternate lint engine, set `lint.engine` in your `.arcconfig` to the
|
||||
name of a class which extends @{class@arcanist:ArcanistLintEngine}. For more
|
||||
information on `.arcconfig`, see @{article:Arcanist User Guide: Configuring a
|
||||
New Project}.
|
||||
|
||||
You can also set a default lint engine by setting `lint.engine` in your global
|
||||
user config with `arc set-config lint.engine`, or specify one explicitly with
|
||||
`arc lint --engine <engine>`. This can be useful for testing.
|
||||
|
||||
There are several other engines bundled with Arcanist, but they are primarily
|
||||
predate `.arclint` and are effectively obsolete.
|
||||
|
||||
|
||||
Using Lint to Improve Code Review
|
||||
=================================
|
||||
|
||||
Code review is most valuable when it's about the big ideas in a change. It is
|
||||
substantially less valuable when it devolves into nitpicking over style,
|
||||
|
@ -140,7 +376,8 @@ especially helpful to new authors, and has the overall effect of pushing
|
|||
discussion away from stylistic nitpicks and toward useful examination of large
|
||||
ideas.
|
||||
|
||||
It can also provide a straightforward solution to arguments about style:
|
||||
It can also provide a straightforward solution to arguments about style, if you
|
||||
adopt a policy like this:
|
||||
|
||||
- If a rule is important enough that it should be enforced, the proponent must
|
||||
add it to lint so it is automatically detected or fixed in the future and
|
||||
|
@ -151,7 +388,9 @@ It can also provide a straightforward solution to arguments about style:
|
|||
This may or may not be an appropriate methodology to adopt at your organization,
|
||||
but it generally puts the incentives in the right places.
|
||||
|
||||
= Philosophy of Lint =
|
||||
|
||||
Philosophy of Lint
|
||||
==================
|
||||
|
||||
Some general thoughts on how to develop lint effectively, based on building
|
||||
lint tools at Facebook:
|
||||
|
@ -168,87 +407,6 @@ lint tools at Facebook:
|
|||
valuable to have the linter not only say "the convention is to put a space
|
||||
after comma in a function call" but to fix it for you.
|
||||
|
||||
|
||||
= Configuring Lint =
|
||||
|
||||
Most built in linters can be setup by creating a file named `.arclint` in the
|
||||
workspace root, which is read by
|
||||
@{class@arcanist:ArcanistConfigurationDrivenLintEngine}.
|
||||
|
||||
It's a JSON file, which should look something like:
|
||||
{
|
||||
"linters" : {
|
||||
"sample" : {
|
||||
"type" : "pep8",
|
||||
"include" : "(\\.py$)",
|
||||
"exclude" : "(^exclude.py)",
|
||||
"severity" : {
|
||||
"E401" : "warning"
|
||||
}
|
||||
}
|
||||
}
|
||||
Where:
|
||||
- The key (`sample`) is only used for reporting,
|
||||
- `type` (`pep8`) is used to find the right linter,
|
||||
- `include` and `exclude` specify paths to run linter on, and
|
||||
- `severity` is specified by the linter implementation; Other linters may
|
||||
define other configurable values.
|
||||
|
||||
You may specify many linters in a single `.arclint` file; For an example, see
|
||||
[[https://github.com/epriestley/arclint-examples/ | the arclint-examples
|
||||
repository]].
|
||||
|
||||
Currently available linter types are: `csharp`, `filename`, `csslint`,
|
||||
`flake8`, `jshint`, `jsonlint`, `lessc`, `pep8`, `phpcs`, `puppet-lint`,
|
||||
`pyflakes`, `ruby`, `generated`, `nolint`, `script-and-regex`,
|
||||
`spelling`, `text`, `xml`.
|
||||
|
||||
= Legacy Setups: Using arcconfig =
|
||||
|
||||
Integration with linters that do not (yet) fully support `.arclint` involves two
|
||||
major components: linters and lint engines.
|
||||
|
||||
Linters themselves are programs which detect problems in a source file. Usually
|
||||
a linter is an external script, which Arcanist runs and passes a path to, like
|
||||
`jshint` or `pep8.py`. The script emits some messages, and Arcanist parses
|
||||
the output into structured errors. A piece of glue code (like
|
||||
@{class@arcanist:ArcanistJSHintLinter} or
|
||||
@{class@arcanist:ArcanistPEP8Linter}) handles calling the external script and
|
||||
interpreting its output.
|
||||
|
||||
Lint engines coordinate linters, and decide which linters should run on which
|
||||
files. For instance, you might want to run `jshint` on all your `.js` files,
|
||||
and `pep8.py` on all your `.py` files. And you might not want to lint anything
|
||||
in `externals/` or `third-party/`, and maybe there are other files which you
|
||||
want to exclude or apply special rules for.
|
||||
|
||||
To configure arc for lint, you specify the name of a lint engine, and possibly
|
||||
provide some additional configuration. To name a lint engine, set `lint.engine`
|
||||
in your `.arcconfig` to the name of a class which extends
|
||||
@{class@arcanist:ArcanistLintEngine}. For more information on `.arcconfig`, see
|
||||
@{article:Arcanist User Guide: Configuring a New Project}.
|
||||
|
||||
You can also set a default lint engine by setting `lint.engine` in your global
|
||||
user config with `arc set-config lint.engine`, or specify one explicitly with
|
||||
`arc lint --engine <engine>`.
|
||||
|
||||
The available engines are:
|
||||
|
||||
- @{class@arcanist:ComprehensiveLintEngine}, which runs a wide array of
|
||||
linters on many types of files. This is probably of limited use in any real
|
||||
project because it is overbroad, but is a good starting point for getting
|
||||
lint doing things.
|
||||
- @{class@arcanist:ArcanistSingleLintEngine}, which runs a single linter on
|
||||
every file unconditionally. This can be used with a glue linter like
|
||||
@{class@arcanist:ArcanistScriptAndRegexLinter} to put engine logic in an
|
||||
external script.
|
||||
- A custom engine you write. For most projects, lint rules are sufficiently
|
||||
specialized that this is the best option. For instructions on writing a
|
||||
custom lint engine, see
|
||||
@{article:Arcanist User Guide: Customizing Lint, Unit Tests and Workflows}
|
||||
and @{class@arcanist:ExampleLintEngine}.
|
||||
|
||||
|
||||
= Next Steps =
|
||||
|
||||
Continue by:
|
||||
|
|
Loading…
Reference in a new issue