Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8,408 commits into from
May 2, 2020

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 1, 2020

r? @eddyb

cc #70651

documentation at #70654

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

eddyb commented Apr 1, 2020

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

@eddyb
Copy link
Member

eddyb commented Apr 1, 2020

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

@Mark-Simulacrum
Copy link
Member

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
Copy link
Member

flip1995 commented Apr 1, 2020

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

@bors

This comment has been minimized.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 2, 2020

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
Copy link
Contributor

bors commented Apr 5, 2020

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

bors and others added 15 commits April 8, 2020 14:17
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
@bors
Copy link
Contributor

bors commented May 2, 2020

📌 Commit bce9fae has been approved by Mark-Simulacrum

@bors 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
@oli-obk
Copy link
Contributor Author

oli-obk commented May 2, 2020

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
Copy link
Member

Yep, I reviewed those three manually.

@oli-obk
Copy link
Contributor Author

oli-obk commented May 2, 2020

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

@bors
Copy link
Contributor

bors commented May 2, 2020

⌛ Testing commit bce9fae with merge 53d3bc0...

@bors
Copy link
Contributor

bors commented May 2, 2020

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 2, 2020
@bors bors merged commit 53d3bc0 into rust-lang:master May 2, 2020
@rust-highfive
Copy link
Collaborator

📣 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).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 2, 2020
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
Copy link
Member

flip1995 commented May 2, 2020

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

mati865 commented May 2, 2020

Instructions were added in #70654

@flip1995
Copy link
Member

flip1995 commented May 2, 2020

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
Copy link
Member

matthiaskrgr commented May 2, 2020

Instructions were added in #70654

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

@oli-obk oli-obk deleted the subrepo_funness branch May 2, 2020 19:47
@flip1995
Copy link
Member

flip1995 commented May 2, 2020

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

@oli-obk
Copy link
Contributor Author

oli-obk commented May 2, 2020

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

