Skip to content

WIP: fix(shell): Switch to annotate snippets for warnings #15917

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

WIP: fix(shell): Switch to annotate snippets for warnings #15917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/cargo/core/shell.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we will need to rework on most of our messages. Not all the help and note should be under the same group.

Copy link
Member

Choose a reason for hiding this comment

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

Couldn't find a concrete example, but conceptually adding indentations to trailing lines would add efforts for people to remove those indentations when copy-pasting.

Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,10 @@ impl Shell {

/// Prints an amber 'warning' message.
pub fn warn<T: fmt::Display>(&mut self, message: T) -> CargoResult<()> {
self.print(&"warning", Some(&message), &WARN, false)
let report = &[annotate_snippets::Group::with_title(
annotate_snippets::Level::WARNING.secondary_title(message.to_string()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used secondary_title (custom styling) as a short term hack because the callers may be passing in styled text to workaround the fact that we don't have annotate snippets. As we migrate things, we'll be able to use primary_title (no custom styling) as we should.

)];
self.print_report(report, false)
}

/// Prints a cyan 'note' message.
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/alt_registry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ fn publish_with_registry_dependency() {
.with_stderr_data(str![[r#"
[UPDATING] `alternative` index
[WARNING] manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
[UPDATING] `alternative` index
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
Expand Down Expand Up @@ -517,7 +517,7 @@ fn publish_to_alt_registry() {
.with_stderr_data(str![[r#"
[UPDATING] `alternative` index
[WARNING] manifest has no description, license, license-file, documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
[VERIFYING] foo v0.0.1 ([ROOT]/foo)
Expand Down Expand Up @@ -595,7 +595,7 @@ fn publish_with_crates_io_dep() {
.with_stderr_data(str![[r#"
[UPDATING] `alternative` index
[WARNING] manifest has no documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
[UPDATING] `dummy-registry` index
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
Expand Down
86 changes: 43 additions & 43 deletions tests/testsuite/bad_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ fn dev_dependencies2() {
.build();
p.cargo("check").with_stderr_data(str![[r#"
[WARNING] `dev_dependencies` is deprecated in favor of `dev-dependencies` and will not work in the 2024 edition
(in the `foo` package)
(in the `foo` package)
[LOCKING] 1 package to latest compatible version
[CHECKING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand Down Expand Up @@ -954,7 +954,7 @@ fn build_dependencies2() {
.build();
p.cargo("check").with_stderr_data(str![[r#"
[WARNING] `build_dependencies` is deprecated in favor of `build-dependencies` and will not work in the 2024 edition
(in the `foo` package)
(in the `foo` package)
[LOCKING] 1 package to latest compatible version
[CHECKING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand Down Expand Up @@ -1062,7 +1062,7 @@ fn lib_crate_type2() {
p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `crate_type` is deprecated in favor of `crate-type` and will not work in the 2024 edition
(in the `foo` library target)
(in the `foo` library target)
[CHECKING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -1153,7 +1153,7 @@ fn bin_crate_type2() {
p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `crate_type` is deprecated in favor of `crate-type` and will not work in the 2024 edition
(in the `foo` binary target)
(in the `foo` binary target)
[CHECKING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -1262,9 +1262,9 @@ fn examples_crate_type2() {
p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `crate_type` is deprecated in favor of `crate-type` and will not work in the 2024 edition
(in the `ex` example target)
(in the `ex` example target)
[WARNING] `crate_type` is deprecated in favor of `crate-type` and will not work in the 2024 edition
(in the `goodbye` example target)
(in the `goodbye` example target)
[CHECKING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -1401,7 +1401,7 @@ fn cargo_platform_build_dependencies2() {
p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `build_dependencies` is deprecated in favor of `build-dependencies` and will not work in the 2024 edition
(in the `[HOST_TARGET]` platform target)
(in the `[HOST_TARGET]` platform target)
[LOCKING] 1 package to latest compatible version
[COMPILING] build v0.5.0 ([ROOT]/foo/build)
[COMPILING] foo v0.5.0 ([ROOT]/foo)
Expand Down Expand Up @@ -1532,7 +1532,7 @@ fn cargo_platform_dev_dependencies2() {
p.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `dev_dependencies` is deprecated in favor of `dev-dependencies` and will not work in the 2024 edition
(in the `[HOST_TARGET]` platform target)
(in the `[HOST_TARGET]` platform target)
[LOCKING] 1 package to latest compatible version
[CHECKING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand Down Expand Up @@ -1661,7 +1661,7 @@ fn default_features2() {

p.cargo("check").with_stderr_data(str![[r#"
[WARNING] `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `a` dependency)
(in the `a` dependency)
[LOCKING] 1 package to latest compatible version
[CHECKING] a v0.1.0 ([ROOT]/foo/a)
[CHECKING] foo v0.1.0 ([ROOT]/foo)
Expand Down Expand Up @@ -1836,7 +1836,7 @@ fn workspace_default_features2() {
[CHECKING] workspace_only v0.1.0 ([ROOT]/foo/workspace_only)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[WARNING] [ROOT]/foo/workspace_only/Cargo.toml: `default_features` is deprecated in favor of `default-features` and will not work in the 2024 edition
(in the `dep_workspace_only` dependency)
(in the `dep_workspace_only` dependency)

"#]]
.unordered(),
Expand Down Expand Up @@ -1947,7 +1947,7 @@ fn lib_proc_macro2() {
foo.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `proc_macro` is deprecated in favor of `proc-macro` and will not work in the 2024 edition
(in the `foo` library target)
(in the `foo` library target)
[CHECKING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -2035,7 +2035,7 @@ fn bin_proc_macro2() {
foo.cargo("check")
.with_stderr_data(str![[r#"
[WARNING] `proc_macro` is deprecated in favor of `proc-macro` and will not work in the 2024 edition
(in the `foo` binary target)
(in the `foo` binary target)
[CHECKING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -3473,22 +3473,22 @@ fn legacy_binary_paths_warnings() {
p.cargo("check -v")
.with_stderr_data(str![[r#"
[WARNING] An explicit [[bin]] section is specified in Cargo.toml which currently
disables Cargo from automatically inferring other binary targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a binary target:

* src/main.rs

This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a binary target today. You can future-proof yourself
and disable this warning by adding `autobins = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.

For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
disables Cargo from automatically inferring other binary targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a binary target:
* src/main.rs
This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a binary target today. You can future-proof yourself
and disable this warning by adding `autobins = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.
For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
[WARNING] path `src/main.rs` was erroneously implicitly accepted for binary `bar`,
please set bin.path in Cargo.toml
please set bin.path in Cargo.toml
[CHECKING] foo v1.0.0 ([ROOT]/foo)
[RUNNING] `rustc [..]`
[RUNNING] `rustc [..]`
Expand Down Expand Up @@ -3518,22 +3518,22 @@ please set bin.path in Cargo.toml
p.cargo("check -v")
.with_stderr_data(str![[r#"
[WARNING] An explicit [[bin]] section is specified in Cargo.toml which currently
disables Cargo from automatically inferring other binary targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a binary target:

* src/bin/main.rs

This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a binary target today. You can future-proof yourself
and disable this warning by adding `autobins = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.

For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
disables Cargo from automatically inferring other binary targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a binary target:
* src/bin/main.rs
This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a binary target today. You can future-proof yourself
and disable this warning by adding `autobins = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.
For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
[WARNING] path `src/bin/main.rs` was erroneously implicitly accepted for binary `bar`,
please set bin.path in Cargo.toml
please set bin.path in Cargo.toml
[CHECKING] foo v1.0.0 ([ROOT]/foo)
[RUNNING] `rustc [..]`
[RUNNING] `rustc [..]`
Expand Down Expand Up @@ -3562,7 +3562,7 @@ please set bin.path in Cargo.toml
p.cargo("check -v")
.with_stderr_data(str![[r#"
[WARNING] path `src/bar.rs` was erroneously implicitly accepted for binary `bar`,
please set bin.path in Cargo.toml
please set bin.path in Cargo.toml
[CHECKING] foo v1.0.0 ([ROOT]/foo)
[RUNNING] `rustc [..]`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
Expand Down
30 changes: 15 additions & 15 deletions tests/testsuite/bench.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,20 +716,20 @@ fn bench_autodiscover_2015() {
p.cargo("bench bench_basic")
.with_stderr_data(str![[r#"
[WARNING] An explicit [[bench]] section is specified in Cargo.toml which currently
disables Cargo from automatically inferring other benchmark targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a benchmark target:

* [..]bench_basic.rs

This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a benchmark target today. You can future-proof yourself
and disable this warning by adding `autobenches = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.

For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
disables Cargo from automatically inferring other benchmark targets.
This inference behavior will change in the Rust 2018 edition and the following
files will be included as a benchmark target:
* benches/bench_basic.rs
This is likely to break cargo build or cargo test as these files may not be
ready to be compiled as a benchmark target today. You can future-proof yourself
and disable this warning by adding `autobenches = false` to your [package]
section. You may also move the files to a location where Cargo would not
automatically infer them to be a target, such as in subfolders.
For more information on this warning you can consult
https://github.com/rust-lang/cargo/issues/5330
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `bench` profile [optimized] target(s) in [ELAPSED]s
[RUNNING] [..] (target/release/deps/foo-[HASH][EXE])
Expand Down Expand Up @@ -1944,7 +1944,7 @@ fn legacy_bench_name() {
p.cargo("bench")
.with_stderr_data(str![[r#"
[WARNING] path `src/bench.rs` was erroneously implicitly accepted for benchmark `bench`,
please set bench.path in Cargo.toml
please set bench.path in Cargo.toml
[COMPILING] foo v0.1.0 ([ROOT]/foo)
[FINISHED] `bench` profile [optimized] target(s) in [ELAPSED]s
[RUNNING] unittests src/lib.rs (target/release/deps/foo-[HASH][EXE])
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ fn cargo_compile_duplicate_build_targets() {
p.cargo("build")
.with_stderr_data(str![[r#"
[WARNING] file `[ROOT]/foo/src/main.rs` found to be present in multiple build targets:
* `lib` target `main`
* `bin` target `foo`
* `lib` target `main`
* `bin` target `foo`
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -1992,7 +1992,7 @@ fn many_crate_types_old_style_lib_location() {
p.cargo("build")
.with_stderr_data(str![[r#"
[WARNING] path `src/foo.rs` was erroneously implicitly accepted for library `foo`,
please rename the file to `src/lib.rs` or set lib.path in Cargo.toml
please rename the file to `src/lib.rs` or set lib.path in Cargo.toml
[COMPILING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/build_script_env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ fn rustc_bootstrap() {
.with_stderr_data(str![[r#"
[COMPILING] has-dashes v0.0.1 ([ROOT]/foo)
[WARNING] [email protected]: Cannot set `RUSTC_BOOTSTRAP=1` from build script of `has-dashes v0.0.1 ([ROOT]/foo)`.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
Expand All @@ -155,7 +155,7 @@ fn rustc_bootstrap() {
.env("RUSTC_BOOTSTRAP", "has_dashes")
.with_stderr_data(str![[r#"
[WARNING] [email protected]: Cannot set `RUSTC_BOOTSTRAP=1` from build script of `has-dashes v0.0.1 ([ROOT]/foo)`.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
Expand Down Expand Up @@ -190,7 +190,7 @@ fn rustc_bootstrap() {
.with_stderr_data(str![[r#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[WARNING] [email protected]: Cannot set `RUSTC_BOOTSTRAP=1` from build script of `foo v0.0.1 ([ROOT]/foo)`.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[NOTE] Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

"#]])
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/build_script_extra_link_arg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,9 @@ fn cdylib_link_arg_transitive() {
[RUNNING] `[ROOT]/foo/target/debug/build/bar-[HASH]/build-script-build`
[WARNING] [email protected]: cargo::rustc-link-arg-cdylib was specified in the build script of bar v1.0.0 \
([ROOT]/foo/bar), but that package does not contain a cdylib target
Allowing this was an unintended change in the 1.50 release, and may become an error in \
the future. For more information, see <https://github.com/rust-lang/cargo/issues/9562>.
Allowing this was an unintended change in the 1.50 release, and may become an error in \
the future. For more information, see <https://github.com/rust-lang/cargo/issues/9562>.
[RUNNING] `rustc --crate-name bar --edition=2015 bar/src/lib.rs [..]
[COMPILING] foo v0.1.0 ([ROOT]/foo)
[RUNNING] `rustc --crate-name foo --edition=2015 src/lib.rs [..]-C link-arg=--bogus[..]`
Expand Down
12 changes: 6 additions & 6 deletions tests/testsuite/cargo_alias_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ fn alias_shadowing_external_subcommand() {
.env("PATH", &path)
.with_stderr_data(str![[r#"
[WARNING] user-defined alias `echo` is shadowing an external subcommand found at: `[ROOT]/cargo-echo/target/debug/cargo-echo[EXE]`
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
[COMPILING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Expand Down Expand Up @@ -226,8 +226,8 @@ fn default_args_alias() {
.with_status(101)
.with_stderr_data(str![[r#"
[WARNING] user-defined alias `echo` is shadowing an external subcommand found at: `[ROOT]/cargo-echo/target/debug/cargo-echo[EXE]`
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
[ERROR] alias echo has unresolvable recursive definition: echo -> echo

"#]])
Expand All @@ -238,8 +238,8 @@ For more information, see issue #10049 <https://github.com/rust-lang/cargo/issue
.with_status(101)
.with_stderr_data(str![[r#"
[WARNING] user-defined alias `echo` is shadowing an external subcommand found at: `[ROOT]/cargo-echo/target/debug/cargo-echo[EXE]`
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
This was previously accepted but is being phased out; it will become a hard error in a future release.
For more information, see issue #10049 <https://github.com/rust-lang/cargo/issues/10049>.
[ERROR] alias test-1 has unresolvable recursive definition: test-1 -> echo -> echo

"#]])
Expand Down
Loading
Loading