summaryrefslogtreecommitdiffstats
path: root/docs/code-quality/lint/create.rst
blob: c4d8b15480ac0df317e7907b1666cac1de45b6c6 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
Adding a New Linter to the Tree
===============================

Linter Requirements
-------------------

For a linter to be integrated into the mozilla-central tree, it needs to have:

* Any required dependencies should be installed as part of ``./mach bootstrap``
* A ``./mach lint`` interface
* Running ``./mach lint`` command must pass (note, linters can be disabled for individual directories)
* Taskcluster/Treeherder integration
* In tree documentation (under ``docs/code-quality/lint``) to give a basic summary, links and any other useful information
* Unit tests (under ``tools/lint/test``) to make sure that the linter works as expected and we don't regress.

The review group in Phabricator is ``#linter-reviewers``.

Linter Basics
-------------

A linter is a yaml file with a ``.yml`` extension. Depending on how the type of linter, there may
be python code alongside the definition, pointed to by the 'payload' attribute.

Here's a trivial example:

no-eval.yml

.. code-block:: yaml

    EvalLinter:
        description: Ensures the string eval doesn't show up.
        extensions: ['js']
        type: string
        payload: eval

Now ``no-eval.yml`` gets passed into :func:`LintRoller.read`.


Linter Types
------------

There are four types of linters, though more may be added in the future.

1. string - fails if substring is found
2. regex - fails if regex matches
3. external - fails if a python function returns a non-empty result list
4. structured_log - fails if a mozlog logger emits any lint_error or lint_warning log messages

As seen from the example above, string and regex linters are very easy to create, but they
should be avoided if possible. It is much better to use a context aware linter for the language you
are trying to lint. For example, use eslint to lint JavaScript files, use ruff to lint Python
files, etc.

Which brings us to the third and most interesting type of linter,
external.  External linters call an arbitrary python function which is
responsible for not only running the linter, but ensuring the results
are structured properly. For example, an external type could shell out
to a 3rd party linter, collect the output and format it into a list of
:class:`Issue` objects. The signature for this python
function is ``lint(files, config, **kwargs)``, where ``files`` is a list of
files to lint and ``config`` is the linter definition defined in the ``.yml``
file.

Structured log linters are much like external linters, but suitable
for cases where the linter code is using mozlog and emits
``lint_error`` or ``lint_warning`` logging messages when the lint
fails. This is recommended for writing novel gecko-specific lints. In
this case the signature for lint functions is ``lint(files, config, logger,
**kwargs)``.


Linter Definition
-----------------

Each ``.yml`` file must have at least one linter defined in it. Here are the supported keys:

* description - A brief description of the linter's purpose (required)
* type - One of 'string', 'regex' or 'external' (required)
* payload - The actual linting logic, depends on the type (required)
* include - A list of file paths that will be considered (optional)
* exclude - A list of file paths or glob patterns that must not be matched (optional)
* extensions - A list of file extensions to be considered (optional)
* setup - A function that sets up external dependencies (optional)
* support-files - A list of glob patterns matching configuration files (optional)
* find-dotfiles - If set to ``true``, run on dot files (.*) (optional)
* ignore-case - If set to ``true`` and ``type`` is regex, ignore the case (optional)

In addition to the above, some ``.yml`` files correspond to a single lint rule. For these, the
following additional keys may be specified:

* message - A string to print on infraction (optional)
* hint - A string with a clue on how to fix the infraction (optional)
* rule - An id string for the lint rule (optional)
* level - The severity of the infraction, either 'error' or 'warning' (optional)

For structured_log lints the following additional keys apply:

* logger - A StructuredLog object to use for logging. If not supplied
  one will be created (optional)


Example
-------

Here is an example of an external linter that shells out to the Python ruff linter,
let's call the file ``ruff_lint.py`` (`in-tree version <https://searchfox.org/mozilla-central/source/tools/lint/python/ruff.py>`__):

