From 5175c8963a23a3cab26ffb032feb34e4dc57b55b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 9 Feb 2024 20:13:00 +0200 Subject: [PATCH] Actually fail on clippy failures (#7619) 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 --- crates/cli/src/main.rs | 326 ++++++++++-------- crates/gpui/src/platform/linux/blade_belt.rs | 4 +- .../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, 208 insertions(+), 154 deletions(-) diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 69cfb7102b..47a824a04b 100644 --- a/crates/cli/src/main.rs +++ b/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,136 +106,6 @@ enum Bundle { }, } -impl Bundle { - fn detect(args_bundle_path: Option<&Path>) -> anyhow::Result { - 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 plist(&self) -> &InfoPlist { - match self { - Self::App { plist, .. } => plist, - Self::LocalPath { plist, .. } => plist, - } - } - - fn path(&self) -> &Path { - match self { - Self::App { app_bundle, .. } => app_bundle, - Self::LocalPath { executable, .. } => executable, - } - } - - fn launch(&self) -> anyhow::Result<(IpcSender, IpcReceiver)> { - let (server, server_name) = - IpcOneShotServer::::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(), - ) - }; - - 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)) - } - - 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(), - ) - } -} - fn touch(path: &Path) -> io::Result<()> { match OpenOptions::new().create(true).write(true).open(path) { Ok(_) => Ok(()), @@ -259,3 +123,187 @@ fn locate_bundle() -> Result { } 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 { + unimplemented!() + } + + pub fn plist(&self) -> &InfoPlist { + unimplemented!() + } + + pub fn path(&self) -> &Path { + unimplemented!() + } + + pub fn launch(&self) -> anyhow::Result<(IpcSender, IpcReceiver)> { + unimplemented!() + } + + pub fn zed_version_string(&self) -> String { + unimplemented!() + } + } +} + +#[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 { + 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 plist(&self) -> &InfoPlist { + match self { + Self::App { plist, .. } => plist, + Self::LocalPath { plist, .. } => plist, + } + } + + fn path(&self) -> &Path { + match self { + Self::App { app_bundle, .. } => app_bundle, + Self::LocalPath { executable, .. } => executable, + } + } + + pub fn launch(&self) -> anyhow::Result<(IpcSender, IpcReceiver)> { + let (server, server_name) = + IpcOneShotServer::::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(), + ) + }; + + 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(), + ) + } + } +} diff --git a/crates/gpui/src/platform/linux/blade_belt.rs b/crates/gpui/src/platform/linux/blade_belt.rs index 2562097cbb..7145ce2d2c 100644 --- a/crates/gpui/src/platform/linux/blade_belt.rs +++ b/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::(); + 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); diff --git a/crates/gpui/src/platform/linux/blade_renderer.rs b/crates/gpui/src/platform/linux/blade_renderer.rs index 665cedb88f..9f2ae6b606 100644 --- a/crates/gpui/src/platform/linux/blade_renderer.rs +++ b/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, diff --git a/crates/gpui/src/platform/linux/dispatcher.rs b/crates/gpui/src/platform/linux/dispatcher.rs index 8aa0631268..fad7d01c87 100644 --- a/crates/gpui/src/platform/linux/dispatcher.rs +++ b/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 { diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index b5ab1fcf93..dfc8f1163f 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/crates/gpui/src/platform/linux/platform.rs @@ -59,7 +59,7 @@ pub(crate) struct LinuxPlatform { pub(crate) struct LinuxPlatformState { quit_requested: bool, - windows: HashMap>, + windows: HashMap>, } 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 { 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)) } diff --git a/crates/gpui/src/platform/linux/window.rs b/crates/gpui/src/platform/linux/window.rs index e5c346b057..29904d8dfb 100644 --- a/crates/gpui/src/platform/linux/window.rs +++ b/crates/gpui/src/platform/linux/window.rs @@ -77,7 +77,7 @@ pub(crate) struct LinuxWindowState { } #[derive(Clone)] -pub(crate) struct LinuxWindow(pub(crate) Arc); +pub(crate) struct LinuxWindow(pub(crate) Rc); //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") } } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 9311c934db..832712762e 100644 --- a/crates/zed/src/main.rs +++ b/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, cx: &mut AppContext) { /// Spawns a background task to watch the themes directory for changes. #[cfg(target_os = "macos")] fn watch_themes(fs: Arc, 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, cx: &mut AppContext) { #[cfg(debug_assertions)] async fn watch_languages(fs: Arc, languages: Arc) { + 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, languages: Arc) #[cfg(debug_assertions)] fn watch_file_types(fs: Arc, cx: &mut AppContext) { + use std::time::Duration; + cx.spawn(|cx| async move { let mut events = fs .watch( diff --git a/script/clippy b/script/clippy index 1a817015f3..71dca2ffef 100755 --- a/script/clippy +++ b/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