Repository: zkat/miette
Pull request #290 (2023-09-18 14:21:34)

Improve ci and fix clippy

waywardmonkeys: @zkat I agree! I can argue either side of it here, so I didn't know what to do. We can at least get most things fixed and I'll be happy to do what's needed in a follow up PR.

`rustix` is causing the problem and that's used by `is_terminal` ... so looking at that, `is_terminal` bumped MSRV in 0.4.8 from 1.48 to 1.63 where it is now (0.4.9).

We could modify the build for the MSRV to use `is_terminal` 0.4.7 perhaps?
zkat: ugh. See, it feels incredibly rude to do an MSRV bump in a _patch release_.

I'm ok with locking our `is_terminal` version to `0.4.7` and just create an issue for removing the lock when we're ready to do a semver-major for miette.
Benjamin-L: You might want to consider committing a lockfile that has version 0.4.7 of `is-terminal`, without bumping the version in `Cargo.toml`. My understanding is that a crate is considered to be compatible with a particular MSRV as long as it is possible to select dependency versions that are compatible with the MSRV and the constraints in `Cargo.toml`. Cargo usually won't find this set automatically. Hopefully some day we'll have a [MSRV-aware solver in cargo](https://github.com/rust-lang/cargo/issues/9930) and this whole problem will go away :)
waywardmonkeys: @Benjamin-L Interesting ... maybe a possible follow up?
Benjamin-L: I think pinning the dependency in `Cargo.toml` is a breaking change. For example, the following now fails to resolve:

```toml
[package]
name = "miette-290-test"
version = "0.1.0"
edition = "2021"

[dependencies]
is-terminal = "0.4.8"

[dependencies.miette]
git = "https://github.com/waywardmonkeys/miette.git"
branch = "improve-ci-and-fix-clippy"
features = [ "fancy" ]
```

The [semver docs][1] don't say anything about this situation, however the [cargo dependency docs][2] discourage upper bounds on library crates for this reason.

[1]: https://doc.rust-lang.org/cargo/reference/semver.html
[2]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-requirements
waywardmonkeys: It is after midnight here, so tomorrow, I will look at fixing this in the CI scripts, instead of pinning the version like I did here.
waywardmonkeys: @zkat @Benjamin-L I pushed a different way to resolve it. What do you think?

zkat: I'm fine with this.

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 4fafac3..912bfcc 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -10,14 +10,12 @@ jobs:
     name: Check fmt & build docs
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: stable
           components: rustfmt
-          override: true
       - name: rustfmt
         run: cargo fmt --all -- --check
       - name: docs
@@ -32,14 +30,15 @@ jobs:
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: ${{ matrix.rust }}
           components: clippy
-          override: true
+      - name: Force older version of is-terminal for MSRV builds
+        if: matrix.rust == '1.56.0'
+        run:  cargo update -p is-terminal --precise 0.4.7
       - name: Clippy
         run: cargo clippy --all -- -D warnings
       - name: Run tests
@@ -54,14 +53,12 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: nightly
           components: miri,rust-src
-          override: true
       - name: Run tests with miri
         env:
           MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance
@@ -75,13 +72,11 @@ jobs:
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
       - name: Install Rust
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@master
         with:
-          profile: minimal
           toolchain: nightly
-          override: true
       - name: Run minimal version build
         run: cargo build -Z minimal-versions --all-features
 
diff --git a/miette-derive/Cargo.toml b/miette-derive/Cargo.toml
index becdc17..b5a7032 100644
--- a/miette-derive/Cargo.toml
+++ b/miette-derive/Cargo.toml
@@ -11,6 +11,6 @@ repository = "https://github.com/zkat/miette"
 proc-macro = true
 
 [dependencies]
-proc-macro2 = "1.0"
+proc-macro2 = "1.0.60"
 quote = "1.0"
 syn = "2.0.11"
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index b5dd754..e61f595 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -382,10 +382,10 @@ impl GraphicalReportHandler {
         Ok(())
     }
 
-    fn render_context<'a>(
+    fn render_context(
         &self,
         f: &mut impl fmt::Write,
-        source: &'a dyn SourceCode,
+        source: &dyn SourceCode,
         context: &LabeledSpan,
         labels: &[LabeledSpan],
     ) -> fmt::Result {
diff --git a/src/miette_diagnostic.rs b/src/miette_diagnostic.rs
index 67b75d0..dc0468e 100644
--- a/src/miette_diagnostic.rs
+++ b/src/miette_diagnostic.rs
@@ -252,7 +252,7 @@ impl MietteDiagnostic {
     /// ```
     pub fn and_labels(mut self, labels: impl IntoIterator<Item = LabeledSpan>) -> Self {
         let mut all_labels = self.labels.unwrap_or_default();
-        all_labels.extend(labels.into_iter());
+        all_labels.extend(labels);
         self.labels = Some(all_labels);
         self
     }


Test patch:
diff --git a/tests/derive.rs b/tests/derive.rs
index 7faae42..dbaf7cb 100644
--- a/tests/derive.rs
+++ b/tests/derive.rs
@@ -189,7 +189,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x hello x \"2\"".to_string(),
-        FooStruct("hello".into()).help().unwrap().to_string()
+        FooStruct("hello").help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -201,12 +201,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x hello x \"2\"".to_string(),
-        BarStruct {
-            my_field: "hello".into()
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        BarStruct { my_field: "hello" }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -224,7 +219,7 @@ fn fmt_help() {
 
     assert_eq!(
         "1 x bar x \"2\"".to_string(),
-        FooEnum::X("bar".into()).help().unwrap().to_string()
+        FooEnum::X("bar").help().unwrap().to_string()
     );
 
     assert_eq!(
@@ -250,12 +245,7 @@ fn help_field() {
 
     assert_eq!(
         "x".to_string(),
-        Foo {
-            do_this: Some("x".into())
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        Foo { do_this: Some("x") }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -271,16 +261,11 @@ fn help_field() {
 
     assert_eq!(
         "x".to_string(),
-        Bar::A(Some("x".into())).help().unwrap().to_string()
+        Bar::A(Some("x")).help().unwrap().to_string()
     );
     assert_eq!(
         "x".to_string(),
-        Bar::B {
-            do_this: Some("x".into())
-        }
-        .help()
-        .unwrap()
-        .to_string()
+        Bar::B { do_this: Some("x") }.help().unwrap().to_string()
     );
 
     #[derive(Debug, Diagnostic, Error)]
@@ -288,20 +273,14 @@ fn help_field() {
     #[diagnostic()]
     struct Baz<'a>(#[help] Option<&'a str>);
 
-    assert_eq!(
-        "x".to_string(),
-        Baz(Some("x".into())).help().unwrap().to_string()
-    );
+    assert_eq!("x".to_string(), Baz(Some("x")).help().unwrap().to_string());
 
     #[derive(Debug, Diagnostic, Error)]
     #[error("welp")]
     #[diagnostic()]
     struct Quux<'a>(#[help] &'a str);
 
-    assert_eq!(
-        "x".to_string(),
-        Quux("x".into()).help().unwrap().to_string()
-    );
+    assert_eq!("x".to_string(), Quux("x").help().unwrap().to_string());
 }
 
 #[test]
diff --git a/tests/test_diagnostic_source_macro.rs b/tests/test_diagnostic_source_macro.rs
index 536aedf..df30b2e 100644
--- a/tests/test_diagnostic_source_macro.rs
+++ b/tests/test_diagnostic_source_macro.rs
@@ -79,7 +79,7 @@ fn test_diagnostic_source() {
 fn test_diagnostic_source_pass_extra_info() {
     let diag = TestBoxedError(Box::new(SourceError {
         code: String::from("Hello\nWorld!"),
-        help: format!("Have you tried turning it on and off again?"),
+        help: String::from("Have you tried turning it on and off again?"),
         label: (1, 4),
     }));
     let mut out = String::new();
diff --git a/tests/test_json.rs b/tests/test_json.rs
index 5bd14cb..ae482b8 100644
--- a/tests/test_json.rs
+++ b/tests/test_json.rs
@@ -52,7 +52,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -98,7 +97,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -144,7 +142,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -190,7 +187,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -235,7 +231,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -281,7 +276,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -347,7 +341,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -393,7 +386,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -456,7 +448,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -532,7 +523,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -588,7 +578,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -644,7 +633,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -700,7 +688,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -728,7 +715,6 @@ mod json_report_handler {
             "related": []
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -822,7 +808,6 @@ mod json_report_handler {
             }]
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);
@@ -920,7 +905,6 @@ mod json_report_handler {
             }]
         }"#
         .lines()
-        .into_iter()
         .map(|s| s.trim_matches(|c| c == ' ' || c == '\n'))
         .collect();
         assert_eq!(expected, out);