1433410 - (codespell) Add codespell support for mach lint

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Sylvestre Ledru [:Sylvestre]]

I am trying to integrate this tool: https://github.com/lucasdemarchi/codespell/

Example:
./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 https://github.com/lucasdemarchi/codespell/issues/314
I can ignore the warning but cannot ignore the autofix as it would require to patch codespell itself.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1433410 - Add codespell support for mach lint

Review commit: https://reviewboard.mozilla.org/r/215852/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/215852/

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8945745 [details]
Bug 1433410 - Add codespell support for mach lint

https://reviewboard.mozilla.org/r/215852/#review221754

::: 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/__init__.py:94
(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/__init__.py:103
(Diff revision 1)
> +        return []
> +
> +    config['root'] = lintargs['root']
> +    cmd_args = [binary,
> +                '--disable-colors',
> +                # Silent some warnings:

nit: Silence

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8945745 [details]
Bug 1433410 - Add codespell support for mach lint

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215852/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e414096f1c3b
Add codespell support for mach lint r=ahal

Comment 5

[Stefan Hindli [:stefan_hindli]]

Backed out changeset e414096f1c3b (bug 1433410) for yaml lint failures in tools/lint/codespell.yml:12:81 on a CLOSED TREE

https://hg.mozilla.org/integration/autoland/rev/e414096f1c3b8fe55c1da6344c87c2df26e28b8c
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=01e0ee57f2f9e3334ab31142d07349784fbfaf40&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8945745 [details]
Bug 1433410 - Add codespell support for mach lint

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/215852/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3eefd4e1087c
Add codespell support for mach lint r=ahal

Comment 8

[Sylvestre Ledru [:Sylvestre]]

I reported https://github.com/mozilla-releng/services/issues/796 about comment #5

Comment 9

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/3eefd4e1087c

Comment 10

[:Gijs (he/him)]

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`.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Sure but, before, I would like to have it running at review phase on all patches which aren't touching thirdparty code.

Comment 12

[:Gijs (he/him)]

(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...

Comment 13

[Andrew Halberstadt [:ahal]]

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.