.. code-block:: python

    import json
    import os
    import subprocess
    from collections import defaultdict
    from shutil import which

    from mozlint import result


    RUFF_NOT_FOUND = """
    Could not find ruff! Install ruff and try again.
    """.strip()


    def lint(paths, config, **lintargs):
        binary = which('ruff')
        if not binary:
            print(RUFF_NOT_FOUND)
            return 1


        cmd = ["ruff", "check", "--force-exclude", "--format=json"] + paths
        output = subprocess.run(cmd, stdout=subprocess.PIPE, env=os.environ).output

        # all passed
        if not output:
            return []

        try:
            issues = json.loads(output)
        except json.JSONDecodeError:
            log.error(f"Could not parse output: {output}")

        results = []
        for issue in issues:
            # convert ruff's format to mozlint's format
            res = {
                "path": issue["filename"],
                "lineno": issue["location"]["row"],
                "column": issue["location"]["column"],
                "lineoffset": issue["end_location"]["row"] - issue["location"]["row"],
                "message": issue["message"],
                "rule": issue["code"],
                "level": "error",
            }

            if issue["fix"]:
                res["hint"] = issue["fix"]["message"]

            results.append(result.from_config(config, **res))

        return {"results": results, "fixed": fixed}

Now here is the linter definition that would call it:

.. code-block:: yaml

    ruff:
        description: Python Linter
        include: ["."]
        extensions: ["py"]
        support-files:
            - "**/.ruff.toml"
            - "**/ruff.toml"
            - "**/pyproject.toml"
        type: external
        payload: py.ruff:lint

Notice the payload has two parts, delimited by ':'. The first is the module
path, which ``mozlint`` will attempt to import. The second is the object path
within that module (e.g, the name of a function to call). It is up to consumers
of ``mozlint`` to ensure the module is in ``sys.path``. Structured log linters
use the same import mechanism.

The ``support-files`` key is used to list configuration files or files related
to the running of the linter itself. If using ``--outgoing`` or ``--workdir``
and one of these files was modified, the entire tree will be linted instead of
just the modified files.

Result definition
-----------------

When generating the list of results, the following values are available.

.. csv-table::
   :header: "Name", "Description", "Optional"
   :widths: 20, 40, 10

    "linter", "Name of the linter that flagged this error", ""
    "path", "Path to the file containing the error", ""
    "message", "Text describing the error", ""
    "lineno", "Line number that contains the error", ""
    "column", "Column containing the error", ""
    "level", "Severity of the error, either 'warning' or 'error' (default 'error')", "Yes"
    "hint", "Suggestion for fixing the error", "Yes"
    "source", "Source code context of the error", "Yes"
    "rule", "Name of the rule that was violated", "Yes"
    "lineoffset", "Denotes an error spans multiple lines, of the form (<lineno offset>, <num lines>)", "Yes"
    "diff", "A diff describing the changes that need to be made to the code", "Yes"


Automated testing
-----------------

Every new checker must have associated tests. If your linter is ``mylinter`` then the
test file should be named ``tools/lint/test/test_mylinter.py`` and any example files
named like ``tools/lint/test/files/mylinter/my-example-file``. Be sure that your test
has been added as a section ``["test_mylinter.py"]`` in the manifest ``tools/lint/test/python.toml``.

They should be pretty easy to write as most of the work is managed by the Mozlint
framework. The key declaration is the ``LINTER`` variable which must match
the linker declaration.

As an example, the `ruff test <https://searchfox.org/mozilla-central/source/tools/lint/test/test_ruff.py>`_ looks like the following snippet:

.. code-block:: python

    import mozunit
    LINTER = 'ruff'

    def test_lint_ruff(lint, paths):
        results = lint(paths('bad.py'))
        assert len(results) == 2
        assert results[0].rule == 'F401'
        assert results[1].rule == 'E501'
        assert results[1].lineno == 5

    if __name__ == '__main__':
        mozunit.main()

As always with tests, please make sure that enough positive and negative cases
are covered.

To run the tests:

.. code-block:: shell

    $ ./mach python-test --subsuite mozlint

To run a specific test:

.. code-block:: shell

    ./mach python-test --subsuite mozlint tools/lint/test/test_black.py

More tests can be `found in-tree <https://searchfox.org/mozilla-central/source/tools/lint/test>`_.

Tracking fixed issues
---------------------

All the linters that provide ``fix support`` returns a dictionary instead of a list.

``{"results":result,"fixed":fixed}``

* results - All the linting errors it was not able to fix
* fixed - Count of fixed errors (for ``fix=False`` this is 0)

Some linters (example: `codespell <https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/tools/lint/spell/__init__.py#145-163>`_) might require two passes to count the number of fixed issues.
Others might just need `some tuning <https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/tools/lint/file-whitespace/__init__.py#28,60,85,112>`_.

For adding tests to check your fixed count, add a global variable ``fixed = 0``
and write a function to add your test as mentioned under ``Automated testing`` section.


Here's an example

.. code-block:: python

    fixed = 0


    def test_lint_codespell_fix(lint, create_temp_file):
    # Typo has been fixed in the contents to avoid triggering warning
    # 'informations' ----> 'information'
        contents = """This is a file with some typos and information.
    But also testing false positive like optin (because this isn't always option)
    or stuff related to our coding style like:
    aparent (aParent).
    but detects mistakes like mozilla
    """.lstrip()

        path = create_temp_file(contents, "ignore.rst")
        lint([path], fix=True)

        assert fixed == 2


Bootstrapping Dependencies
--------------------------

Many linters, especially 3rd party ones, will require a set of dependencies. It
could be as simple as installing a binary from a package manager, or as
complicated as pulling a whole graph of tools, plugins and their dependencies.

Either way, to reduce the burden on users, linters should strive to provide
automated bootstrapping of all their dependencies. To help with this,
``mozlint`` allows linters to define a ``setup`` config, which has the same
path object format as an external payload. For example (`in-tree version <https://searchfox.org/mozilla-central/source/tools/lint/ruff.yml>`__):

.. code-block:: yaml

    ruff:
        description: Python linter
        include: ['.']
        extensions: ['py']
        type: external
        payload: py.ruff:lint
        setup: py.ruff:setup

The setup function takes a single argument, the root of the repository being
linted. In the case of ``ruff``, it might look like:

.. code-block:: python

    import subprocess
    from shutil import which

    def setup(root, **lintargs):
        # This is a simple example. Please look at the actual source for better examples.
        if not which("ruff"):
            subprocess.call(["pip", "install", "ruff"])

The setup function will be called implicitly before running the linter. This
means it should return fast and not produce any output if there is no setup to
be performed.

The setup functions can also be called explicitly by running ``mach lint
--setup``. This will only perform setup and not perform any linting. It is
mainly useful for other tools like ``mach bootstrap`` to call into.


Adding the linter to the CI
---------------------------

First, the job will have to be declared in Taskcluster.

This should be done in the `mozlint Taskcluster configuration <https://searchfox.org/mozilla-central/source/taskcluster/ci/source-test/mozlint.yml>`_.
You will need to define a symbol, how it is executed and on what kind of change.

For example, for ruff, the configuration is the following:

.. code-block:: yaml

    py-ruff:
        description: run ruff over the gecko codebase
        treeherder:
            symbol: py(ruff)
        run:
            mach: lint -l ruff -f treeherder -f json:/builds/worker/mozlint.json .
        when:
            files-changed:
                - '**/*.py'
                - '**/.ruff.toml'

If the linter requires an external program, you will have to install it in the `setup script <https://searchfox.org/mozilla-central/source/taskcluster/docker/lint/system-setup.sh>`_
and maybe install the necessary files in the `Docker configuration <https://searchfox.org/mozilla-central/source/taskcluster/docker/lint/Dockerfile>`_.

.. note::

    If the defect found by the linter is minor, make sure that it is logged as
    a warning by setting `{"level": "warning"}` in the
    :class:`~mozlint.result.Issue`. This means the defect will not cause a
    backout if landed, but will still be surfaced by reviewbot at review time,
    or when using `-W/--warnings` locally.