Make clippy a git subtree instead of a git submodule #70655

oli-obk · 2020-04-01T12:55:00Z

r? @eddyb

cc #70651

documentation at #70654

eddyb · 2020-04-01T13:12:51Z

Whoa, clippy commit hashes were kept intact, this makes me really happy!11

eddyb · 2020-04-01T13:26:02Z

r? @Mark-Simulacrum for the src/bootstrap changes

Mark-Simulacrum · 2020-04-01T14:08:51Z

bootstrap changes look good (presuming they work, though the mingw-check job should confirm that clippy check works -- and it seems to have passed).

flip1995 · 2020-04-01T15:15:51Z

Does this also enable running (at least UI) tests for Clippy on every PR?

oli-obk · 2020-04-02T08:07:23Z

Does this also enable running (at least UI) tests for Clippy on every PR?

yes. We already run these right now on every PR, but we used to ignore it if clippy failed to successfully run its tests.

bors · 2020-04-05T19:43:20Z

☔ The latest upstream changes (presumably #70809) made this pull request unmergeable. Please resolve the merge conflicts.

@ghost

Rollup of 11 pull requests Successful merges: - rust-lang#5406 (Fix update_lints) - rust-lang#5409 (Downgrade let_unit_value to pedantic) - rust-lang#5410 (Downgrade trivially_copy_pass_by_ref to pedantic) - rust-lang#5412 (Downgrade inefficient_to_string to pedantic) - rust-lang#5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`) - rust-lang#5417 (Update doc links and mentioned names in docs) - rust-lang#5419 (Downgrade unreadable_literal to pedantic) - rust-lang#5420 (Downgrade new_ret_no_self to pedantic) - rust-lang#5422 (CONTRIBUTING.md: fix broken triage link) - rust-lang#5424 (Incorrect suspicious_op_assign_impl) - rust-lang#5425 (Ehance opt_as_ref_deref lint.) Failed merges: - rust-lang#5345 (Add lint for float in array comparison) - rust-lang#5411 (Downgrade implicit_hasher to pedantic) - rust-lang#5428 (Move cognitive_complexity to nursery) r? @ghost changelog: rollup

When checking for functions that are potential candidates for trait implementations check the function header to make sure modifiers like asyncness, constness and safety match before triggering the lint. Fixes rust-lang#5413, rust-lang#4290

Move cognitive_complexity to nursery As discussed in rust-lang/rust-clippy#5418 (comment); Clippy's current understanding of cognitive complexity is not good enough yet at analyzing code for understandability to have this lint be enabled by default. changelog: Remove cognitive_complexity from default set of enabled lints

Check fn header along with decl when suggesting to implement trait When checking for functions that are potential candidates for trait implementations check the function header to make sure modifiers like asyncness, constness and safety match before triggering the lint. Fixes rust-lang#5413, rust-lang#4290 changelog: check fn header along with decl for should_implement_trait

Downgrade implicit_hasher to pedantic From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher), this lint is intended to suggest: ```diff - pub fn foo(map: &mut HashMap<i32, i32>) { } + pub fn foo<S: BuildHasher>(map: &mut HashMap<i32, i32, S>) { } ``` I think this is pedantic. I get that this lint can benefit core libraries like serde, but that's exactly the use case for pedantic lints; a library like serde will [enable clippy_pedantic](https://github.com/serde-rs/json/blob/fd6741f4b0b3fc90a58a6f578e33a9adc6403f3f/src/lib.rs#L304) and take the time to go through everything possible. Similar for libraries doing a libz blitz style checkup before committing to a 1.0 release; it would make sense to run through all the available pedantic lints then. But otherwise, for most codebases and certainly for industrial codebases, the above suggested change just makes the codebase more obtuse for questionable benefit. changelog: Remove implicit_hasher from default set of enabled lints

Finishes rust-lang#4277

bors · 2020-05-02T11:44:04Z

📌 Commit bce9fae has been approved by Mark-Simulacrum

oli-obk · 2020-05-02T11:48:44Z

