1361341 - Add rust clippy to mozlint

Status: RESOLVED FIXED

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

Description

[Bob Clary [:bc] (inactive)]

Track work necessary to get in tree Rust linting implmented.

Comment 1

[Andrew Halberstadt [:ahal]]

Just wanted to mention that I had previously looked into this, but using rustfmt (with the checkstyle option) instead:
https://github.com/rust-lang-nursery/rustfmt

But if the rust devs prefer clippy, we should definitely go with that. The only thing I'd ask is to consider somehow integrating this with mozlint:
http://gecko.readthedocs.io/en/latest/tools/lint/index.html

While rustfmt would be easy to integrate with mozlint, clippy would be trickier as it is a compiler extension. So I'm not 100% sure if integration makes sense in this case, or if we should make a brand new thing. And even if mozlint isn't the right fit for this, we could still re-use the existing |mach lint| mach command. Anyway, just something to consider.

I'm not back until the 23rd, but I'd be happy to discuss/answer questions either here or via e-mail until then.

Comment 2

[Andrew Halberstadt [:ahal]]

Doing some bug cleanup.

Once we have clippy support for rust stable, I don't think this will need more than one bug to complete, so I'm just turning this into the bug that will implement clippy as a mozlint linter. Doing this will also automatically provide the mach command, so would make bug 1361343 invalid.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

clippy cannot run on a single file. it needs to build the file
It can be built using:
cargo clippy --manifest-path js/src/wasm/cranelift/Cargo.toml

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Add rust clippy to mozlint

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Add clippy in the mozlint docker image

Depends on D58248

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Add rust clippy to mozlint r?#linter-reviewers

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Add clippy in the mozlint docker image

Depends on D58250

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Run clippy in taskcluster

Depends on D58251

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - TC: Create a new lint/rust category

Depends on D58252

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1578058 - Ride along: move the mozlint task in the file with the same name

Otherwise, cargo isn't installed

Depends on D58267

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Create the list of supported/unsupported directory with clippy r?#linter-reviewers

Depends on D58250

Comment 12

[Phabricator Automation]

Comment on attachment 9117786 [details]
Bug 1361341 - Add clippy in the mozlint docker image

Revision D58251 was moved to bug 1578058. Setting attachment 9117786 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9117787 [details]
Bug 1361341 - Run clippy in taskcluster

Revision D58252 was moved to bug 1578058. Setting attachment 9117787 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9117813 [details]
Bug 1361341 - TC: Create a new lint/rust category

Revision D58267 was moved to bug 1578058. Setting attachment 9117813 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9117814 [details]
Bug 1578058 - Ride along: move the mozlint task in the file with the same name

Revision D58268 was moved to bug 1578058. Setting attachment 9117814 [details] to obsolete.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/066a58eea385
Add rust clippy to mozlint r=ahal
https://hg.mozilla.org/integration/autoland/rev/76d91acac82c
Create the list of supported/unsupported directory with clippy r=ahal

Comment 17

[Cristina Coroiu [:ccoroiu]]

Backed out 4 changesets (bug 1361341, bug 1578058) for mozilint failure at test_clippy.py

Backout: https://hg.mozilla.org/integration/autoland/rev/f72cd435a4cfb1dce6714580f6f6f4d622b7f717

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=76d91acac82cab17165f838fec297ab2cb1936b0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=288821237&repo=autoland&lineNumber=559

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a63ccd650889
Add rust clippy to mozlint r=ahal

Comment 19

[Andrei Ciure[:aciure]]

Backed out changeset a63ccd650889 (bug 1361341) for causing mozlint failures
https://hg.mozilla.org/integration/autoland/rev/6f4f37881859373b597115ab621fff145b589d14

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=288888131&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=a63ccd65088995da39af4189d76972d95757e660

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1361341 - Move the mozlint job into the mozlint.yml job r?#linter-reviewers

Instead of python.yml
Otherwise, it is getting the incorrect docker image (without rust).
And it makes sense that the mozlint job is next to the other tasks

Depends on D58293

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1131a9b0f8c0
Add rust clippy to mozlint r=ahal
https://hg.mozilla.org/integration/autoland/rev/2a33639bf079
Create the list of supported/unsupported directory with clippy r=ahal

Comment 22

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/1131a9b0f8c0
https://hg.mozilla.org/mozilla-central/rev/2a33639bf079

Comment 23

[Sylvestre Ledru [:Sylvestre]]

It landed!