Actually fail on clippy failures (#7619)

Kirill Bulatov and Mikayla Maki created

Before the change to `script/clippy`, bash ignored first `clippy`
invocation failure and CI moved on with Linux errors and warnings
emitted.


Release Notes:

- N/A

---------

Co-authored-by: Mikayla Maki <mikayla@zed.dev>

Change summary

crates/cli/src/main.rs                           | 320 ++++++++++-------
crates/gpui/src/platform/linux/blade_belt.rs     |   4 
crates/gpui/src/platform/linux/blade_renderer.rs |   4 
crates/gpui/src/platform/linux/dispatcher.rs     |   2 
crates/gpui/src/platform/linux/platform.rs       |  10 
crates/gpui/src/platform/linux/window.rs         |   6 
crates/zed/src/main.rs                           |   6 
script/clippy                                    |   4 
8 files changed, 205 insertions(+), 151 deletions(-)

Detailed changes

crates/cli/src/main.rs 🔗

@@ -1,20 +1,14 @@
+#![cfg_attr(target_os = "linux", allow(dead_code))]
+
 use anyhow::{anyhow, Context, Result};
 use clap::Parser;
-use cli::{CliRequest, CliResponse, IpcHandshake, FORCE_CLI_MODE_ENV_VAR_NAME};
-use core_foundation::{
-    array::{CFArray, CFIndex},
-    string::kCFStringEncodingUTF8,
-    url::{CFURLCreateWithBytes, CFURL},
-};
-use core_services::{kLSLaunchDefaults, LSLaunchURLSpec, LSOpenFromURLSpec, TCFType};
-use ipc_channel::ipc::{IpcOneShotServer, IpcReceiver, IpcSender};
+use cli::{CliRequest, CliResponse};
 use serde::Deserialize;
 use std::{
     ffi::OsStr,
     fs::{self, OpenOptions},
     io,
     path::{Path, PathBuf},
-    ptr,
 };
 use util::paths::PathLikeWithPosition;
 
@@ -112,150 +106,204 @@ enum Bundle {
     },
 }
 
