cc #4577

This is just an experiment. I don't think we have a consensus if we should move away from travis/appveyor.

GHA would let us run up to 20 concurrent jobs. Since we have 15 integration tests and 4 (linux, linux 32-bit, macos, windows) basic tests, we would be able to run everything concurrently. ~~Also IIUC we only have to build Clippy once for every initegration test and then only check the repos.~~ Nope, dependent jobs exist, but they won't keep the artifacts (not even the checkout).

TODO before merge:

• [x] Add DEPLOY_KEY secret to github repo
• [ ] test deployment on test branch gh-test
• [ ] talk with @rust-lang/infra for bors
• [ ] Add GHA badge to Cargo.toml (blocked on rust-lang/crates.io#1838)
• [ ] Add back travis + appveyor files for transition period (?)

changelog: none

This is a continuation of #5071. This time from a branch inside the rust-lang/rust-clippy repo, not from my fork, since secrets are not available in PRs from forks.

Copying the description of #5071 to here:

Closes #4577

~~This is just an experiment. I don't think we have a consensus if we should move away from travis/appveyor.~~ We have consensus: https://github.com/rust-lang/rust-clippy/pull/5071#issuecomment-576647638

~~GHA would let us run up to 20 concurrent jobs. Since we have 15 integration tests and 4 (linux, linux 32-bit, macos, windows) basic tests, we would be able to run everything concurrently.~~ The org has a limit of 60 jobs across the org, so we limit the matrix of the integration tests to 6 concurrent jobs.

~~Also IIUC we only have to build Clippy once for every initegration test and then only check the repos.~~ Nope, dependent jobs exist, but they won't keep the artifacts (not even the checkout).

TODO before merge:

• [x] Add DEPLOY_KEY secret to github repo
• [x] test deployment on test branch gh-test#
  • [x] Test normal deployment
  • [x] Test deployment no changes
  • [x] Test deployment of tag
• [x] talk with @rust-lang/infra for bors, @rust-lang/infra is good with the move (crater also uses GHA+bors)
• [x] ~~Get remark + clippy_dev check to work on pushes (https://github.community/t5/GitHub-Actions/~Builds-are-not-triggered-with-on-paths/m-p/44075; I contacted GH support about this) ~~That seems to start working again yesterday. Let's hope it keeps working.~~ Or not: df9be48. Now it works again: 723786a. I think we should wait, until this is reliable. It appears, that it doesn't work on force pushes (sometimes?): 5814142~~ We need to run the bors tests unconditionally anyway (47138d1) so it doesn't really matter.
• [x] ~~impl homu checks for GHA https://github.com/rust-lang/rust-clippy/pull/5071#issuecomment-576642983 -- I prepared: https://github.com/flip1995/rust-central-station/commit/f40230dc3cebfe4120c9dce027f61c18dd8935cb. I'd suggest to first add GHA and keep the travis and appveyor checks for a few days and to remove them in a second pass. The bors dummy jobs are added in https://github.com/rust-lang/rust-clippy/pull/5088/commits/1a83b7ad7a45aa8db4db7da55a45931e725a1ca3 and work as expected: https://github.com/rust-lang/rust-clippy/pull/5088#issuecomment-582055695. I opened https://github.com/rust-lang/rust-central-station/pull/578~~ See https://github.com/rust-lang/rust-clippy/pull/5088#issuecomment-584581420
• [x] ~Add GHA badge to Cargo.toml (blocked on rust-lang/crates.io # 1838)~ Added a FIXME in 2332b57
• [x] ~Maybe we should also wait until GHA supports yaml anchors. https://github.community/t5/GitHub-Actions/Support-for-YAML-anchors/td-p/30336/~ WIll probably not be implemented in the near future.
• [x] Add back travis + appveyor files for transition period (!)

changelog: none

This lint deals with two separate cases of needless continue statements.

Case 1: continue is the first/only statement in the else block in a loop

loop {
    // region A
    if cond {
        // region B
    } else {
        continue;
    }
    // region C
}

Here, C can be merged with B and the else block can be thrown away:

loop {
    // region A
    if cond {
        // region B
        // region C
    }
}

Case 2: continue is the first/only statement in the if block in a loop

loop {
    // region A
    if cond {
        continue;
        // potentially more code here.
    } else {
        // region B
    }
    // region C
}

Here, we can negate the if condition cond and then merge B and C in the if block:

loop {
    // region A
    if !cond {
        // region B
        // region C
    }
}

This lint simplifies the negated condition expression by applying comparison operator inversions and de Morgan's laws for logical operations. Maybe this can be moved into a lint by itself.

[EDIT] I have my own function for indentation trimming because the one in utils did not work for me, and I did not want to change it to break other lints that might be depending on it. Also have a function for aligning code snippets such that the last line of the first snippet aligns with the first line of the second. Maybe we can add this to utils.

Originally part of #6161, but now this PR only deals with std crates

This change:

• Updates the std message .
• Updates the docs to mention how the busy loops should be fixed
  • Gives examples of how to do this for no_std targets
• Updates the tests/stderr files to test this change.

changelog: Update empty_loop lint documentation

This PR is to tackle https://github.com/rust-lang/rust-clippy/issues/3764 it's still a WIP and doesn't work but this is an initial stab. It builds though I haven't added any tests as I'm not sure where lint tests should go?

Unfortunately, it seems id isn't tied to the type itself but I guess where it is in the AST? Looking at https://manishearth.github.io/rust-internals-docs/syntax/ast/struct.Ty.html I can't see any members that would let me tell if a type was repeated in multiple trait bounds.

There may be other issues with how I've implemented this so any assistance is appreciated!

changelog: Add new lint: type_repetition_in_bounds

changelog: Added a new restriction lint pattern-type-mismatch. This lint is especially helpful for beginners learning about the magic behind pattern matching. (This explanation might be worth to include in the next changelog.)

This lint checks if all if-blocks contain some statements that are the same and can be moved out of the blocks to prevent code duplication. Here is an example:

let _ = if ... {
    println!("Start"); // <-- Lint for code duplication
    let _a = 99;
    println!("End"); // <-- Lint for code duplication
    false
} else {
    println!("Start");
    let _b = 17;
    println!("End"); 
    false
};

This could be written as:

println!("Start");

let _ = if ... {
    let _a = 99;
    false
} else {
    let _b = 17;
    false
};

println!("End"); 

This lint will get masked by the IF_SAME_THEN_ELSE lint. I think it makes more sense to only emit one lint per if block. This means that the folloing example:

if ... {
    let _a = 17;
} else {
    let _a = 17;
}

Will only trigger the IF_SAME_THEN_ELSE lint and not the SHARED_CODE_IN_IF_BLOCKS lint.

closes: #5234

changelog: Added a new lint: branches_sharing_code

And hello to the one that is writing the changelog for this release :D

changelog: Pin Clippy to a specific nightly version (No more master/custom toolchain required to compile Clippy)

Addresses partially #5561. As proposed there in this comment, this kicks off the process, to help us get acquainted with how the syncs should work, before working on improving the tooling.

Open questions:

• When performing a rustup, we will need to exclude the commits that were merged that same day, or else wait until that nightly is released. I did not update the documentation about this part, mainly because I'm not sure about how to do that.
• When should we perform the rustups now? My first idea is to do it at the same time we do the clippyups, to have a clear cadence and to avoid the two copies of the repo to diverge enough to make the process painful.
• Who does the rustups now? If we follow my previous idea and do both rustup and clippyup at the same time, it would be more work for @flip1995 who currently does the clippyups. I would prefer to establish some kind of rotation to spead the work. Other ideas?
• I'm not sure if this affects the release process in any way.
• ???

@rust-lang/clippy thoughts?

r? @flip1995

Implement https://github.com/rust-lang/rust-clippy/issues/8368 - a new lint to inline format arguments such as print!("{}", var) into print!("{var}").

Supported cases

code | suggestion | comment ---|---|--- print!("{}", var) | print!("{var}") | simple variables print!("{0}", var) | print!("{var}") | positional variables print!("{v}", v=var) | print!("{var}") | named variables print!("{0} {0}", var) | print!("{var} {var}") | aliased variables print!("{0:1$}", var, width) | print!("{var:width$}") | width support print!("{0:.1$}", var, prec) | print!("{var:.prec$}") | precision support print!("{:.*}", prec, var) | print!("{var:.prec$}") | asterisk support

Known Problems

• There may be a false positive if the format string is wrapped in a macro call:

# let var = 42;
macro_rules! no_param_str { () => { "{}" }; }
macro_rules! pass_through { ($expr:expr) => { $expr }; }
println!(no_param_str!(), var);
println!(pass_through!("{}"), var);

• Format string uses an indexed argument that cannot be inlined. Supporting this case requires re-indexing of the format string. Until implemented, print!("{0}={1}", var, 1+2) should be changed to print!("{var}={0}", 1+2) by hand.

changelog: [uninlined_format_args]: A new lint to inline format arguments, i.e. print!("{}", var) into print!("{var}")

The proposed enhancement causes needless_borrow to suggest removing & from &e when &e is an argument position requiring trait implementations, and e implements the required traits. Example:

error: the borrowed expression implements the required traits
  --> $DIR/needless_borrow.rs:131:51
   |
LL |     let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
   |                                                   ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`

r? @Jarcho

changelog: Enhance needless_borrow to consider trait implementations

Fixes #5203

There are two issues with this code that I have noticed:

• Use of Option::map_or causes it to always evaluate the code in the else block. If that block is computationally expensive or if it updates some state (such as getting the next value from an iterator), then this change would cause the code to behave differently. In either of those circumstances, it should suggest Option::map_or_else, which takes both cases as a closure and runs one. However, I don't know how to check if the expression would change some state, so I left the lint's applicability as MaybeIncorrect.

• There are lints which can trigger on specific sub-cases of this lint (if_let_some_result, question_mark, and while_let_loop) and suggest different changes (usually better ones because they're more specific). Is this acceptable for clippy to give multiple suggestions, or should I have the code check if those other lints trigger and then not trigger this lint if they do?

changelog: Add lint [option_if_let_else]

fixes #10136

This change makes the ContainsName struct visit all interior expressions, which means that ContainsName will return true even if name is used in a closure within expr.

changelog: [field_reassign_with_default]: fix a false positive where values captured in a closure were not detected as being used.

fixes #10122

As established in issue #9482, it is idiomatic to use a single drop() expression in a match arm to achieve a side-effect of a function while discarding its output. This should also apply to cases where the function returns a reference.

The change to the lint's code was less than 1 line, because all the heavy lifting was done in PR #9491.

changelog: [drop_ref]: Do not lint idiomatic expression in match arm

Summary

The field_reassign_with_default lint performs a check to make sure that the the reassignment does not involve the variable that was initialized with default(), but this check fails if the variable is captured by a closure.

Lint Name

field_reassign_with_default

Reproducer

I tried this code:

struct Collection {
    items: Vec<i32>,
    len: usize,
}

impl Default for Collection {
    fn default() -> Self {
        Self {
            items: vec![1, 2, 3],
            len: 0,
        }
    }
}

fn main() {
    let mut c = Collection::default();
    c.len = (|| c.items.len())();
}

I saw this happen:

warning: field assignment outside of initializer for an instance created with Default::default()
  --> src/main.rs:17:5
   |
17 |     c.len = (|| c.items.len())();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: consider initializing the variable with `Collection { len: (|| c.items.len())(), ..Default::default() }` and removing relevant reassignments
  --> src/main.rs:16:5
   |
16 |     let mut c = Collection::default();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
   = note: `#[warn(clippy::field_reassign_with_default)]` on by default

I expected to see this happen:

The lint should not trigger since the expression that c.len is being reassigned to involves c, making the suggestion invalid.

Version

rustc 1.66.0 (69f9c33d7 2022-12-12)
binary: rustc
commit-hash: 69f9c33d71c871fc16ac445211281c6e7a340943
commit-date: 2022-12-12
host: x86_64-unknown-linux-gnu
release: 1.66.0
LLVM version: 15.0.2

Additional Labels

@rustbot label +I-suggestion-causes-error