Skip to content
/ rustc Public
forked from rust-lang/rust

Commit fcc0e8f

Browse files
committed
Auto merge of rust-lang#72671 - flip1995:clippyup, r=Xanewok
Update Clippy, RLS, and rustfmt r? @Dylan-DPC This makes Clippy test-pass again: 3089c3b Otherwise this includes bugfixes and a few new lints. Fixes rust-lang#72231 Fixes rust-lang#72232
2 parents 28c690e + b6c58f0 commit fcc0e8f

File tree

84 files changed

+1553
-433
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

84 files changed

+1553
-433
lines changed

.github/workflows/clippy.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ jobs:
4949
run: cargo update
5050

5151
- name: Cache cargo dir
52-
uses: actions/cache@v1
52+
uses: actions/cache@v2
5353
with:
5454
path: ~/.cargo
5555
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }}

.github/workflows/clippy_bors.yml

+5-5
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ jobs:
9494
run: cargo update
9595

9696
- name: Cache cargo dir
97-
uses: actions/cache@v1
97+
uses: actions/cache@v2
9898
with:
9999
path: ~/.cargo
100100
key: ${{ runner.os }}-${{ matrix.host }}-${{ hashFiles('Cargo.lock') }}
@@ -190,7 +190,7 @@ jobs:
190190
run: cargo update
191191

192192
- name: Cache cargo dir
193-
uses: actions/cache@v1
193+
uses: actions/cache@v2
194194
with:
195195
path: ~/.cargo
196196
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }}
@@ -269,7 +269,7 @@ jobs:
269269
run: cargo update
270270

271271
- name: Cache cargo dir
272-
uses: actions/cache@v1
272+
uses: actions/cache@v2
273273
with:
274274
path: ~/.cargo
275275
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }}
@@ -312,7 +312,7 @@ jobs:
312312
name: bors test finished
313313
if: github.event.pusher.name == 'bors' && success()
314314
runs-on: ubuntu-latest
315-
needs: [base, integration]
315+
needs: [changelog, base, integration_build, integration]
316316

317317
steps:
318318
- name: Mark the job as successful
@@ -322,7 +322,7 @@ jobs:
322322
name: bors test finished
323323
if: github.event.pusher.name == 'bors' && (failure() || cancelled())
324324
runs-on: ubuntu-latest
325-
needs: [base, integration]
325+
needs: [changelog, base, integration_build, integration]
326326