-impl Bundle {
-    fn detect(args_bundle_path: Option<&Path>) -> anyhow::Result<Self> {
-        let bundle_path = if let Some(bundle_path) = args_bundle_path {
-            bundle_path
-                .canonicalize()
-                .with_context(|| format!("Args bundle path {bundle_path:?} canonicalization"))?
-        } else {
-            locate_bundle().context("bundle autodiscovery")?
-        };
-
-        match bundle_path.extension().and_then(|ext| ext.to_str()) {
-            Some("app") => {
-                let plist_path = bundle_path.join("Contents/Info.plist");
-                let plist = plist::from_file::<_, InfoPlist>(&plist_path).with_context(|| {
-                    format!("Reading *.app bundle plist file at {plist_path:?}")
-                })?;
-                Ok(Self::App {
-                    app_bundle: bundle_path,
-                    plist,
-                })
-            }
-            _ => {
-                println!("Bundle path {bundle_path:?} has no *.app extension, attempting to locate a dev build");
-                let plist_path = bundle_path
-                    .parent()
-                    .with_context(|| format!("Bundle path {bundle_path:?} has no parent"))?
-                    .join("WebRTC.framework/Resources/Info.plist");
-                let plist = plist::from_file::<_, InfoPlist>(&plist_path)
-                    .with_context(|| format!("Reading dev bundle plist file at {plist_path:?}"))?;
-                Ok(Self::LocalPath {
-                    executable: bundle_path,
-                    plist,
-                })
-            }
-        }
+fn touch(path: &Path) -> io::Result<()> {
+    match OpenOptions::new().create(true).write(true).open(path) {
+        Ok(_) => Ok(()),
+        Err(e) => Err(e),
     }
+}
 
-    fn plist(&self) -> &InfoPlist {
-        match self {
-            Self::App { plist, .. } => plist,
-            Self::LocalPath { plist, .. } => plist,
+fn locate_bundle() -> Result<PathBuf> {
+    let cli_path = std::env::current_exe()?.canonicalize()?;
+    let mut app_path = cli_path.clone();
+    while app_path.extension() != Some(OsStr::new("app")) {
+        if !app_path.pop() {
+            return Err(anyhow!("cannot find app bundle containing {:?}", cli_path));
         }
     }
+    Ok(app_path)
+}
+
+#[cfg(target_os = "linux")]
+mod linux {
+    use std::path::Path;
+
+    use cli::{CliRequest, CliResponse};
+    use ipc_channel::ipc::{IpcReceiver, IpcSender};
+
+    use crate::{Bundle, InfoPlist};
+
+    impl Bundle {
+        pub fn detect(_args_bundle_path: Option<&Path>) -> anyhow::Result<Self> {
+            unimplemented!()
+        }
 
-    fn path(&self) -> &Path {
-        match self {
-            Self::App { app_bundle, .. } => app_bundle,
-            Self::LocalPath { executable, .. } => executable,
+        pub fn plist(&self) -> &InfoPlist {
+            unimplemented!()
+        }
+
+        pub fn path(&self) -> &Path {
+            unimplemented!()
+        }
+
+        pub fn launch(&self) -> anyhow::Result<(IpcSender<CliRequest>, IpcReceiver<CliResponse>)> {
+            unimplemented!()
+        }
+
+        pub fn zed_version_string(&self) -> String {
+            unimplemented!()
         }
     }
+}
 
-    fn launch(&self) -> anyhow::Result<(IpcSender<CliRequest>, IpcReceiver<CliResponse>)> {
-        let (server, server_name) =
-            IpcOneShotServer::<IpcHandshake>::new().context("Handshake before Zed spawn")?;
-        let url = format!("zed-cli://{server_name}");
-
-        match self {
-            Self::App { app_bundle, .. } => {
-                let app_path = app_bundle;
-
-                let status = unsafe {
-                    let app_url = CFURL::from_path(app_path, true)
-                        .with_context(|| format!("invalid app path {app_path:?}"))?;
-                    let url_to_open = CFURL::wrap_under_create_rule(CFURLCreateWithBytes(
-                        ptr::null(),
-                        url.as_ptr(),
-                        url.len() as CFIndex,
-                        kCFStringEncodingUTF8,
-                        ptr::null(),
-                    ));
-                    // equivalent to: open zed-cli:... -a /Applications/Zed\ Preview.app
-                    let urls_to_open = CFArray::from_copyable(&[url_to_open.as_concrete_TypeRef()]);
-                    LSOpenFromURLSpec(
-                        &LSLaunchURLSpec {
-                            appURL: app_url.as_concrete_TypeRef(),
-                            itemURLs: urls_to_open.as_concrete_TypeRef(),
-                            passThruParams: ptr::null(),
-                            launchFlags: kLSLaunchDefaults,
-                            asyncRefCon: ptr::null_mut(),
-                        },
-                        ptr::null_mut(),
-                    )
-                };
+#[cfg(target_os = "macos")]
+mod mac_os {
+    use anyhow::Context;
+    use core_foundation::{
+        array::{CFArray, CFIndex},
+        string::kCFStringEncodingUTF8,
+        url::{CFURLCreateWithBytes, CFURL},
+    };
+    use core_services::{kLSLaunchDefaults, LSLaunchURLSpec, LSOpenFromURLSpec, TCFType};
+    use std::{fs, path::Path, ptr};
+
+    use cli::{CliRequest, CliResponse, IpcHandshake, FORCE_CLI_MODE_ENV_VAR_NAME};
+    use ipc_channel::ipc::{IpcOneShotServer, IpcReceiver, IpcSender};
+
+    use crate::{locate_bundle, Bundle, InfoPlist};
+
+    impl Bundle {
+        pub fn detect(args_bundle_path: Option<&Path>) -> anyhow::Result<Self> {
+            let bundle_path = if let Some(bundle_path) = args_bundle_path {
+                bundle_path
+                    .canonicalize()
+                    .with_context(|| format!("Args bundle path {bundle_path:?} canonicalization"))?
+            } else {
+                locate_bundle().context("bundle autodiscovery")?
+            };
 
-                anyhow::ensure!(
-                    status == 0,
-                    "cannot start app bundle {}",
-                    self.zed_version_string()
-                );
+            match bundle_path.extension().and_then(|ext| ext.to_str()) {
+                Some("app") => {
+                    let plist_path = bundle_path.join("Contents/Info.plist");
+                    let plist =
+                        plist::from_file::<_, InfoPlist>(&plist_path).with_context(|| {
+                            format!("Reading *.app bundle plist file at {plist_path:?}")
+                        })?;
+                    Ok(Self::App {
+                        app_bundle: bundle_path,
+                        plist,
+                    })
+                }
+                _ => {
+                    println!("Bundle path {bundle_path:?} has no *.app extension, attempting to locate a dev build");
+                    let plist_path = bundle_path
+                        .parent()
+                        .with_context(|| format!("Bundle path {bundle_path:?} has no parent"))?
+                        .join("WebRTC.framework/Resources/Info.plist");
+                    let plist =
+                        plist::from_file::<_, InfoPlist>(&plist_path).with_context(|| {
+                            format!("Reading dev bundle plist file at {plist_path:?}")
+                        })?;
+                    Ok(Self::LocalPath {
+                        executable: bundle_path,
+                        plist,
+                    })
+                }
             }
+        }
 
-            Self::LocalPath { executable, .. } => {
-                let executable_parent = executable
-                    .parent()
-                    .with_context(|| format!("Executable {executable:?} path has no parent"))?;
-                let subprocess_stdout_file =
-                    fs::File::create(executable_parent.join("zed_dev.log"))
-                        .with_context(|| format!("Log file creation in {executable_parent:?}"))?;
-                let subprocess_stdin_file =
-                    subprocess_stdout_file.try_clone().with_context(|| {
-                        format!("Cloning descriptor for file {subprocess_stdout_file:?}")
-                    })?;
-                let mut command = std::process::Command::new(executable);
-                let command = command
-                    .env(FORCE_CLI_MODE_ENV_VAR_NAME, "")
-                    .stderr(subprocess_stdout_file)
-                    .stdout(subprocess_stdin_file)
-                    .arg(url);
-
-                command
-                    .spawn()
-                    .with_context(|| format!("Spawning {command:?}"))?;
+        fn plist(&self) -> &InfoPlist {
+            match self {
+                Self::App { plist, .. } => plist,
+                Self::LocalPath { plist, .. } => plist,
             }
         }
 
-        let (_, handshake) = server.accept().context("Handshake after Zed spawn")?;
-        Ok((handshake.requests, handshake.responses))
-    }
+        fn path(&self) -> &Path {
+            match self {
+                Self::App { app_bundle, .. } => app_bundle,
+                Self::LocalPath { executable, .. } => executable,
+            }
+        }
 
-    fn zed_version_string(&self) -> String {
-        let is_dev = matches!(self, Self::LocalPath { .. });
-        format!(
-            "Zed {}{} – {}",
-            self.plist().bundle_short_version_string,
-            if is_dev { " (dev)" } else { "" },
-            self.path().display(),
-        )
-    }
-}
+        pub fn launch(&self) -> anyhow::Result<(IpcSender<CliRequest>, IpcReceiver<CliResponse>)> {
+            let (server, server_name) =
+                IpcOneShotServer::<IpcHandshake>::new().context("Handshake before Zed spawn")?;
+            let url = format!("zed-cli://{server_name}");
 
-fn touch(path: &Path) -> io::Result<()> {
-    match OpenOptions::new().create(true).write(true).open(path) {
-        Ok(_) => Ok(()),
-        Err(e) => Err(e),
-    }
-}
+            match self {
+                Self::App { app_bundle, .. } => {
+                    let app_path = app_bundle;
 
-fn locate_bundle() -> Result<PathBuf> {
-    let cli_path = std::env::current_exe()?.canonicalize()?;
-    let mut app_path = cli_path.clone();
-    while app_path.extension() != Some(OsStr::new("app")) {
-        if !app_path.pop() {
-            return Err(anyhow!("cannot find app bundle containing {:?}", cli_path));
+                    let status = unsafe {
+                        let app_url = CFURL::from_path(app_path, true)
+                            .with_context(|| format!("invalid app path {app_path:?}"))?;
+                        let url_to_open = CFURL::wrap_under_create_rule(CFURLCreateWithBytes(
+                            ptr::null(),
+                            url.as_ptr(),
+                            url.len() as CFIndex,
+                            kCFStringEncodingUTF8,
+                            ptr::null(),
+                        ));
+                        // equivalent to: open zed-cli:... -a /Applications/Zed\ Preview.app
+                        let urls_to_open =
+                            CFArray::from_copyable(&[url_to_open.as_concrete_TypeRef()]);
+                        LSOpenFromURLSpec(
+                            &LSLaunchURLSpec {
+                                appURL: app_url.as_concrete_TypeRef(),
+                                itemURLs: urls_to_open.as_concrete_TypeRef(),
+                                passThruParams: ptr::null(),
+                                launchFlags: kLSLaunchDefaults,
+                                asyncRefCon: ptr::null_mut(),
+                            },
+                            ptr::null_mut(),
+                        )
+                    };
+
+                    anyhow::ensure!(
+                        status == 0,
+                        "cannot start app bundle {}",
+                        self.zed_version_string()
+                    );
+                }
+
+                Self::LocalPath { executable, .. } => {
+                    let executable_parent = executable
+                        .parent()
+                        .with_context(|| format!("Executable {executable:?} path has no parent"))?;
+                    let subprocess_stdout_file = fs::File::create(
+                        executable_parent.join("zed_dev.log"),
+                    )
+                    .with_context(|| format!("Log file creation in {executable_parent:?}"))?;
+                    let subprocess_stdin_file =
+                        subprocess_stdout_file.try_clone().with_context(|| {
+                            format!("Cloning descriptor for file {subprocess_stdout_file:?}")
+                        })?;
+                    let mut command = std::process::Command::new(executable);
+                    let command = command
+                        .env(FORCE_CLI_MODE_ENV_VAR_NAME, "")
+                        .stderr(subprocess_stdout_file)
+                        .stdout(subprocess_stdin_file)
+                        .arg(url);
+
+                    command
+                        .spawn()
+                        .with_context(|| format!("Spawning {command:?}"))?;
+                }
+            }
+
+            let (_, handshake) = server.accept().context("Handshake after Zed spawn")?;
+            Ok((handshake.requests, handshake.responses))
+        }
+
+        pub fn zed_version_string(&self) -> String {
+            let is_dev = matches!(self, Self::LocalPath { .. });
+            format!(
+                "Zed {}{} – {}",
+                self.plist().bundle_short_version_string,
+                if is_dev { " (dev)" } else { "" },
+                self.path().display(),
+            )
         }
     }
-    Ok(app_path)
 }

crates/gpui/src/platform/linux/blade_belt.rs 🔗

@@ -52,7 +52,7 @@ impl BladeBelt {
         let index_maybe = self
             .buffers
             .iter()
-            .position(|&(ref rb, ref sp)| size <= rb.size && gpu.wait_for(sp, 0));
+            .position(|(rb, sp)| size <= rb.size && gpu.wait_for(sp, 0));
         if let Some(index) = index_maybe {
             let (rb, _) = self.buffers.remove(index);
             let piece = rb.raw.into();
@@ -85,7 +85,7 @@ impl BladeBelt {
             "Type alignment {} is too big",
             type_alignment
         );
-        let total_bytes = data.len() * mem::size_of::<T>();
+        let total_bytes = std::mem::size_of_val(data);
         let bp = self.alloc(total_bytes as u64, gpu);
         unsafe {
             std::ptr::copy_nonoverlapping(data.as_ptr() as *const u8, bp.data(), total_bytes);

crates/gpui/src/platform/linux/blade_renderer.rs 🔗

@@ -455,7 +455,7 @@ impl BladeRenderer {
                         sprites,
                     } => {
                         let tex_info = self.atlas.get_texture_info(texture_id);
-                        let instance_buf = self.instance_belt.alloc_data(&sprites, &self.gpu);
+                        let instance_buf = self.instance_belt.alloc_data(sprites, &self.gpu);
                         let mut encoder = pass.with(&self.pipelines.mono_sprites);
                         encoder.bind(
                             0,
@@ -473,7 +473,7 @@ impl BladeRenderer {
                         sprites,
                     } => {
                         let tex_info = self.atlas.get_texture_info(texture_id);
-                        let instance_buf = self.instance_belt.alloc_data(&sprites, &self.gpu);
+                        let instance_buf = self.instance_belt.alloc_data(sprites, &self.gpu);
                         let mut encoder = pass.with(&self.pipelines.poly_sprites);
                         encoder.bind(
                             0,

crates/gpui/src/platform/linux/dispatcher.rs 🔗

@@ -109,7 +109,7 @@ impl PlatformDispatcher for LinuxDispatcher {
         let moment = Instant::now() + duration;
         let mut timed_tasks = self.timed_tasks.lock();
         timed_tasks.push((moment, runnable));
-        timed_tasks.sort_unstable_by(|&(ref a, _), &(ref b, _)| b.cmp(a));
+        timed_tasks.sort_unstable_by(|(a, _), (b, _)| b.cmp(a));
     }
 
     fn tick(&self, background_only: bool) -> bool {

crates/gpui/src/platform/linux/platform.rs 🔗

@@ -59,7 +59,7 @@ pub(crate) struct LinuxPlatform {
 
 pub(crate) struct LinuxPlatformState {
     quit_requested: bool,
-    windows: HashMap<x::Window, Arc<LinuxWindowState>>,
+    windows: HashMap<x::Window, Rc<LinuxWindowState>>,
 }
 
 impl Default for LinuxPlatform {
@@ -133,7 +133,7 @@ impl Platform for LinuxPlatform {
                 xcb::Event::X(x::Event::Expose(ev)) => {
                     let window = {
                         let state = self.state.lock();
-                        Arc::clone(&state.windows[&ev.window()])
+                        Rc::clone(&state.windows[&ev.window()])
                     };
                     window.expose();
                 }
@@ -150,7 +150,7 @@ impl Platform for LinuxPlatform {
                     };
                     let window = {
                         let state = self.state.lock();
-                        Arc::clone(&state.windows[&ev.window()])
+                        Rc::clone(&state.windows[&ev.window()])
                     };
                     window.configure(bounds)
                 }
@@ -217,7 +217,7 @@ impl Platform for LinuxPlatform {
     ) -> Box<dyn PlatformWindow> {
         let x_window = self.xcb_connection.generate_id();
 
-        let window_ptr = Arc::new(LinuxWindowState::new(
+        let window_ptr = Rc::new(LinuxWindowState::new(
             options,
             &self.xcb_connection,
             self.x_root_index,
@@ -228,7 +228,7 @@ impl Platform for LinuxPlatform {
         self.state
             .lock()
             .windows
-            .insert(x_window, Arc::clone(&window_ptr));
+            .insert(x_window, Rc::clone(&window_ptr));
         Box::new(LinuxWindow(window_ptr))
     }
 

crates/gpui/src/platform/linux/window.rs 🔗

@@ -77,7 +77,7 @@ pub(crate) struct LinuxWindowState {
 }
 
 #[derive(Clone)]
-pub(crate) struct LinuxWindow(pub(crate) Arc<LinuxWindowState>);
+pub(crate) struct LinuxWindow(pub(crate) Rc<LinuxWindowState>);
 
 //todo!(linux): Remove other RawWindowHandle implementation
 unsafe impl blade_rwh::HasRawWindowHandle for RawWindow {
@@ -191,7 +191,7 @@ impl LinuxWindowState {
 
         //Warning: it looks like this reported size is immediately invalidated
         // on some platforms, followed by a "ConfigureNotify" event.
-        let gpu_extent = query_render_extent(&xcb_connection, x_window);
+        let gpu_extent = query_render_extent(xcb_connection, x_window);
 
         let raw = RawWindow {
             connection: as_raw_xcb_connection::AsRawXcbConnection::as_raw_xcb_connection(
@@ -430,6 +430,6 @@ impl PlatformWindow for LinuxWindow {
     }
 
     fn set_graphics_profiler_enabled(&self, enabled: bool) {
-        todo!("linux")
+        unimplemented!("linux")
     }
 }

crates/zed/src/main.rs 🔗

@@ -42,7 +42,6 @@ use std::{
         Arc,
     },
     thread,
-    time::Duration,
 };
 use theme::{ActiveTheme, SystemAppearance, ThemeRegistry, ThemeSettings};
 use util::{
@@ -930,6 +929,7 @@ fn load_user_themes_in_background(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
 /// Spawns a background task to watch the themes directory for changes.
 #[cfg(target_os = "macos")]
 fn watch_themes(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
+    use std::time::Duration;
     cx.spawn(|cx| async move {
         let mut events = fs
             .watch(&paths::THEMES_DIR.clone(), Duration::from_millis(100))
@@ -962,6 +962,8 @@ fn watch_themes(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
 
 #[cfg(debug_assertions)]
 async fn watch_languages(fs: Arc<dyn fs::Fs>, languages: Arc<LanguageRegistry>) {
+    use std::time::Duration;
+
     let reload_debounce = Duration::from_millis(250);
 
     let mut events = fs
@@ -975,6 +977,8 @@ async fn watch_languages(fs: Arc<dyn fs::Fs>, languages: Arc<LanguageRegistry>)
 
 #[cfg(debug_assertions)]
 fn watch_file_types(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
+    use std::time::Duration;
+
     cx.spawn(|cx| async move {
         let mut events = fs
             .watch(

script/clippy 🔗

@@ -1,4 +1,6 @@
-#!/bin/bash
+#!/usr/bin/env bash
+
+set -euxo pipefail
 
 # clippy.toml is not currently supporting specifying allowed lints
 # so specify those here, and disable the rest until Zed's workspace