Okay, obviously this is ~impossible to review, but I checked over it a bit locally and it looks good.

Only

are actual changes to this repo, the rest is just clippy being moved into the repo.

Mark-Simulacrum · 2020-05-02T12:00:11Z

Yep, I reviewed those three manually.

oli-obk · 2020-05-02T12:37:12Z

Thanks for the quick work! I'm very excited to be trying this out for clippy.

bors · 2020-05-02T13:19:42Z

⌛ Testing commit bce9fae with merge 53d3bc0...

bors · 2020-05-02T16:24:42Z

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 53d3bc0 to master...

rust-highfive · 2020-05-02T16:26:19Z

📣 Toolstate changed by #70655!

Tested on commit 53d3bc0.
Direct link to PR: #70655

💔 clippy-driver on windows: test-pass → test-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
💔 clippy-driver on linux: test-pass → test-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).

@mcarton

Tested on commit rust-lang/rust@53d3bc0. Direct link to PR: <rust-lang/rust#70655> 💔 clippy-driver on windows: test-pass → test-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq). 💔 clippy-driver on linux: test-pass → test-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).

flip1995 · 2020-05-02T16:33:55Z

CI logs show, that all Clippy tests passed.

~~Also, currently the clippy fmt test is run on CI. We might want to turn this of, until clippy uses the same rustfmt version + rustfmt.toml as rustc. But this has to be done in Clippy.~~ Nvm, fmt already gets skipped.

Mark-Simulacrum · 2020-05-02T16:35:39Z