Comment on lines -551 to -553
if try_run(builder, &mut cargo.into()) {
builder.save_toolstate("clippy-driver", ToolState::TestPass);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@petrochenkov
Copy link
Contributor

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

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 3, 2020
…ulacrum

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

rust-lang#70655 successfully made clippy get built and tested on CI on every merge, but the lack of emitted toolstate info caused the toolstate to get updated to test-fail. We should remove clippy entirely from toolstate, as it now is always test-pass.

The changes made in this PR reflect what we do for `rustdoc`, which is our preexisting tool that is gated on CI.

r? @Mark-Simulacrum
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 5, 2020
…ulacrum

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

rust-lang#70655 successfully made clippy get built and tested on CI on every merge, but the lack of emitted toolstate info caused the toolstate to get updated to test-fail. We should remove clippy entirely from toolstate, as it now is always test-pass.

The changes made in this PR reflect what we do for `rustdoc`, which is our preexisting tool that is gated on CI.

r? @Mark-Simulacrum
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 6, 2020
While here clean up all pkglint warnings.  Changes since 1.44.1:

Version 1.45.2 (2020-08-03)
==========================

* [Fix bindings in tuple struct patterns][74954]
* [Fix track_caller integration with trait objects][74784]

[74954]: rust-lang/rust#74954
[74784]: rust-lang/rust#74784

Version 1.45.1 (2020-07-30)
==========================

* [Fix const propagation with references.][73613]
* [rustfmt accepts rustfmt_skip in cfg_attr again.][73078]
* [Avoid spurious implicit region bound.][74509]
* [Install clippy on x.py install][74457]

[73613]: rust-lang/rust#73613
[73078]: rust-lang/rust#73078
[74509]: rust-lang/rust#74509
[74457]: rust-lang/rust#74457

Version 1.45.0 (2020-07-16)
==========================

Language
--------
- [Out of range float to int conversions using `as` has been defined as a saturating
  conversion.][71269] This was previously undefined behaviour, but you can use the
   `{f64, f32}::to_int_unchecked` methods to continue using the current behaviour, which
   may be desirable in rare performance sensitive situations.
- [`mem::Discriminant<T>` now uses `T`'s discriminant type instead of always
  using `u64`.][70705]
- [Function like procedural macros can now be used in expression, pattern, and  statement
  positions.][68717] This means you can now use a function-like procedural macro
  anywhere you can use a declarative (`macro_rules!`) macro.

Compiler
--------
- [You can now override individual target features through the `target-feature`
  flag.][72094] E.g. `-C target-feature=+avx2 -C target-feature=+fma` is now
  equivalent to `-C target-feature=+avx2,+fma`.
- [Added the `force-unwind-tables` flag.][69984] This option allows
  rustc to always generate unwind tables regardless of panic strategy.
- [Added the `embed-bitcode` flag.][71716] This codegen flag allows rustc
  to include LLVM bitcode into generated `rlib`s (this is on by default).
- [Added the `tiny` value to the `code-model` codegen flag.][72397]
- [Added tier 3 support\* for the `mipsel-sony-psp` target.][72062]
- [Added tier 3 support for the `thumbv7a-uwp-windows-msvc` target.][72133]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.


Libraries
---------
- [`net::{SocketAddr, SocketAddrV4, SocketAddrV6}` now implements `PartialOrd`
  and `Ord`.][72239]
- [`proc_macro::TokenStream` now implements `Default`.][72234]
- [You can now use `char` with
  `ops::{Range, RangeFrom, RangeFull, RangeInclusive, RangeTo}` to iterate over
  a range of codepoints.][72413] E.g.
  you can now write the following;
  ```rust
  for ch in 'a'..='z' {
      print!("{}", ch);
  }
  println!();
  // Prints "abcdefghijklmnopqrstuvwxyz"
  ```
- [`OsString` now implements `FromStr`.][71662]
- [The `saturating_neg` method as been added to all signed integer primitive
  types, and the `saturating_abs` method has been added for all integer
  primitive types.][71886]
- [`Arc<T>`, `Rc<T>` now implement  `From<Cow<'_, T>>`, and `Box` now
  implements `From<Cow>` when `T` is `[T: Copy]`, `str`, `CStr`, `OsStr`,
  or `Path`.][71447]
- [`Box<[T]>` now implements `From<[T; N]>`.][71095]
- [`BitOr` and `BitOrAssign` are implemented for all `NonZero`
  integer types.][69813]
- [The `fetch_min`, and `fetch_max` methods have been added to all atomic
  integer types.][72324]
- [The `fetch_update` method has been added to all atomic integer types.][71843]

Stabilized APIs
---------------
- [`Arc::as_ptr`]
- [`BTreeMap::remove_entry`]
- [`Rc::as_ptr`]
- [`rc::Weak::as_ptr`]
- [`rc::Weak::from_raw`]
- [`rc::Weak::into_raw`]
- [`str::strip_prefix`]
- [`str::strip_suffix`]
- [`sync::Weak::as_ptr`]
- [`sync::Weak::from_raw`]
- [`sync::Weak::into_raw`]
- [`char::UNICODE_VERSION`]
- [`Span::resolved_at`]
- [`Span::located_at`]
- [`Span::mixed_site`]
- [`unix::process::CommandExt::arg0`]

Cargo
-----

Misc
----
- [Rustdoc now supports strikethrough text in Markdown.][71928] E.g.
  `~~outdated information~~` becomes "~~outdated information~~".
- [Added an emoji to Rustdoc's deprecated API message.][72014]

Compatibility Notes
-------------------
- [Trying to self initialize a static value (that is creating a value using
  itself) is unsound and now causes a compile error.][71140]
- [`{f32, f64}::powi` now returns a slightly different value on Windows.][73420]
  This is due to changes in LLVM's intrinsics which `{f32, f64}::powi` uses.
- [Rustdoc's CLI's extra error exit codes have been removed.][71900] These were
  previously undocumented and not intended for public use. Rustdoc still provides
  a non-zero exit code on errors.

Internals Only
--------------
- [Make clippy a git subtree instead of a git submodule][70655]
- [Unify the undo log of all snapshot types][69464]

[73420]: rust-lang/rust#73420
[72324]: rust-lang/rust#72324
[71843]: rust-lang/rust#71843
[71886]: rust-lang/rust#71886
[72234]: rust-lang/rust#72234
[72239]: rust-lang/rust#72239
[72397]: rust-lang/rust#72397
[72413]: rust-lang/rust#72413
[72014]: rust-lang/rust#72014
[72062]: rust-lang/rust#72062
[72094]: rust-lang/rust#72094
[72133]: rust-lang/rust#72133
[71900]: rust-lang/rust#71900
[71928]: rust-lang/rust#71928
[71662]: rust-lang/rust#71662
[71716]: rust-lang/rust#71716
[71447]: rust-lang/rust#71447
[71269]: rust-lang/rust#71269
[71095]: rust-lang/rust#71095
[71140]: rust-lang/rust#71140
[70655]: rust-lang/rust#70655
[70705]: rust-lang/rust#70705
[69984]: rust-lang/rust#69984
[69813]: rust-lang/rust#69813
[69464]: rust-lang/rust#69464
[68717]: rust-lang/rust#68717
[`Arc::as_ptr`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.as_ptr
[`BTreeMap::remove_entry`]: https://doc.rust-lang.org/stable/std/collections/struct.BTreeMap.html#method.remove_entry
[`Rc::as_ptr`]: https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.as_ptr
[`rc::Weak::as_ptr`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.as_ptr
[`rc::Weak::from_raw`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.from_raw
[`rc::Weak::into_raw`]: https://doc.rust-lang.org/stable/std/rc/struct.Weak.html#method.into_raw
[`sync::Weak::as_ptr`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.as_ptr
[`sync::Weak::from_raw`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.from_raw
[`sync::Weak::into_raw`]: https://doc.rust-lang.org/stable/std/sync/struct.Weak.html#method.into_raw
[`str::strip_prefix`]: https://doc.rust-lang.org/stable/std/primitive.str.html#method.strip_prefix
[`str::strip_suffix`]: https://doc.rust-lang.org/stable/std/primitive.str.html#method.strip_suffix
[`char::UNICODE_VERSION`]: https://doc.rust-lang.org/stable/std/char/constant.UNICODE_VERSION.html
[`Span::resolved_at`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.resolved_at
[`Span::located_at`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.located_at
[`Span::mixed_site`]: https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.mixed_site
[`unix::process::CommandExt::arg0`]: https://doc.rust-lang.org/std/os/unix/process/trait.CommandExt.html#tymethod.arg0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.