Closed Bug 1433410 (codespell) Opened 4 years ago Closed 4 years ago

Add codespell support for mach lint


(Firefox Build System :: Lint and Formatting, enhancement)

Not set


(firefox60 fixed)

Tracking Status
firefox60 --- fixed


(Reporter: Sylvestre, Assigned: Sylvestre)


(Depends on 1 open bug, Blocks 1 open bug)



(1 file)

I am trying to integrate this tool:

./mach lint -l codespell tools/lint/docs/linters/eslint-plugin-mozilla.rst tools/lint/eslint/eslint-plugin-mozilla/lib/processors/xbl-bindings.js

For now, --fix isn't managed because of
I can ignore the warning but cannot ignore the autofix as it would require to patch codespell itself.
Blocks: 1433417
Comment on attachment 8945745 [details]
Bug 1433410 - Add codespell support for mach lint

::: tools/lint/codespell.yml:5
(Diff revision 1)
> +---
> +codespell:
> +    description: Check code for common misspellings
> +    include: ['.']
> +    exclude: []

We should at least add `third_party` here. Alternatively we could block on fixing bug 1433522 and bug 1375861 first.

::: tools/lint/spell/
(Diff revision 1)
> +    if not binary:
> +        print(CODESPELL_NOT_FOUND)
> +        if 'MOZ_AUTOMATION' in os.environ:
> +            return 1
> +        return []

This is fine for now because there's precedent.. but at some point we'll need to make bootstrapping these  things easier.

Maybe with a bit of refactoring we can import `mozboot` (the library behind |mach bootstrap|) and re-use the code in there to get things installed.

::: tools/lint/spell/
(Diff revision 1)
> +        return []
> +
> +    config['root'] = lintargs['root']
> +    cmd_args = [binary,
> +                '--disable-colors',
> +                # Silent some warnings:

nit: Silence
Attachment #8945745 - Flags: review?(ahalberstadt) → review+
Pushed by
Add codespell support for mach lint r=ahal
Pushed by
Add codespell support for mach lint r=ahal
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Can we have a dev-platform+fx-dev post about this? I was confused when I suddenly got warning messages about codespell ("Unable to locate codespell, ...") when using `hg commit`.
Flags: needinfo?(sledru)
Sure but, before, I would like to have it running at review phase on all patches which aren't touching thirdparty code.
Flags: needinfo?(sledru)
(In reply to Sylvestre Ledru [:sylvestre] from comment #11)
> Sure but, before, I would like to have it running at review phase on all
> patches which aren't touching thirdparty code.

OK. It's up to you. I was just surprised because things relating to the build and/or linting hooks changed (ie I got error/warning messages that made no sense to me ("You don't have this thing you've never heard of and didn't know you needed, here's how to get it!") without warning beforehand. I would imagine I'm not the only person who's surprised...
Yeah, that's fair. In hindsight I should have ensured that there was some kind of automated bootstrapping before granting review, that was my fault.

We filed bug 1434337 and bug 1434332 to codify the rules for adding new linters to the tree a bit. I'm also in the progress of hooking up lint dependency installation to |mach bootstrap|, and I'll add bootstrapping code for shellcheck and codespell while I'm at it.
Product: Testing → Firefox Build System
Blocks: 1443508
Depends on: 1444048
Depends on: 1445245
Blocks: 1448934
Depends on: 1455086
Depends on: 1523092
Depends on: 1552430
Depends on: 1548485
Depends on: 1552560
Alias: codespell
Depends on: 1553060
Depends on: 1679058
Depends on: 1679053
Depends on: 1717447
You need to log in before you can comment on or make changes to this bug.