[ci] enable clippy

[gnzlbg]

Closes #110 .

[BurntSushi]

I'm not sure that I want this, primarily because it targets Rust's unstable plugin API and is therefore prone to breakage. Case in point, this PR is failing because Clippy doesn't compile. I personally will get pretty annoyed pretty quickly if we can't merge PRs because of this.

I also generally feel like most of the code in this crate is pretty simplistic, so the bang-to-buck ratio here isn't that great IMO.

[gnzlbg]

I'm not sure that I want this, primarily because it targets Rust's unstable plugin API and is therefore prone to breakage. Case in point, this PR is failing because Clippy doesn't compile. I personally will get pretty annoyed pretty quickly if we can't merge PRs because of this.

Clippy is currently broken on nightly. I've added cargo clippy as the last test to be able to manually check if all other tests pass when clippy breaks. This should get better when clippy works with stable.

An alternative could be to add some extra builds to travis and appveyor to just run clippy. That way it is easy to see if clippy is broken, and we can still merge PRs.

I also generally feel like most of the code in this crate is pretty simplistic, so the bang-to-buck ratio here isn't that great IMO.

I've fixed some clippy errors locally but if I commit them would like to have somehow clippy enabled as a sanity test.

[alexcrichton]

I'm personally not a huge fan and I feel like an "essentially auto generated" crate such as this probably won't benefit too much?

[gnzlbg]

So I've fixed all the clippy warnings that I considered relevant, that is, some shifts by zero, a duplicated function in the runtime that felt through the code-review cracks, etc.

I've silenced all of the lints that do not apply to this library, but I've also silenced one lint that I cannot fix right now but we should probably improve on (missing_docs_in_private_items).

Now I hope that travis marks this as green and let me know what you think. IMO it did not found a lot of issues, but it found some beginner issues. I think that if this doesn't break CI all the time (I've put it in an allow_failure build) maybe we can just "try our bests" to keep it green.

[gnzlbg]

@BurntSushi and @alexcrichton so I've documented all missing items and enabled the clippy lint. IMO PR's for intrinsics should require documentation, clippy will complain if this isn't the case.

[alexcrichton]

I think the CLIPPY env var isn't needed here?

[alexcrichton]

Er nvmd, this is needed to distinguish it in allow_failures

[gnzlbg]

yes that's right, I should add a comment, I did the same for the RUSTFMT=On variable, I just left it there so that one can tell the builds appart, maybe there is a better way to name the builds?

[alexcrichton]

I'm personally not a huge fan of all the #[allow] directives, but otherwise seems fine to me!

[gnzlbg]

@alexcrichton me neither, AFAIK there isn't a way to specify which lints are allowed in the clippy.toml file. Not all lints apply to all projects, and I'd rather have them in a clippy.toml than in the lib.rs. Maybe I should file a clippy issue.

[gnzlbg]

@llogiq any tips on how to set up clippy in CI ?

[llogiq]

I assume you are on nightly already. So, you should be able to cargo install clippy -f && cargo clippy || true to allow failures (this means you'll need to look into the logs). You can deactivate lints via -A lint_name, but you need to precede cargo clippy's args with a double dash.

I personally don't think it makes much sense to even let a successful clippy run break the build at the moment. We are still adding more lints, so even when there is no breakage, every new clippy might uncover new warnings.

[gnzlbg]

An advantage of the allow_failure approach is that you immediately see in the build matrix if there is any issue with clippy, while doing it in the way @llogiq proposes we would need to look into the logs of green builds for that. In both cases the travis build remains green if clippy either introduces new lints or cannot build, so we can just try to keep it green on a best effort basis.

[gnzlbg]

@BurntSushi I will update this once the formatting commit is merged, hope we can merge this as well.

[gnzlbg]

@alexcrichton I've rebased this :)

[alexcrichton]

Looks like CI may be failing?

[gnzlbg]

@alexcrichton done!