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

Display errors when cargo fix fails. #6419

Merged
merged 2 commits into from
Dec 14, 2018
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Dec 12, 2018

It can be difficult to figure out what's wrong when a user reports that cargo fix fails. It can be hard to figure out which suggestion caused a compile error, especially if the error is in another file/location.

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@ehuss
Copy link
Contributor Author

ehuss commented Dec 12, 2018

cc @killercup

  • I'm not sure if it was intentional to not show the errors, but I think they would be useful.
  • An update to rustfix will be needed before this can be merged. I needed pub for the rendered diagnostic (ehuss/rustfix@d904271).
  • This also updates broken_fixes_backed_out. In Warn users when fixes break code rustfix#93 the check was removed for whether or not it actually backed out the changes. I'm not sure why. I've updated it so that it actually checks whether or not it was backed out.
  • I'm a bit surprised that cargo fix exits 0 with a warning when it fails. Is there some reason it's not a error?

Here's an example of what the full output looks like:

    Checking badfix v0.1.0 (.../badfix)
warning: failed to automatically apply fixes suggested by rustc to crate `badfix`

after fixes were automatically applied the compiler reported errors within these files:

  * src/main.rs

This likely indicates a bug in either rustc or cargo itself,
and we would appreciate a bug report! You're likely to see
a number of compiler warnings after this message which cargo
attempted to fix but failed. If you could open an issue at
https://github.com/rust-lang/cargo/issues
quoting the full output of this command we'd be very appreciative!

The following errors were reported:
error[E0408]: variable `_x` is not bound in all patterns
 --> src/main.rs:7:20
  |
7 |         E::A(_x) | E::B(x) => {}
  |              --    ^^^^^^^ pattern doesn't bind `_x`
  |              |
  |              variable not in all patterns

error[E0408]: variable `x` is not bound in all patterns
 --> src/main.rs:7:9
  |
7 |         E::A(_x) | E::B(x) => {}
  |         ^^^^^^^^        - variable not in all patterns
  |         |
  |         pattern doesn't bind `x`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0408`.
Original diagnostics will follow.

warning: unused variable: `x`
 --> src/main.rs:7:14
  |
7 |         E::A(x) | E::B(x) => {}
  |              ^ help: consider using `_x` instead
  |
  = note: #[warn(unused_variables)] on by default

warning: variant is never constructed: `B`
 --> src/main.rs:4:9
  |
4 |         B(i32,),
  |         ^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: unused variable: `x`
 --> src/main.rs:7:14
  |
7 |         E::A(x) | E::B(x) => {}
  |              ^ help: consider using `_x` instead
  |
  = note: #[warn(unused_variables)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 1.02s

@alexcrichton
Copy link
Member

I'm not sure if it was intentional to not show the errors, but I think they would be useful.

Definitely looks useful to me, these look like much better diagnostics!

I've updated it so that it actually checks whether or not it was backed out.

Makes sense!

I'm a bit surprised that cargo check exits 0 with a warning when it fails. Is there some reason it's not a error?

I think this is because cargo check actually succeeds (the underlying implementation) because the broken code was backed out. Seems plausible to me to change though

It can be difficult to figure out what's wrong when a user reports that
`cargo fix` fails. There's often a large list of warnings, and it can
be hard to figure out which one caused a compile error.
@ehuss
Copy link
Contributor Author

ehuss commented Dec 13, 2018

Updated with new rustfix.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Dec 13, 2018

📌 Commit 502ab65 has been approved by alexcrichton

bors added a commit that referenced this pull request Dec 14, 2018
Display errors when `cargo fix` fails.

It can be difficult to figure out what's wrong when a user reports that `cargo fix` fails. It can be hard to figure out which suggestion caused a compile error, especially if the error is in another file/location.
@bors
Copy link
Contributor

bors commented Dec 14, 2018

⌛ Testing commit 502ab65 with merge a3a3c25...

@bors
Copy link
Contributor

bors commented Dec 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing a3a3c25 to master...

@bors bors merged commit 502ab65 into rust-lang:master Dec 14, 2018
bors added a commit to rust-lang/rust that referenced this pull request Dec 29, 2018
Update cargo, rls, miri

Update cargo, rls, miri

Added `rustc-workspace-hack` to miri so that it shares the same features for serde as other tools.

cc @alexcrichton

## cargo