327327
steps:
328328
- name: Mark the job as a failure

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,7 @@ Released 2018-09-13
14391439
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
14401440
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
14411441
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
1442+
[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
14421443
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
14431444
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
14441445
[`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget

CONTRIBUTING.md

+65-35
Original file line numberDiff line numberDiff line change
@@ -155,47 +155,77 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun
155155

156156
## Fixing build failures caused by Rust
157157

158-
Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of
159-
the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures
160-
caused by Rust updates, can be a good way to learn about Rust internals.
158+
Clippy currently gets built with `rustc` of the `rust-lang/rust` `master`
159+
branch. Most of the times we have to adapt to the changes and only very rarely
160+
there's an actual bug in Rust.
161+
162+
If you decide to make Clippy work again with a Rust commit that breaks it, you
163+
have to sync the `rust-lang/rust-clippy` repository with the `subtree` copy of
164+
Clippy in the `rust-lang/rust` repository.
165+
166+
For general information about `subtree`s in the Rust repository see [Rust's
167+
`CONTRIBUTING.md`][subtree].
168+
169+
Here is a TL;DR version of the sync process (all of the following commands have
170+
to be run inside the `rust` directory):
171+
172+
1. Clone the [`rust-lang/rust`] repository
173+
2. Sync the changes to the rust-copy of Clippy to your Clippy fork:
174+
```bash
175+
# Make sure to change `your-github-name` to your github name in the following command
176+
git subtree push -P src/tools/clippy [email protected]:your-github-name/rust-clippy sync-from-rust
177+
```
178+
_Note:_ This will directly push to the remote repository. You can also push
179+
to your local copy by replacing the remote address with `/path/to/rust-clippy`
180+
directory.
181+
182+
_Note:_ Most of the time you have to create a merge commit in the
183+
`rust-clippy` repo (this has to be done in the Clippy repo, not in the
184+
rust-copy of Clippy):
185+
```bash
186+
git fetch origin && git fetch upstream
187+
git checkout sync-from-rust
188+
git merge upstream/master
189+
```
190+
3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to
191+
accelerate the process ping the `@rust-lang/clippy` team in your PR and/or
192+
~~annoy~~ ask them in the [Discord] channel.)
193+
4. Sync the `rust-lang/rust-clippy` master to the rust-copy of Clippy:
194+
```bash
195+
git checkout -b sync-from-clippy
196+
git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master
197+
```
198+
5. Open a PR to [`rust-lang/rust`]
199+
200+
Also, you may want to define remotes, so you don't have to type out the remote
201+
addresses on every sync. You can do this with the following commands (these
202+
commands still have to be run inside the `rust` directory):
161203
162-
In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit
163-
history][toolstate_commit_history]. You will then have to look for the last commit that contains
164-
`test-pass -> build-fail` or `test-pass -> test-fail` for the `clippy-driver` component.
165-
[Here][toolstate_commit] is an example.
166-
167-
The commit message contains a link to the PR. The PRs are usually small enough to discover the breaking API change and
168-
if they are bigger, they likely include some discussion that may help you to fix Clippy.
169-
170-
To check if Clippy is available for a specific target platform, you can check
171-
the [rustup component history][rustup_component_history].
172-
173-
If you decide to make Clippy work again with a Rust commit that breaks it,
174-
you probably want to install the latest Rust from master locally and run Clippy
175-
using that version of Rust.
176-
177-
You can set up the master toolchain by running `./setup-toolchain.sh`. That script will install
178-
[rustup-toolchain-install-master][rtim] and master toolchain, then run `rustup override set master`.
179-
180-
After fixing the build failure on this repository, we can submit a pull request
181-
to [`rust-lang/rust`] to fix the toolstate.
204+
```bash
205+
# Set clippy-upstream remote for pulls
206+
$ git remote add clippy-upstream https://github.com/rust-lang/rust-clippy
207+
# Make sure to not push to the upstream repo
208+
$ git remote set-url --push clippy-upstream DISABLED
209+
# Set clippy-origin remote to your fork for pushes
210+
$ git remote add clippy-origin [email protected]:your-github-name/rust-clippy
211+
# Set a local remote
212+
$ git remote add clippy-local /path/to/rust-clippy
213+
```
182214
183-
To submit a pull request, you should follow these steps:
215+
You can then sync with the remote names from above, e.g.:
184216
185217
```bash
186-
# Assuming you already cloned the rust-lang/rust repo and you're in the correct directory
187-
git submodule update --remote src/tools/clippy
188-
cargo update -p clippy
189-
git add -u
190-
git commit -m "Update Clippy"
191-
./x.py test -i --stage 1 src/tools/clippy # This is optional and should succeed anyway
192-
# Open a PR in rust-lang/rust
218+
$ git subtree push -P src/tools/clippy clippy-local sync-from-rust
193219
```
194220
195-
[rustup_component_history]: https://rust-lang.github.io/rustup-components-history
196-
[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master
197-
[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/aad74d8294e198a7cf8ac81a91aebb7f3bbcf727
198-
[rtim]: https://github.com/kennytm/rustup-toolchain-install-master
221+
_Note:_ The first time running `git subtree push` a cache has to be built. This
222+
involves going through the complete Clippy history once. For this you have to
223+
increase the stack limit though, which you can do with `ulimit -s 60000`. For
224+
this to work, you will need the fix of `git subtree` available
225+
[here][gitgitgadget-pr].
226+
227+
[gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493
228+
[subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree
199229
[`rust-lang/rust`]: https://github.com/rust-lang/rust
200230
201231
## Issue and PR triage

clippy_dev/src/new_lint.rs

+115-75
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,111 @@
11
use crate::clippy_project_root;
2-
use std::fs::{File, OpenOptions};
3-
use std::io;
2+
use std::fs::{self, OpenOptions};
43
use std::io::prelude::*;
5-
use std::io::ErrorKind;
6-
use std::path::Path;
4+
use std::io::{self, ErrorKind};
5+
use std::path::{Path, PathBuf};
6+
7+
struct LintData<'a> {
8+
pass: &'a str,
9+
name: &'a str,
10+
category: &'a str,
11+
project_root: PathBuf,
12+
}
13+
14+
trait Context {
15+
fn context<C: AsRef<str>>(self, text: C) -> Self;
16+
}
17+
18+
impl<T> Context for io::Result<T> {
19+
fn context<C: AsRef<str>>(self, text: C) -> Self {
20+
match self {
21+
Ok(t) => Ok(t),
22+
Err(e) => {
23+
let message = format!("{}: {}", text.as_ref(), e);
24+
Err(io::Error::new(ErrorKind::Other, message))
25+
},
26+
}
27+
}
28+
}
729

8-
/// Creates files required to implement and test a new lint and runs `update_lints`.
30+
/// Creates the files required to implement and test a new lint and runs `update_lints`.
931
///
1032
/// # Errors
1133
///
12-
/// This function errors, if the files couldn't be created
13-
pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> {
14-
let pass = pass.expect("`pass` argument is validated by clap");
15-
let lint_name = lint_name.expect("`name` argument is validated by clap");
16-
let category = category.expect("`category` argument is validated by clap");
17-
18-
match open_files(lint_name) {
19-
Ok((mut test_file, mut lint_file)) => {
20-
let (pass_type, pass_lifetimes, pass_import, context_import) = match pass {
21-
"early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
22-
"late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"),
23-
_ => {
24-
unreachable!("`pass_type` should only ever be `early` or `late`!");
25-
},
26-
};
27-
28-
let camel_case_name = to_camel_case(lint_name);
29-
30-
if let Err(e) = test_file.write_all(get_test_file_contents(lint_name).as_bytes()) {
31-
return Err(io::Error::new(
32-
ErrorKind::Other,
33-
format!("Could not write to test file: {}", e),
34-
));
35-
};
36-
37-
if let Err(e) = lint_file.write_all(
38-
get_lint_file_contents(
39-
pass_type,
40-
pass_lifetimes,
41-
lint_name,
42-
&camel_case_name,
43-
category,
44-
pass_import,
45-
context_import,
46-
)
47-
.as_bytes(),
48-
) {
49-
return Err(io::Error::new(
50-
ErrorKind::Other,
51-
format!("Could not write to lint file: {}", e),
52-
));
53-
}
54-
Ok(())
34+
/// This function errors out if the files couldn't be created or written to.
35+
pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> io::Result<()> {
36+
let lint = LintData {
37+
pass: pass.expect("`pass` argument is validated by clap"),
38+
name: lint_name.expect("`name` argument is validated by clap"),
39+
category: category.expect("`category` argument is validated by clap"),
40+
project_root: clippy_project_root(),
41+
};
42+
43+
create_lint(&lint).context("Unable to create lint implementation")?;
44+
create_test(&lint).context("Unable to create a test for the new lint")
45+
}
46+
47+
fn create_lint(lint: &LintData) -> io::Result<()> {
48+
let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass {
49+
"early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
50+
"late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"),
51+
_ => {
52+
unreachable!("`pass_type` should only ever be `early` or `late`!");
5553
},
56-
Err(e) => Err(io::Error::new(
57-
ErrorKind::Other,
58-
format!("Unable to create lint: {}", e),
59-
)),
60-
}
54+
};
55+
56+
let camel_case_name = to_camel_case(lint.name);
57+
let lint_contents = get_lint_file_contents(
58+
pass_type,
59+
pass_lifetimes,
60+
lint.name,
61+
&camel_case_name,
62+
lint.category,
63+
pass_import,
64+
context_import,
65+
);
66+
67+
let lint_path = format!("clippy_lints/src/{}.rs", lint.name);
68+
write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes())
6169
}
6270

63-
fn open_files(lint_name: &str) -> Result<(File, File), io::Error> {
64-
let project_root = clippy_project_root();
71+
fn create_test(lint: &LintData) -> io::Result<()> {
72+
fn create_project_layout<P: Into<PathBuf>>(lint_name: &str, location: P, case: &str, hint: &str) -> io::Result<()> {
73+
let mut path = location.into().join(case);
74+
fs::create_dir(&path)?;
75+
write_file(path.join("Cargo.toml"), get_manifest_contents(lint_name, hint))?;
6576

66-
let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name));
67-
let lint_file_path = project_root
68-
.join("clippy_lints")
69-
.join("src")
70-
.join(format!("{}.rs", lint_name));
77+
path.push("src");
78+
fs::create_dir(&path)?;
79+
let header = format!("// compile-flags: --crate-name={}", lint_name);
80+
write_file(path.join("main.rs"), get_test_file_contents(lint_name, Some(&header)))?;
7181

72-
if Path::new(&test_file_path).exists() {
73-
return Err(io::Error::new(
74-
ErrorKind::AlreadyExists,
75-
format!("test file {:?} already exists", test_file_path),
76-
));
82+
Ok(())
7783
}
78-
if Path::new(&lint_file_path).exists() {
79-
return Err(io::Error::new(
80-
ErrorKind::AlreadyExists,
81-
format!("lint file {:?} already exists", lint_file_path),
82-
));
84+
85+
if lint.category == "cargo" {
86+
let relative_test_dir = format!("tests/ui-cargo/{}", lint.name);
87+
let test_dir = lint.project_root.join(relative_test_dir);
88+
fs::create_dir(&test_dir)?;
89+
90+
create_project_layout(lint.name, &test_dir, "fail", "Content that triggers the lint goes here")?;
91+
create_project_layout(lint.name, &test_dir, "pass", "This file should not trigger the lint")
92+
} else {
93+
let test_path = format!("tests/ui/{}.rs", lint.name);
94+
let test_contents = get_test_file_contents(lint.name, None);
95+
write_file(lint.project_root.join(test_path), test_contents)
8396
}
97+
}
8498

85-
let test_file = OpenOptions::new().write(true).create_new(true).open(test_file_path)?;
86-
let lint_file = OpenOptions::new().write(true).create_new(true).open(lint_file_path)?;
99+
fn write_file<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result<()> {
100+
fn inner(path: &Path, contents: &[u8]) -> io::Result<()> {
101+
OpenOptions::new()
102+
.write(true)
103+
.create_new(true)
104+
.open(path)?
105+
.write_all(contents)
106+
}
87107

88-
Ok((test_file, lint_file))
108+
inner(path.as_ref(), contents.as_ref()).context(format!("writing to file: {}", path.as_ref().display()))
89109
}
90110

91111
fn to_camel_case(name: &str) -> String {
@@ -100,15 +120,35 @@ fn to_camel_case(name: &str) -> String {
100120
.collect()
101121
}
102122

103-
fn get_test_file_contents(lint_name: &str) -> String {
104-
format!(
123+
fn get_test_file_contents(lint_name: &str, header_commands: Option<&str>) -> String {
124+
let mut contents = format!(
105125
"#![warn(clippy::{})]
106126
107127
fn main() {{
108128
// test code goes here
109129
}}
110130
",
111131
lint_name
132+
);
133+
134+
if let Some(header) = header_commands {
135+
contents = format!("{}\n{}", header, contents);
136+
}
137+
138+
contents
139+
}
140+
141+
fn get_manifest_contents(lint_name: &str, hint: &str) -> String {
142+
format!(
143+
r#"
144+
# {}
145+
146+
[package]
147+
name = "{}"
148+
version = "0.1.0"
149+
publish = false
150+
"#,
151+
hint, lint_name
112152
)
113153
}
114154

0 commit comments

Comments
 (0)