Bug 42470: Add lint check to CI

Merge Info

Related Issues

• #42470 (closed)

Backporting

Timeline

• Immediate: patchset needed as soon as possible
• Next Minor Stable Release: patchset that needs to be verified in nightly before backport
• Eventually: patchset that needs to be verified in alpha before backport
• No Backport (preferred): patchset for the next major stable

Merging

• Merge to tor-browser - !fixups to tor-browser-specific commits, new features, security backports
• Merge to base-browser - !fixups to base-browser-specific commits, new features to be shared with mullvad-browser, and security backports
  • Add CI for Base Browser Will be cherry-picekd into the base-browser branch.

Issue Tracking

• Link resolved issues with appropriate Release Prep issue for changelog generation

Review

Request Reviewer

/cc @henry @clairehurst @dan @boklm @ma1 @jwilde @pierov @richard

CCing everyone, because this might affect everyone's workflow.

Note for reviewers: There is a bunch of lint fixes whcih makes it weird to review. I have separated it in commits in order to try and make reviewing a bit easier.

Change Description

I have implemented a stage in Gitlab CI to execute most of the mozilla-central linters. Most of these can be listed with ./mach lint --list the only exception being clang-tidy.

The way I implemented the CI jobs, they will run for every pull request depending on types of files changed i.e. only when JS files are changed in a MR does the eslint job run. To speed things up the linter also only runs for the files touched by the MR unless the MR has more than 50 commits, in which case the linters run for the full repository -- think the ESR rebase MR. This does take longer, but it's no an unreasonable amount of time and should be unusual, so I am not too worried.

Whenever a commit is made directly to the repository default branch all linters run for the full repository. Like I mentioned before, running all linters for the whole repository sounds scary, but it really doesn't take that long. Should be less than 10min, which is the time I got when doing this locally. Things are going to get funkier once we start building on CI :D

• ❌ android-api-lint Android lints were not added in this PR. Will do in a follow up.
• ❌ android-checkstyle
• ❌ android-format
• ❌ android-javadoc
• ❌ android-lint
• ❌ android-test
• ✔️ black
• ✔️ clang-format
• ❌ clang-tidy Will do in a follow up.
• ❌ clippy I was not able to get this to run properly, it seems even in Mozilla's CI the output is full of errors.. Anyways I didn't want to hold this whole patch on clippy. Can do in a follow up.
• ❌ codespell Too many false positives... We can do it in a follow up, but I wouldn't prioritize it too highly.
• ✔️ eslint
• ❌ file-perm Too specific, didn't seem worth it to add it. FWIW, this is quick to run and to add -- there are failing no lints.
• ✔️ file-whitespace
• ✔️ fluent-lint
• ✔️ l10n
• ❌ license This is autofixable, quick and easy to add. It would result in a lot of changes. It is not clear to me if we need it.
• ❌ lintpref This checks for prefs being overwritten. IIUC we do that deliberately, so this lint doesn't make sense for us.
• ✔️ mingw-capitalization
• ✔️ mscom-init
• ❌ perfdocs Too specific, not adding it.
• ❌ rejected-words Didn't seem to add much value. We are a small enough team to just call this out if it happens.
• ❌ rst We have literally no changes to rst files in our own commits, made no sense to add this.
• ✔️ ruff
• ✔️ rustfmt
• ✔️ shellcheck
• ✔️ stylelint
• ✔️ test-manifest-alpha
• ✔️ test-manifest-disable
• ✔️ test-manifest-skip-if
• ✔️ trojan-source
• ❌ updatebot This is specific to Mozilla internal software vendoring process, we don't need it.
• ❌ wpt Too specific, not adding it.
• ✔️ yaml

How Tested

No manual testing required. If the CI passes, that is enough.