Yeah not sure what's up with that. We'll want to remove clippy from toolstate at least for sure since it should always be test-pass now (I suspect the above might be spurious? we're no longer recording clippy toolstate I think as of this PR)

Manishearth · 2020-05-02T17:32:39Z

So is this also gating on subtree clippy? (Not sure if we were doing that in two steps) @oli-obk do we have a set of instructions for what we should do about syncs?

mati865 · 2020-05-02T18:31:39Z

Instructions were added in #70654

flip1995 · 2020-05-02T18:44:52Z

So is this also gating on subtree clippy?

If I understand the changes correctly, Clippy now gets checked like rustdoc. So yes, this also gates at subtree clippy.

matthiaskrgr · 2020-05-02T19:27:10Z

Instructions were added in #70654

Should we have a copy of that inside the clippy repo as well?

flip1995 · 2020-05-02T19:51:18Z

We should change the documentation in clippy that talks about updating clippy in rust-lang/rust (I think that exists?)

oli-obk · 2020-05-02T19:52:10Z

The reason toolstate fails is that we now don't report any toolstate for clippy anymore, and thus toolstate assumes "failure". I completely forgot about that part of toolstate, I'll look into it tomorrow

flip1995 · 2020-05-02T19:56:36Z

src/bootstrap/test.rs

-            if try_run(builder, &mut cargo.into()) {
-                builder.save_toolstate("clippy-driver", ToolState::TestPass);
-            }


This porbably caused rust-highfive to think that clippy is test-fail now.

petrochenkov · 2020-05-03T07:08:54Z

This will break rebase on master for all people in the near future:

error: The following untracked working tree files would be overwritten by checkout:
        src/tools/clippy/.editorconfig
        src/tools/clippy/.gitattributes
        src/tools/clippy/.github/ISSUE_TEMPLATE.md
        src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md
        src/tools/clippy/.github/deploy.sh
        ...

The solution is to get rid of the clippy submodule before the rebase:

git submodule deinit src/tools/clippy

rust-highfive assigned eddyb Apr 1, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2020

oli-obk mentioned this pull request Apr 1, 2020

Explain how to work with subtree #70654

Merged

eddyb mentioned this pull request Apr 1, 2020

Replace submodules with subtrees #70651

Closed

rust-highfive assigned Mark-Simulacrum and unassigned eddyb Apr 1, 2020

oli-obk mentioned this pull request Apr 1, 2020

Use git subtree (instead of git submodule) for external dependencies rust-lang/compiler-team#266

Closed

This comment has been minimized.

Sign in to view

oli-obk force-pushed the subrepo_funness branch from b57b33b to 149d20e Compare April 2, 2020 08:06

bors and others added 15 commits April 8, 2020 14:17

Move cognitive_complexity to nursery

899a1b5

Downgrade implicit_hasher to pedantic

5f92fae

Add lint when comparing floats in an array

d440935

Finishes rust-lang#4277

Add tests for float in array comparison

0d00eaf

Update field names in is_float

03f584c

Update stderr of float_cmp test

b4ff774

Add handling of float arrays to miri_to_const

2153abb

Handle evaluating constant index expression

6217671

Allow for const arrays of zeros

bcbb9d9

Don't show comparison suggestion for arrays

1bab67c

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 2, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label May 2, 2020

bors merged commit 53d3bc0 into rust-lang:master May 2, 2020

oli-obk deleted the subrepo_funness branch May 2, 2020 19:47

flip1995 reviewed May 2, 2020

View reviewed changes

oli-obk mentioned this pull request May 3, 2020

Remove clippy from some leftover lists of "possibly failing" tools #71830

Merged

tesuji mentioned this pull request May 14, 2020

Merge dirs-sys and directories to dirs xdg-rs/dirs#3

Merged

ehuss mentioned this pull request Aug 19, 2020

Remove clippy rust-lang-nursery/rust-toolstate#20

Merged

aDotInTheVoid mentioned this pull request Dec 27, 2020

ICE unwrap of none value in librustc_hir/definitions.rs #74318

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make clippy a git subtree instead of a git submodule #70655

Make clippy a git subtree instead of a git submodule #70655

oli-obk commented Apr 1, 2020 •

edited

Loading

eddyb commented Apr 1, 2020

eddyb commented Apr 1, 2020

Mark-Simulacrum commented Apr 1, 2020

flip1995 commented Apr 1, 2020

This comment has been minimized.

oli-obk commented Apr 2, 2020

bors commented Apr 5, 2020

bors commented May 2, 2020

oli-obk commented May 2, 2020

Mark-Simulacrum commented May 2, 2020

oli-obk commented May 2, 2020

bors commented May 2, 2020

bors commented May 2, 2020

rust-highfive commented May 2, 2020

flip1995 commented May 2, 2020 •

edited

Loading

Mark-Simulacrum commented May 2, 2020

Manishearth commented May 2, 2020

mati865 commented May 2, 2020

flip1995 commented May 2, 2020

matthiaskrgr commented May 2, 2020 •

edited

Loading

flip1995 commented May 2, 2020

oli-obk commented May 2, 2020

flip1995 May 2, 2020

petrochenkov commented May 3, 2020

Make clippy a git subtree instead of a git submodule #70655

Make clippy a git subtree instead of a git submodule #70655

Conversation

oli-obk commented Apr 1, 2020 • edited Loading

eddyb commented Apr 1, 2020

eddyb commented Apr 1, 2020

Mark-Simulacrum commented Apr 1, 2020

flip1995 commented Apr 1, 2020

This comment has been minimized.

oli-obk commented Apr 2, 2020

bors commented Apr 5, 2020

bors commented May 2, 2020

oli-obk commented May 2, 2020

Mark-Simulacrum commented May 2, 2020

oli-obk commented May 2, 2020

bors commented May 2, 2020

bors commented May 2, 2020

rust-highfive commented May 2, 2020

flip1995 commented May 2, 2020 • edited Loading

Mark-Simulacrum commented May 2, 2020

Manishearth commented May 2, 2020

mati865 commented May 2, 2020

flip1995 commented May 2, 2020

matthiaskrgr commented May 2, 2020 • edited Loading

flip1995 commented May 2, 2020

oli-obk commented May 2, 2020

flip1995 May 2, 2020

Choose a reason for hiding this comment

petrochenkov commented May 3, 2020

oli-obk commented Apr 1, 2020 •

edited

Loading

flip1995 commented May 2, 2020 •

edited

Loading

matthiaskrgr commented May 2, 2020 •

edited

Loading