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

Commit

Permalink
Merge commit '7ea7cd165ad6705603852771bf82cc2fd6560db5' into clippyup2
Browse files Browse the repository at this point in the history
  • Loading branch information
flip1995 committed May 28, 2020
1 parent e820a03 commit a0e9f9b
Show file tree
Hide file tree
Showing 84 changed files with 1,550 additions and 433 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
run: cargo update

- name: Cache cargo dir
uses: actions/cache@v1
uses: actions/cache@v2
with:
path: ~/.cargo
key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }}
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/clippy_bors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ jobs:
run: cargo update

- name: Cache cargo dir
uses: actions/cache@v1
uses: actions/cache@v2
with:
path: ~/.cargo
key: ${{ runner.os }}-${{ matrix.host }}-${{ hashFiles('Cargo.lock') }}
Expand Down Expand Up @@ -190,7 +190,7 @@ jobs:
run: cargo update

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

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

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

steps:
- name: Mark the job as a failure
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,7 @@ Released 2018-09-13
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
[`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
Expand Down
100 changes: 65 additions & 35 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,47 +155,77 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun

## Fixing build failures caused by Rust

Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of
the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures
caused by Rust updates, can be a good way to learn about Rust internals.
Clippy currently gets built with `rustc` of the `rust-lang/rust` `master`
branch. Most of the times we have to adapt to the changes and only very rarely
there's an actual bug in Rust.

If you decide to make Clippy work again with a Rust commit that breaks it, you
have to sync the `rust-lang/rust-clippy` repository with the `subtree` copy of
Clippy in the `rust-lang/rust` repository.

For general information about `subtree`s in the Rust repository see [Rust's
`CONTRIBUTING.md`][subtree].

Here is a TL;DR version of the sync process (all of the following commands have
to be run inside the `rust` directory):

1. Clone the [`rust-lang/rust`] repository
2. Sync the changes to the rust-copy of Clippy to your Clippy fork:
```bash
# Make sure to change `your-github-name` to your github name in the following command
git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust
```
_Note:_ This will directly push to the remote repository. You can also push
to your local copy by replacing the remote address with `/path/to/rust-clippy`
directory.

_Note:_ Most of the time you have to create a merge commit in the
`rust-clippy` repo (this has to be done in the Clippy repo, not in the
rust-copy of Clippy):
```bash
git fetch origin && git fetch upstream
git checkout sync-from-rust
git merge upstream/master
```
3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to
accelerate the process ping the `@rust-lang/clippy` team in your PR and/or
~~annoy~~ ask them in the [Discord] channel.)
4. Sync the `rust-lang/rust-clippy` master to the rust-copy of Clippy:
```bash
git checkout -b sync-from-clippy
git subtree pull -P src/tools/clippy https://github.com/rust-lang/rust-clippy master
```
5. Open a PR to [`rust-lang/rust`]

Also, you may want to define remotes, so you don't have to type out the remote
addresses on every sync. You can do this with the following commands (these
commands still have to be run inside the `rust` directory):
In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit
history][toolstate_commit_history]. You will then have to look for the last commit that contains
`test-pass -> build-fail` or `test-pass -> test-fail` for the `clippy-driver` component.
[Here][toolstate_commit] is an example.

The commit message contains a link to the PR. The PRs are usually small enough to discover the breaking API change and
if they are bigger, they likely include some discussion that may help you to fix Clippy.

To check if Clippy is available for a specific target platform, you can check
the [rustup component history][rustup_component_history].

If you decide to make Clippy work again with a Rust commit that breaks it,
you probably want to install the latest Rust from master locally and run Clippy
using that version of Rust.

You can set up the master toolchain by running `./setup-toolchain.sh`. That script will install
[rustup-toolchain-install-master][rtim] and master toolchain, then run `rustup override set master`.

After fixing the build failure on this repository, we can submit a pull request
to [`rust-lang/rust`] to fix the toolstate.
```bash
# Set clippy-upstream remote for pulls
$ git remote add clippy-upstream https://github.com/rust-lang/rust-clippy
# Make sure to not push to the upstream repo
$ git remote set-url --push clippy-upstream DISABLED
# Set clippy-origin remote to your fork for pushes
$ git remote add clippy-origin git@github.com:your-github-name/rust-clippy
# Set a local remote
$ git remote add clippy-local /path/to/rust-clippy
```
To submit a pull request, you should follow these steps:
You can then sync with the remote names from above, e.g.:
```bash
# Assuming you already cloned the rust-lang/rust repo and you're in the correct directory
git submodule update --remote src/tools/clippy
cargo update -p clippy
git add -u
git commit -m "Update Clippy"
./x.py test -i --stage 1 src/tools/clippy # This is optional and should succeed anyway
# Open a PR in rust-lang/rust
$ git subtree push -P src/tools/clippy clippy-local sync-from-rust
```
[rustup_component_history]: https://rust-lang.github.io/rustup-components-history
[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master
[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/aad74d8294e198a7cf8ac81a91aebb7f3bbcf727
[rtim]: https://github.com/kennytm/rustup-toolchain-install-master
_Note:_ The first time running `git subtree push` a cache has to be built. This
involves going through the complete Clippy history once. For this you have to
increase the stack limit though, which you can do with `ulimit -s 60000`. For
this to work, you will need the fix of `git subtree` available
[here][gitgitgadget-pr].
[gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493
[subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree
[`rust-lang/rust`]: https://github.com/rust-lang/rust
## Issue and PR triage
Expand Down
190 changes: 115 additions & 75 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
@@ -1,91 +1,111 @@
use crate::clippy_project_root;
use std::fs::{File, OpenOptions};
use std::io;
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
use std::io::ErrorKind;
use std::path::Path;
use std::io::{self, ErrorKind};
use std::path::{Path, PathBuf};

struct LintData<'a> {
pass: &'a str,
name: &'a str,
category: &'a str,
project_root: PathBuf,
}

trait Context {
fn context<C: AsRef<str>>(self, text: C) -> Self;
}

impl<T> Context for io::Result<T> {
fn context<C: AsRef<str>>(self, text: C) -> Self {
match self {
Ok(t) => Ok(t),
Err(e) => {
let message = format!("{}: {}", text.as_ref(), e);
Err(io::Error::new(ErrorKind::Other, message))
},
}
}
}

/// Creates files required to implement and test a new lint and runs `update_lints`.
/// Creates the files required to implement and test a new lint and runs `update_lints`.
///
/// # Errors
///
/// This function errors, if the files couldn't be created
pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> {
let pass = pass.expect("`pass` argument is validated by clap");
let lint_name = lint_name.expect("`name` argument is validated by clap");
let category = category.expect("`category` argument is validated by clap");

match open_files(lint_name) {
Ok((mut test_file, mut lint_file)) => {
let (pass_type, pass_lifetimes, pass_import, context_import) = match pass {
"early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
"late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"),
_ => {
unreachable!("`pass_type` should only ever be `early` or `late`!");
},
};

let camel_case_name = to_camel_case(lint_name);

if let Err(e) = test_file.write_all(get_test_file_contents(lint_name).as_bytes()) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to test file: {}", e),
));
};

if let Err(e) = lint_file.write_all(
get_lint_file_contents(
pass_type,
pass_lifetimes,
lint_name,
&camel_case_name,
category,
pass_import,
context_import,
)
.as_bytes(),
) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to lint file: {}", e),
));
}
Ok(())
/// This function errors out if the files couldn't be created or written to.
pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> io::Result<()> {
let lint = LintData {
pass: pass.expect("`pass` argument is validated by clap"),
name: lint_name.expect("`name` argument is validated by clap"),
category: category.expect("`category` argument is validated by clap"),
project_root: clippy_project_root(),
};

create_lint(&lint).context("Unable to create lint implementation")?;
create_test(&lint).context("Unable to create a test for the new lint")
}

fn create_lint(lint: &LintData) -> io::Result<()> {
let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass {
"early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"),
"late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"),
_ => {
unreachable!("`pass_type` should only ever be `early` or `late`!");
},
Err(e) => Err(io::Error::new(
ErrorKind::Other,
format!("Unable to create lint: {}", e),
)),
}
};

let camel_case_name = to_camel_case(lint.name);
let lint_contents = get_lint_file_contents(
pass_type,
pass_lifetimes,
lint.name,
&camel_case_name,
lint.category,
pass_import,
context_import,
);

let lint_path = format!("clippy_lints/src/{}.rs", lint.name);
write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes())
}

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

let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name));
let lint_file_path = project_root
.join("clippy_lints")
.join("src")
.join(format!("{}.rs", lint_name));
path.push("src");
fs::create_dir(&path)?;
let header = format!("// compile-flags: --crate-name={}", lint_name);
write_file(path.join("main.rs"), get_test_file_contents(lint_name, Some(&header)))?;

if Path::new(&test_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("test file {:?} already exists", test_file_path),
));
Ok(())
}
if Path::new(&lint_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("lint file {:?} already exists", lint_file_path),
));

if lint.category == "cargo" {
let relative_test_dir = format!("tests/ui-cargo/{}", lint.name);
let test_dir = lint.project_root.join(relative_test_dir);
fs::create_dir(&test_dir)?;

create_project_layout(lint.name, &test_dir, "fail", "Content that triggers the lint goes here")?;
create_project_layout(lint.name, &test_dir, "pass", "This file should not trigger the lint")
} else {
let test_path = format!("tests/ui/{}.rs", lint.name);
let test_contents = get_test_file_contents(lint.name, None);
write_file(lint.project_root.join(test_path), test_contents)
}
}

let test_file = OpenOptions::new().write(true).create_new(true).open(test_file_path)?;
let lint_file = OpenOptions::new().write(true).create_new(true).open(lint_file_path)?;
fn write_file<P: AsRef<Path>, C: AsRef<[u8]>>(path: P, contents: C) -> io::Result<()> {
fn inner(path: &Path, contents: &[u8]) -> io::Result<()> {
OpenOptions::new()
.write(true)
.create_new(true)
.open(path)?
.write_all(contents)
}

Ok((test_file, lint_file))
inner(path.as_ref(), contents.as_ref()).context(format!("writing to file: {}", path.as_ref().display()))
}

fn to_camel_case(name: &str) -> String {
Expand All @@ -100,15 +120,35 @@ fn to_camel_case(name: &str) -> String {
.collect()
}

fn get_test_file_contents(lint_name: &str) -> String {
format!(
fn get_test_file_contents(lint_name: &str, header_commands: Option<&str>) -> String {
let mut contents = format!(
"#![warn(clippy::{})]
fn main() {{
// test code goes here
}}
",
lint_name
);

if let Some(header) = header_commands {
contents = format!("{}\n{}", header, contents);
}

contents
}

fn get_manifest_contents(lint_name: &str, hint: &str) -> String {
format!(
r#"
# {}
[package]
name = "{}"
version = "0.1.0"
publish = false
"#,
hint, lint_name
)
}

Expand Down
Loading

0 comments on commit a0e9f9b

Please sign in to comment.