25 commits in 2cf1f5dda2f7ed84e94c4d32f643e0f1f15352f0..0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4
2018-12-11 03:44:04 +0000 to 2018-12-19 14:45:14 +0000
- Remove Stale bot's configuration (rust-lang/cargo#6463)
- Add labels to issue templates (rust-lang/cargo#6464)
- Fix new man page links. (rust-lang/cargo#6459)
- Fix metabuild compile errors with --message-format=json. (rust-lang/cargo#6432)
- Support alt-registry names in [patch] table. (rust-lang/cargo#6456)
- Update the rustup URL (rust-lang/cargo#6455)
- New man pages. (rust-lang/cargo#6405)
- Reify the DepFingerprint type (rust-lang/cargo#6451)
- Extract Fingerprint::new (rust-lang/cargo#6449)
- Upgrade the metabuild to Rust 2018 (rust-lang/cargo#6448)
- Make edition comparing code consistent (rust-lang/cargo#6450)
- Document `name` and `authors` in [package] (rust-lang/cargo#6447)
- Travis: only use mdbook 0.1.7. (rust-lang/cargo#6443)
- Update git2-curl requirement from 0.8.1 to 0.9.0 (rust-lang/cargo#6439)
- Update git2 requirement from 0.7.5 to 0.8.0 (rust-lang/cargo#6438)
- Display errors when `cargo fix` fails. (rust-lang/cargo#6419)
- cargo fix: fix targets with shared sources. (rust-lang/cargo#6434)
- Fix panic-in-panic in tests. (rust-lang/cargo#6431)
- More Rust 2018 edition cleanups (rust-lang/cargo#6422)
- Cleanup some trait impls for SourceId (rust-lang/cargo#6429)
- Remove a nightly check from doc tests (rust-lang/cargo#6427)
- Replace CargoError with failure::Error (rust-lang/cargo#6425)
- Allow testsuite warnings in dev (rust-lang/cargo#6426)
- add `--dry-run` option to cargo update (rust-lang/cargo#6371)
- Migrate to some Rust 2018 idioms (rust-lang/cargo#6416)

## rls

16 commits in bd5b899afb05e14d33e210ede3da241ca1ca088f..6f5e4bba7b1586fca6e0ea7724cadb5683b2f308
2018-12-10 08:53:00 +0100 to 2018-12-21 17:11:08 +0100
- Update jsonrpc-core (rust-lang/rls#1206)
- Use `home_dir` from `home` crate (rust-lang/rls#1207)
- Update cargo. (rust-lang/rls#1204)
- Fix deprecated `trim_{left,right}` warnings (rust-lang/rls#1203)
- Respect ${CARGO,RUSTUP}_HOME for tooltip relative dirs (rust-lang/rls#1201)
- Separate tooltip tests that require Racer fallback (rust-lang/rls#1200)
- tests: Don't generate tooltip results in tests/fixtures (rust-lang/rls#1199)
- Overhaul fixture handling in tests (rust-lang/rls#1190)
- Don't return symbols with empty names (rust-lang/rls#1193)
- Don't check AppVeyor CI status for bors
- Properly infer full_docs (rust-lang/rls#1192)
- Update cargo (rust-lang/rls#1191)
- Improve hover test_tooltip tests (rust-lang/rls#1175)
- Fix unused warnings (rust-lang/rls#1185)
- Workaround rust-lang/rls#703 to prevent obscure failures due to sccache. (rust-lang/rls#1177)
- Disable travis cache (rust-lang/rls#1182)

## miri

14 commits in bccadeb..6c2fc6d
2018-12-08 11:07:22 +0100 to 2018-12-26 14:28:25 +0100
- use memory::check_bounds_ptr for offset check (rust-lang/miri#589)
- Fix comparing function pointers (rust-lang/miri#587)
- fix for infallible allocation (rust-lang/miri#586)
- fix test for latest nightly (rust-lang/miri#585)
- Treat ref-to-raw cast like a reborrow: do a special kind of retag (rust-lang/miri#572)
- Test cargo-miri on Windows (rust-lang/miri#578)
- Cargo miri tweaks and test that we can exclude tests (rust-lang/miri#580)
- Fix cargo miri test (rust-lang/miri#550)
- fix for latest nightly (rust-lang/miri#574)
- Add rustc-workspace-hack. (rust-lang/miri#575)
- use RUSTC_WRAPPER for the cargo hook (rust-lang/miri#573)
- do not auto-detect the targets in the sysroot, instead specify target manually through env var (rust-lang/miri#570)
- Cleanup: Avoid repeating signatures, get rid of to_bytes hack (rust-lang/miri#568)
- Support building and running with full MIR on foreign architectures, drop support for missing MIR (rust-lang/miri#566)
@ehuss ehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants