Some linters include source code as context, often with little ascii
arrows that point at the problem. This can be invaluable for figuring
out where the problem is.
We have an attribute to store this information in the `Issue` class, but
unfortunately none of the mozlint formatters display it!
This gets the default formatter to attempt displaying the source, with
some basic colorization. The colorization is dependent on the format of
each linter, so it may need to be adjusted as we add different linters.
Differential Revision: https://phabricator.services.mozilla.com/D257531
Make clippy a global linter. This means it only runs once per
workspace, which is good because `cargo` handles parallelization
internally. There's no reason to split things out into multiple commands.
Updated the global linter config handling to pass a list of files. This
is needed for clippy and we can just ignore them for the android lints.
Updated the clippy linter to work with crates outside of `gkrust`. For
these crates, we call `cargo clippy` directly and does not filter by
path name. These crates need to be listed in `tools/lint/clippy.yml`,
currently this is only `uniffi-bindgen-gecko-js`, but it's easy to add
more.
Fixed some lint warnings in the `mozlint` package itself.
Differential Revision: https://phabricator.services.mozilla.com/D246488
The pipelint tool was not working correctly on Windows for three reasons:
1. The module was being imported into every child process spawned by the
lint roller's multiprocessing executor. Because the import of
pipelint has side-effects (the actual running of mach lint on the
input), each child process would end up recursively triggering these
side-effects and causing exponential process growth until they all
eventually crashed.
To address this we now have the logic of the pipelint script behind a
`if __name__ == "__main__" guard.
2. We were keeping the file handle open before we called into mach,
which resulted in a permissions error trying to read from the file
handle after mach had finished.
To address this we now immediately close the file before invoking
`mach lint` and re-open it for reading after.
3. We were using default newline handling inside `pipelint` and
`mozlint`, which resulted in LFs being translated to CRLFs
automatically in all the intermediate temporary files.
To address this, we open files for reading and writing in binary mode
so that we deal directly with the underlying bytes instead of
particular interpretations of bytes (e.g., newlines, CP252 vs UTF-8),
including `stdin` (when reading file input from `jj`) and `stdout`
(when outputting the final formatted files).
Differential Revision: https://phabricator.services.mozilla.com/D244513
This allows `./mach lint` to be used with integrations that expect to
pass in file contents on stdin and get a fixed version of the file on
stdout.
Differential Revision: https://phabricator.services.mozilla.com/D236224
This ensures consistency in output between the parent and subprocess. It
also adjusts the formatting by log level, so we only get the more
detailed format when `--verbose` is enabled.
Differential Revision: https://phabricator.services.mozilla.com/D236252
Under the old logic, if a linter has a file directly in its include config, e.g:
```
include: [path/to/some/file.py]
```
Then it would always get linted, even if the user never specified it on
the cli. This is a bug.
Differential Revision: https://phabricator.services.mozilla.com/D236251
Changing a linter's `support-files` causes that linter to
run against the entire tree. This is to avoid introducing
a regression in files that weren't touched, which would
be confusing for developers who trusted `--outgoing` to
catch their regressions.
Turn this behaviour off when `--fix` is passed as a linter
argument so these regressions aren't also fixed tree-wide.
Move the logic for tree-wide linting into a function
`should_lint_entire_tree` and add tests for the behaviour
in `test_roller.py`.
Differential Revision: https://phabricator.services.mozilla.com/D164761
The new `mozlint` behaviour will always lean on the VCS
to only lint changed files.
However, in tests, the VCS is going to be slow and also provide
inconsistent data between tests. Additionally, in some contexts,
the VCS won't be available.
Resolve this by hardcoding a file to lint for each case.
Differential Revision: https://phabricator.services.mozilla.com/D138033
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
In the process, fixed a few bugs:
- the template eg `{file_adds % "\n{file}"}` produced a leading blank line,
which led to everything being linted.
- 'd' was replaced with 'r' in diff_filters, but the replacement was discarded.
- as a result of the above, `hg status -d` was being used ("show only deleted (but tracked) files") and now it is `hg status -r` ("show only removed files"). I *think* this is what was intended?
Differential Revision: https://phabricator.services.mozilla.com/D66714
Adds --config-paths parameter to mach mozlint where test configuration
files (.yml) can be located. The default path "tools/lint" is set automatically
and additional paths have preference over the default.
This allows Thunderbird to run mozlint-based tests in Taskcluster with a
different configuration.
Differential Revision: https://phabricator.services.mozilla.com/D67077
While we validate user specified paths early and only consider the 'AM' (added
+ modified) diff filter from versioncontrol, it's still possible to get
non-existent files when using --outgoing. This situation can happen when you
have a commit series that first modifies a file in an earlier commit, and then
moves/deletes it in a later commit.
To get around this, just silently filter out any file paths that don't exist.
Differential Revision: https://phabricator.services.mozilla.com/D67411
This was happening because we had two exclude paths that were run through the 'collapse' algorithm:
editor/libeditor/tests/browserscope/lib/richtext
editor/libeditor/tests/browserscope/lib/richtext2
The problem was that in order to determine the base directory, the algorithm called
'os.path.commonprefix'. This function just returns the common string prefix, which
is the '.../lib/richtext' path. Even though in the above, the base *should* have
been '.../lib'.
To fix the problem, we add a check to ensure the computed base doesn't have any
sibling directories with the same prefix. If there are, it means 'commonprefix' was
too greedy.
Differential Revision: https://phabricator.services.mozilla.com/D60173
Arnaud Bienner