diff --git a/.agents/skills/lint-creator/SKILL.md b/.agents/skills/lint-creator/SKILL.md new file mode 100644 index 00000000000..e72e83a9aba --- /dev/null +++ b/.agents/skills/lint-creator/SKILL.md @@ -0,0 +1,17 @@ +--- +name: lint-creator +description: An auxiliary skill to add more dylints to `tooling/lints` +disable-model-invocation: false +--- + +# Lint RULES + +1. Every lint MUST have accompanying `ui` tests +2. `ui` tests MUST be in the `ui` folder +3. Every lint MUST be in a separate module +4. Every lint MUST have negative `ui` tests +5. Lints should be as simple as possible. +6. Reporting is fine if it's simple, it does not need to be elaborate or lengthy code. +7. Do NOT suggest how to fix the lint, only flag it. +8. Do NOT make lints machine applicable. +9. Detect if lints are redundant vs clippy's capabilities. diff --git a/Cargo.toml b/Cargo.toml index 5e7703aaaee..2adf179e513 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1070,3 +1070,10 @@ ignored = [ "documented", "sea-orm-macros", ] + +# Dylint discovers our custom lints through this entry, so `cargo dylint --all` +# runs them without a `--path` argument. The `lints` package pins its own +# nightly toolchain (see `tooling/lints/rust-toolchain.toml`) and is kept out of +# this workspace on purpose. +[workspace.metadata.dylint] +libraries = [{ path = "tooling/lints" }] diff --git a/tooling/lints/.cargo/config.toml b/tooling/lints/.cargo/config.toml new file mode 100644 index 00000000000..93dceb69922 --- /dev/null +++ b/tooling/lints/.cargo/config.toml @@ -0,0 +1,2 @@ +[target.'cfg(all())'] +linker = "dylint-link" diff --git a/tooling/lints/.gitignore b/tooling/lints/.gitignore new file mode 100644 index 00000000000..26b452057c5 --- /dev/null +++ b/tooling/lints/.gitignore @@ -0,0 +1,4 @@ +/target/ +/test_fixture/target/ +Cargo.lock +test_fixture/Cargo.lock diff --git a/tooling/lints/Cargo.toml b/tooling/lints/Cargo.toml new file mode 100644 index 00000000000..d70050bfa61 --- /dev/null +++ b/tooling/lints/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "lints" +version = "0.1.0" +edition = "2024" +publish = false +description = "Dylint lints for catching bad Zed specific patterns." + +[lib] +crate-type = ["cdylib"] + +[dependencies] +clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "86390a3c03438b660c5efc64d4e18ae65982f5c0" } +dylint_linting = "6.0" + +[dev-dependencies] +# `deny_warnings` restores `-D warnings` for the UI fixtures (off by default +# since dylint 3.0), so toolchain drift surfaces as a failing test. +dylint_testing = { version = "6.0", features = ["deny_warnings"] } + +[package.metadata.rust-analyzer] +rustc_private = true + +# Keep this crate out of the zed workspace. It pins its own nightly toolchain +# (see `rust-toolchain.toml`) to match `clippy_utils`. +[workspace] + +[lints.rust.unexpected_cfgs] +level = "warn" +check-cfg = ["cfg(dylint_lib, values(any()))"] diff --git a/tooling/lints/README.md b/tooling/lints/README.md new file mode 100644 index 00000000000..6804a2e9093 --- /dev/null +++ b/tooling/lints/README.md @@ -0,0 +1,86 @@ +# lints + +A [dylint](https://github.com/trailofbits/dylint) library that flags various bad patterns in our codebase. + +Install `dylint`, a pinned nightly toolchain and the necessary tools with + +``` +cargo install cargo-dylint dylint-link +cd tooling/lints +rustup toolchain install +``` + +The channel and its components (`rustc-dev`, `rust-src`, `llvm-tools-preview`) +are declared in `tooling/lints/rust-toolchain.toml`, so `rustup toolchain install` +picks them up automatically when run from that directory. + +# Demo + +``` +./single-lint blocking_io_on_foreground +``` + + +## Current lints +- `shared_string_from_str_literal` — `SharedString::new/from` etc where `SharedString::from_static` should be used instead. +- `async_block_without_await` — `async { … }` blocks whose body contains no `.await` expression. +- `entity_update_in_render` — `Entity::update`/`WeakEntity::update` mutating an entity inside `Render::render`. +- `notify_in_render` — `Context::notify()` called inside `Render::render`. +- `owned_string_into_shared` — `String::from().into()` / `.to_string().into()` / `.to_owned().into()` whose target is `SharedString`, `Arc`, `Rc`, or `Cow<'_, str>`. +- `blocking_io_on_foreground` - Catch blocking IO calls that are called on the main thread (but not on closures or background threads) + +## How to run + +Ideally you run this as part of the `clippy` script in the `zed/scripts` directory since this will also run our other linters. + +### Prerequisites + +Install both tools (version 6 or later): + +``` +cargo install cargo-dylint dylint-link +``` + +- `cargo-dylint` is the `cargo` subcommand that builds and runs the lints; `dylint-link` is the linker used to build the lint library. + +The workspace registers this library under `[workspace.metadata.dylint]` in the +root `Cargo.toml`, so Dylint discovers it automatically — you do not pass a +`--path`. The first run builds the library against its pinned nightly (see +`rust-toolchain.toml`) and is slow; later runs are cached. + +### Run all lints against the whole repo + +``` +cargo dylint --all -- --workspace +``` + +### Run all lints against a single crate + +``` +cargo dylint --all -- -p project_panel +``` + +### Run a single lint + +The library loads every lint at once. To run just one, use the `single-lint` +helper, which silences the rest and force-enables the one you name: + +``` +tooling/lints/single-lint blocking_io_on_foreground -p project_panel +``` + +The first argument is the lint name (one of the snake_case identifiers under +[Current lints](#current-lints)); everything after it is passed to `cargo check` +and defaults to `--workspace`. Under the hood the script runs: + +``` +DYLINT_RUSTFLAGS="-A warnings --force-warn " cargo dylint --all -- +``` + +It also handles two non-obvious gotchas: + +- `--force-warn` is required: after `-A warnings` silences the group, a plain + `-W ` does not reliably re-enable a driver-registered lint. +- `DYLINT_RUSTFLAGS` is not part of Cargo's fingerprint, so the script cleans the + targeted package(s) first; otherwise Cargo replays a stale cache and the filter + appears to do nothing. diff --git a/tooling/lints/rust-toolchain.toml b/tooling/lints/rust-toolchain.toml new file mode 100644 index 00000000000..0ba86155420 --- /dev/null +++ b/tooling/lints/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "nightly-2026-03-21" +components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/tooling/lints/single-lint b/tooling/lints/single-lint new file mode 100755 index 00000000000..e481cf91bc6 --- /dev/null +++ b/tooling/lints/single-lint @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# +# Run a single lint from this dylint library against the Zed workspace. +# +# Usage: tooling/lints/single-lint [cargo check args...] +# Example: tooling/lints/single-lint blocking_io_on_foreground -p project_panel +# +# Dylint loads the whole library, so we silence every lint with `-A warnings` +# and force just the requested one back on with `--force-warn`. A plain +# `-W ` is dropped for a driver-registered lint once the group is allowed, +# which is why `--force-warn` is required. +# +# `DYLINT_RUSTFLAGS` is not part of Cargo's fingerprint, so changing it does not +# invalidate already-checked crates. We therefore clean the targeted package(s) +# first so the filter actually applies instead of replaying a stale cache. When +# no package is named (a `--workspace` run) we drop the whole check cache. +set -euo pipefail + +if [ "$#" -lt 1 ]; then + echo "usage: $(basename "$0") [cargo check args...]" >&2 + exit 1 +fi + +lint="$1" +shift +if [ "$#" -eq 0 ]; then + set -- --workspace +fi + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +repo_root="$(cd "$script_dir/../.." && pwd)" + +toolchain="$(awk -F'"' '/^channel/ {print $2}' "$script_dir/rust-toolchain.toml")" +host="$(rustc -vV | awk '/^host:/ {print $2}')" +check_target="$repo_root/target/dylint/target/${toolchain}-${host}" + +# Force a re-check of the requested package(s) so the lint filter takes effect. +cleaned_any=0 +prev="" +for arg in "$@"; do + pkg="" + case "$arg" in + -p=* | --package=*) + pkg="${arg#*=}" + ;; + *) + if [ "$prev" = "-p" ] || [ "$prev" = "--package" ]; then + pkg="$arg" + fi + ;; + esac + if [ -n "$pkg" ]; then + cargo clean -p "$pkg" --target-dir "$check_target" 2>/dev/null || true + cleaned_any=1 + fi + prev="$arg" +done + +if [ "$cleaned_any" -eq 0 ]; then + rm -rf "$check_target" +fi + +cd "$repo_root" +DYLINT_RUSTFLAGS="-A warnings --force-warn $lint" exec cargo dylint --all -- "$@" diff --git a/tooling/lints/src/blocking_io_on_foreground.rs b/tooling/lints/src/blocking_io_on_foreground.rs new file mode 100644 index 00000000000..d8d7fe16e91 --- /dev/null +++ b/tooling/lints/src/blocking_io_on_foreground.rs @@ -0,0 +1,246 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_hir::def::Res; +use rustc_hir::{Expr, ExprKind, HirId, Node}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Ty; + +use crate::render_helpers::is_directly_in_render_method; + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags calls to known blocking IO functions from the standard library + /// (`std::fs`, `std::thread::sleep`, `std::process::Command`, `std::net`) + /// when they appear inside a function that receives a synchronous GPUI + /// context parameter (`&App`, `&mut App`, `&Context`, + /// `&mut Context`, `&mut Window`) or directly inside a + /// `Render::render` / `RenderOnce::render` method. + /// + /// ### Why is this bad? + /// + /// In GPUI, code that receives a synchronous context type runs on the + /// foreground (UI) thread. A blocking IO call on this thread freezes the + /// application until the syscall returns. + pub BLOCKING_IO_ON_FOREGROUND, + Warn, + "blocking IO call on the GPUI foreground thread" +} + +pub(crate) struct BlockingIoOnForeground; + +rustc_session::impl_lint_pass!(BlockingIoOnForeground => [BLOCKING_IO_ON_FOREGROUND]); + +const BLOCKING_FN_PATHS: &[&str] = &[ + // std::fs free functions + "std::fs::read", + "std::fs::read_to_string", + "std::fs::write", + "std::fs::read_dir", + "std::fs::read_link", + "std::fs::metadata", + "std::fs::symlink_metadata", + "std::fs::set_permissions", + "std::fs::canonicalize", + "std::fs::create_dir", + "std::fs::create_dir_all", + "std::fs::remove_file", + "std::fs::remove_dir", + "std::fs::remove_dir_all", + "std::fs::copy", + "std::fs::rename", + "std::fs::hard_link", + // std::fs::File associated functions + "std::fs::File::open", + "std::fs::File::create", + "std::fs::File::create_new", + // std::thread + "std::thread::sleep", + // std::path::Path methods (resolved via method call def_id) + "std::path::Path::metadata", + "std::path::Path::symlink_metadata", + "std::path::Path::read_link", + "std::path::Path::read_dir", + "std::path::Path::exists", + "std::path::Path::try_exists", + "std::path::Path::is_file", + "std::path::Path::is_dir", + "std::path::Path::is_symlink", + "std::path::Path::canonicalize", + // std::net associated functions + "std::net::TcpStream::connect", + "std::net::TcpStream::connect_timeout", + "std::net::TcpListener::bind", + "std::net::UdpSocket::bind", +]; + +const BLOCKING_METHODS: &[(&str, &str)] = &[ + // std::process + ("Command", "output"), + ("Command", "status"), + ("Command", "spawn"), + ("Child", "wait"), + ("Child", "wait_with_output"), + // std::fs::File instance methods + ("File", "sync_all"), + ("File", "sync_data"), + ("File", "set_len"), + ("File", "metadata"), + ("File", "try_clone"), + ("File", "set_permissions"), + // std::net — TCP + ("TcpStream", "connect"), + ("TcpStream", "peek"), + ("TcpListener", "bind"), + ("TcpListener", "accept"), + ("TcpListener", "incoming"), + // std::net — UDP + ("UdpSocket", "send"), + ("UdpSocket", "send_to"), + ("UdpSocket", "recv"), + ("UdpSocket", "recv_from"), + ("UdpSocket", "peek"), + ("UdpSocket", "peek_from"), + // std::sync + ("Mutex", "lock"), + ("RwLock", "read"), + ("RwLock", "write"), + ("Condvar", "wait"), + ("Condvar", "wait_timeout"), + ("Condvar", "wait_while"), + ("Barrier", "wait"), + // std::sync::mpsc + ("Receiver", "recv"), + ("Receiver", "recv_timeout"), + ("SyncSender", "send"), +]; + +fn is_blocking_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match &expr.kind { + ExprKind::Call(callee, _) => { + if let ExprKind::Path(qpath) = &callee.kind { + if let Res::Def(_, def_id) = cx.qpath_res(qpath, callee.hir_id) { + let path = cx.tcx.def_path_str(def_id); + return BLOCKING_FN_PATHS.iter().any(|blocked| path == *blocked); + } + } + false + } + ExprKind::MethodCall(segment, receiver, _args, _span) => { + if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { + let path = cx.tcx.def_path_str(def_id); + if BLOCKING_FN_PATHS.iter().any(|blocked| path == *blocked) { + return true; + } + } + let method_name = segment.ident.name.as_str(); + if !BLOCKING_METHODS + .iter() + .any(|(_, name)| *name == method_name) + { + return false; + } + let receiver_ty = cx.typeck_results().expr_ty(receiver).peel_refs(); + if let Some(adt) = receiver_ty.ty_adt_def() { + let type_name = cx.tcx.item_name(adt.did()); + return BLOCKING_METHODS + .iter() + .any(|(ty, name)| *name == method_name && type_name.as_str() == *ty); + } + false + } + _ => false, + } +} + +/// Returns `true` if `ty` (after peeling references) is a synchronous GPUI +/// foreground type: `App`, `Context`, or `Window`. +fn is_gpui_foreground_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + let peeled = ty.peel_refs(); + let Some(adt) = peeled.ty_adt_def() else { + return false; + }; + let did = adt.did(); + let crate_name = cx.tcx.crate_name(did.krate); + if crate_name.as_str() != "gpui" { + return false; + } + let name = cx.tcx.item_name(did); + matches!(name.as_str(), "App" | "Context" | "Window") +} + +/// Walks up the HIR parent chain from `hir_id` to find the enclosing +/// function. Returns `true` if that function has a parameter whose type is a +/// synchronous GPUI context type. Returns `false` if a closure boundary is +/// crossed first (the closure might run on a background thread). +fn is_in_foreground_fn(cx: &LateContext<'_>, hir_id: HirId) -> bool { + for (_parent_id, node) in cx.tcx.hir_parent_iter(hir_id) { + match node { + Node::Expr(expr) if matches!(expr.kind, ExprKind::Closure(_)) => { + return false; + } + Node::Item(item) => { + if let rustc_hir::ItemKind::Fn { .. } = &item.kind { + let owner_id = item.owner_id.def_id; + return owner_has_foreground_param(cx, owner_id); + } + return false; + } + Node::ImplItem(impl_item) => { + if let rustc_hir::ImplItemKind::Fn(_, _) = &impl_item.kind { + let owner_id = impl_item.owner_id.def_id; + return owner_has_foreground_param(cx, owner_id); + } + return false; + } + Node::TraitItem(trait_item) => { + if let rustc_hir::TraitItemKind::Fn(_, _) = &trait_item.kind { + let owner_id = trait_item.owner_id.def_id; + return owner_has_foreground_param(cx, owner_id); + } + return false; + } + _ => {} + } + } + false +} + +/// Checks whether the function identified by `local_def_id` has any parameter +/// whose type is a synchronous GPUI foreground type. +fn owner_has_foreground_param( + cx: &LateContext<'_>, + local_def_id: rustc_hir::def_id::LocalDefId, +) -> bool { + let def_id = local_def_id.to_def_id(); + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + sig.inputs() + .skip_binder() + .iter() + .any(|ty| is_gpui_foreground_type(cx, *ty)) +} + +impl<'tcx> LateLintPass<'tcx> for BlockingIoOnForeground { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if expr.span.from_expansion() { + return; + } + + if !is_blocking_call(cx, expr) { + return; + } + + let in_render = is_directly_in_render_method(cx, expr.hir_id); + let in_foreground_fn = is_in_foreground_fn(cx, expr.hir_id); + + if !in_render && !in_foreground_fn { + return; + } + + span_lint( + cx, + BLOCKING_IO_ON_FOREGROUND, + expr.span, + "blocking IO call on the GPUI foreground thread", + ); + } +} diff --git a/tooling/lints/src/entity_update_in_render.rs b/tooling/lints/src/entity_update_in_render.rs new file mode 100644 index 00000000000..fc17f3de2be --- /dev/null +++ b/tooling/lints/src/entity_update_in_render.rs @@ -0,0 +1,66 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; + +use crate::render_helpers::{ + is_directly_in_render_method, is_gpui_entity_or_weak, is_unit_or_result_unit, +}; + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags calls to `Entity::update` or `WeakEntity::update` that execute + /// synchronously inside a `Render::render` or `RenderOnce::render` method + /// and whose closure returns `()` (indicating mutation rather than reading). + /// + /// ### Why is this bad? + /// + /// The `render` method should be a pure function of state. Calling + /// `.update()` mutates an entity during the render pass, which can trigger + /// re-renders mid-render and lead to inconsistent UI state or infinite + /// render loops. + pub ENTITY_UPDATE_IN_RENDER, + Warn, + "mutating an entity via `.update()` during render" +} + +pub(crate) struct EntityUpdateInRender; + +rustc_session::impl_lint_pass!(EntityUpdateInRender => [ENTITY_UPDATE_IN_RENDER]); + +impl<'tcx> LateLintPass<'tcx> for EntityUpdateInRender { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if expr.span.from_expansion() { + return; + } + + let ExprKind::MethodCall(segment, receiver, _args, _span) = &expr.kind else { + return; + }; + + if segment.ident.name.as_str() != "update" { + return; + } + + let receiver_ty = cx.typeck_results().expr_ty(receiver); + if !is_gpui_entity_or_weak(cx, receiver_ty) { + return; + } + + let call_ty = cx.typeck_results().expr_ty(expr); + if !is_unit_or_result_unit(cx, call_ty) { + return; + } + + if !is_directly_in_render_method(cx, expr.hir_id) { + return; + } + + span_lint( + cx, + ENTITY_UPDATE_IN_RENDER, + expr.span, + "entity `.update()` called during render mutates state in the render pass", + ); + } +} diff --git a/tooling/lints/src/lib.rs b/tooling/lints/src/lib.rs new file mode 100644 index 00000000000..bf4289673dd --- /dev/null +++ b/tooling/lints/src/lib.rs @@ -0,0 +1,565 @@ +#![feature(rustc_private)] +#![warn(unused_extern_crates)] + +extern crate rustc_ast; +extern crate rustc_errors; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_session; +extern crate rustc_span; + +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; +use clippy_utils::is_def_id_trait_method; +use clippy_utils::source::snippet_opt; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::{Visitor, walk_expr}; +use rustc_hir::{ + Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, + YieldSource, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty::Ty; +use rustc_span::Span; + +mod blocking_io_on_foreground; +mod entity_update_in_render; +mod notify_in_render; +mod owned_string_into_shared; +mod render_helpers; + +use blocking_io_on_foreground::BLOCKING_IO_ON_FOREGROUND; +use entity_update_in_render::ENTITY_UPDATE_IN_RENDER; +use notify_in_render::NOTIFY_IN_RENDER; +use owned_string_into_shared::OWNED_STRING_INTO_SHARED; + +// --------------------------------------------------------------------------- +// Boilerplate: export the dylint ABI version symbol. +// --------------------------------------------------------------------------- +dylint_linting::dylint_library!(); + +// --------------------------------------------------------------------------- +// Registration: a single entry point that hands both lints to the compiler. +// --------------------------------------------------------------------------- +#[allow(clippy::no_mangle_with_rust_abi)] +#[unsafe(no_mangle)] +pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { + dylint_linting::init_config(sess); + lint_store.register_lints(&[ + SHARED_STRING_FROM_STR_LITERAL, + ASYNC_BLOCK_WITHOUT_AWAIT, + BLOCKING_IO_ON_FOREGROUND, + ENTITY_UPDATE_IN_RENDER, + NOTIFY_IN_RENDER, + OWNED_STRING_INTO_SHARED, + ]); + lint_store.register_late_pass(|_| Box::new(SharedStringFromStrLiteral)); + lint_store.register_late_pass(|_| Box::new(AsyncBlockWithoutAwait)); + lint_store.register_late_pass(|_| Box::new(blocking_io_on_foreground::BlockingIoOnForeground)); + lint_store.register_late_pass(|_| Box::new(entity_update_in_render::EntityUpdateInRender)); + lint_store.register_late_pass(|_| Box::new(notify_in_render::NotifyInRender)); + lint_store.register_late_pass(|_| Box::new(owned_string_into_shared::OwnedStringIntoShared)); +} + +// =========================================================================== +// Lint A — SHARED_STRING_FROM_STR_LITERAL +// =========================================================================== + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags `gpui::SharedString` values constructed from a string literal by + /// any path other than `SharedString::new_static`. + /// + /// ### Why is this bad? + /// + /// `SharedString` wraps a `SmolStr`. `SmolStr::from` either copies the + /// bytes into inline storage (literals ≤ 23 bytes) or allocates a fresh + /// `Arc` on the heap (literals > 23 bytes). `SharedString::new_static` + /// does neither: it stores the `'static` pointer directly. For a string + /// literal the constant-pointer path is always available and strictly + /// cheaper. + /// + /// This lint fires on `SharedString::from("…")`, `SharedString::new("…")`, + /// `>::from("…")`, and `"…".into()` whose inferred + /// target type is `SharedString`. It does not fire on + /// `SharedString::new_static(…)`. + /// + /// The lint distinguishes two tiers of wastefulness: + /// * Literals > 23 bytes trigger a heap allocation per call site, flagged + /// at full severity. + /// * Literals ≤ 23 bytes "only" pay a memcpy; still strictly worse than + /// `new_static`, but cheaper to leave alone. + /// + /// ### Example + /// + /// ```ignore + /// let s: SharedString = SharedString::from("Right-click for more options"); + /// let t: SharedString = "hello".into(); + /// ``` + /// + /// Use instead: + /// + /// ```ignore + /// let s = SharedString::new_static("Right-click for more options"); + /// let t = SharedString::new_static("hello"); + /// ``` + pub SHARED_STRING_FROM_STR_LITERAL, + Warn, + "constructing a `SharedString` from a string literal via a copying/allocating path" +} + +rustc_session::declare_lint_pass!(SharedStringFromStrLiteral => [SHARED_STRING_FROM_STR_LITERAL]); + +/// Maximum number of bytes that `SmolStr` (and therefore `SharedString`) can +/// store inline on 64-bit targets. Literals larger than this trigger an +/// `Arc` allocation on every conversion. +/// +/// Source: `smol_str` v0.3's `INLINE_CAP`. See +/// . +const SMOL_STR_INLINE_CAP: usize = 23; + +impl<'tcx> LateLintPass<'tcx> for SharedStringFromStrLiteral { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + // Do not descend into macro-expanded code: we'd be suggesting edits to + // spans the user cannot actually touch. + if expr.span.from_expansion() { + return; + } + + let ty = cx.typeck_results().expr_ty(expr); + if !is_shared_string(cx, ty) { + return; + } + + let Some(literal) = extract_literal_source(cx, expr) else { + return; + }; + + emit_shared_string(cx, expr.span, literal); + } +} + +/// A string literal the user wrote and that we are confident we can replace. +struct LiteralSource { + /// The literal's decoded contents. + contents: String, + /// The span of the full expression that should be replaced (e.g. the whole + /// `SharedString::from("x")` call), not just the literal token. + replace_span: Span, +} + +fn extract_literal_source<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> Option { + match &expr.kind { + // `SharedString::from(lit)`, `SharedString::new(lit)`, or any other + // associated/trait function resolving onto `SharedString` with a + // single string-literal argument. + ExprKind::Call(func, [arg]) => { + let def_id = call_def_id(cx, func)?; + if !is_interesting_shared_string_constructor(cx, def_id) { + return None; + } + let contents = str_literal_contents(arg)?; + Some(LiteralSource { + contents, + replace_span: expr.span, + }) + } + + // `lit.into()` where the target type is `SharedString`. + ExprKind::MethodCall(path_seg, receiver, [], _) + if path_seg.ident.name.as_str() == "into" => + { + let contents = str_literal_contents(receiver)?; + Some(LiteralSource { + contents, + replace_span: expr.span, + }) + } + + _ => None, + } +} + +/// Extract the contents of a string literal expression, peeling through a +/// single layer of reference if the user wrote `&"lit"`. +fn str_literal_contents<'tcx>(expr: &'tcx Expr<'tcx>) -> Option { + let inner = match &expr.kind { + ExprKind::AddrOf(_, _, inner) => *inner, + _ => expr, + }; + if let ExprKind::Lit(lit) = &inner.kind + && let LitKind::Str(sym, _) = lit.node + { + Some(sym.as_str().to_owned()) + } else { + None + } +} + +/// Returns the `DefId` of the function being called, if `func` is a direct +/// path to a function or associated function. This handles both +/// `Type::method(...)` syntax (including type-relative paths that resolve +/// through `typeck_results`) and free-function paths. +fn call_def_id<'tcx>(cx: &LateContext<'tcx>, func: &'tcx Expr<'tcx>) -> Option { + let ExprKind::Path(qpath) = &func.kind else { + return None; + }; + match cx.qpath_res(qpath, func.hir_id) { + Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) => Some(def_id), + _ => None, + } +} + +/// True if `def_id` names a `SharedString` constructor that we treat as a +/// wasteful alternative to `SharedString::new_static` when passed a string +/// literal. +/// +/// This covers two distinct resolutions rustc produces for these call sites: +/// +/// * `SharedString::new("x")` resolves to the inherent associated function +/// on `impl SharedString`. The impl's `Self` type is `SharedString`. +/// * `SharedString::from("x")` resolves to the trait method +/// `core::convert::From::from`. The impl is not recorded on the `def_id` +/// itself; we instead verify the enclosing trait is `From` and rely on the +/// caller having already checked that the call's result type is +/// `SharedString`. +/// +/// `SharedString::new_static` is explicitly exempted because it is the +/// preferred alternative. +fn is_interesting_shared_string_constructor(cx: &LateContext<'_>, def_id: DefId) -> bool { + let tcx = cx.tcx; + let name = tcx.item_name(def_id); + if name.as_str() == "new_static" { + return false; + } + if !matches!(name.as_str(), "from" | "new") { + return false; + } + if let Some(impl_id) = tcx.impl_of_assoc(def_id) { + let self_ty = tcx.type_of(impl_id).skip_binder(); + return is_shared_string(cx, self_ty); + } + if let Some(trait_id) = tcx.trait_of_assoc(def_id) { + // The caller has already asserted that the call's result type is + // `SharedString`, so a `From::from` call resolving here is + // equivalent to `>::from`. + let path = tcx.def_path_str(trait_id); + return path == "core::convert::From" || path == "std::convert::From"; + } + false +} + +/// Match the canonical definition path of `gpui_shared_string::SharedString`. +/// Re-exports through `gpui` resolve back to the same `DefId`. +fn is_shared_string(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + let Some(adt) = ty.ty_adt_def() else { + return false; + }; + let did = adt.did(); + let krate = cx.tcx.crate_name(did.krate); + if krate.as_str() != "gpui_shared_string" { + return false; + } + cx.tcx.item_name(did).as_str() == "SharedString" +} + +fn emit_shared_string(cx: &LateContext<'_>, call_span: Span, literal: LiteralSource) { + let LiteralSource { + contents, + replace_span, + } = literal; + let byte_len = contents.len(); + let over_inline = byte_len > SMOL_STR_INLINE_CAP; + + // Use the original source text for the replacement where possible so we + // preserve raw-string syntax, escapes, etc. Fall back to debug-formatting + // the decoded contents if the source is unavailable (e.g. macro-generated). + let replacement_lit = snippet_opt(cx, replace_span) + .and_then(extract_embedded_string_literal) + .unwrap_or_else(|| format!("{contents:?}")); + + let suggestion = format!("SharedString::new_static({replacement_lit})"); + + let primary_msg = if over_inline { + "this `SharedString` construction heap-allocates on every call" + } else { + "this `SharedString` construction copies the literal on every call" + }; + + span_lint_and_then( + cx, + SHARED_STRING_FROM_STR_LITERAL, + call_span, + primary_msg, + |diag| { + if over_inline { + diag.note(format!( + "the literal is {byte_len} bytes, which exceeds `SmolStr`'s {SMOL_STR_INLINE_CAP}-byte inline capacity, so `SmolStr::from` allocates an `Arc` here", + )); + } else { + diag.note(format!( + "the literal is {byte_len} bytes (≤ {SMOL_STR_INLINE_CAP}) so it stays inline, but the copy is still avoidable", + )); + } + diag.note("`SharedString::new_static` stores the `'static` pointer directly and performs no allocation or copy"); + diag.span_suggestion( + replace_span, + "use the zero-cost static constructor", + suggestion, + Applicability::MachineApplicable, + ); + }, + ); +} + +/// Given a snippet like `SharedString::from("hi")` or `"hi".into()`, extract +/// the first embedded string literal token (including any `r#"..."#` prefix) +/// so we can paste it back unchanged. This is a best-effort scanner and +/// returns `None` when the snippet has no literal or an unterminated one. +fn extract_embedded_string_literal(snippet: String) -> Option { + let bytes = snippet.as_bytes(); + let mut i = 0; + while i < bytes.len() { + // Raw string: optional `b`, then `r`, then `#`*, then `"`. + let raw_start = i; + let mut j = i; + if j < bytes.len() && bytes[j] == b'b' { + j += 1; + } + if j < bytes.len() && bytes[j] == b'r' { + let mut hashes = 0; + let mut k = j + 1; + while k < bytes.len() && bytes[k] == b'#' { + hashes += 1; + k += 1; + } + if k < bytes.len() && bytes[k] == b'"' { + // Scan for closing `"` followed by the same number of `#`. + let mut m = k + 1; + while m < bytes.len() { + if bytes[m] == b'"' { + let mut close_hashes = 0; + let mut n = m + 1; + while close_hashes < hashes && n < bytes.len() && bytes[n] == b'#' { + close_hashes += 1; + n += 1; + } + if close_hashes == hashes { + return Some(snippet[raw_start..n].to_owned()); + } + } + m += 1; + } + return None; + } + } + // Regular string: optional `b`, then `"`, until matching unescaped `"`. + let mut j = i; + if j < bytes.len() && bytes[j] == b'b' { + j += 1; + } + if j < bytes.len() && bytes[j] == b'"' { + let mut m = j + 1; + while m < bytes.len() { + match bytes[m] { + b'\\' => { + m += 2; + continue; + } + b'"' => return Some(snippet[raw_start..=m].to_owned()), + _ => m += 1, + } + } + return None; + } + i += 1; + } + None +} + +// =========================================================================== +// Lint B — ASYNC_BLOCK_WITHOUT_AWAIT +// =========================================================================== + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags `async { … }` and `async move { … }` blocks whose body contains + /// no `.await` expression at their own nesting level. + /// + /// ### Why is this bad? + /// + /// An async block without an `.await` wraps synchronous code in a `Future` + /// state machine for no benefit. The state machine adds binary size, and + /// the indirection may hide the fact that the code never actually yields. + /// Either the `async` should be removed (the code is synchronous), or a + /// missing `.await` is a bug. + /// + /// ### Example + /// + /// ```ignore + /// let future = async { compute_something() }; + /// ``` + /// + /// Use instead: + /// + /// ```ignore + /// let value = compute_something(); + /// ``` + pub ASYNC_BLOCK_WITHOUT_AWAIT, + Warn, + "`async` block that contains no `.await` expression" +} + +rustc_session::declare_lint_pass!(AsyncBlockWithoutAwait => [ASYNC_BLOCK_WITHOUT_AWAIT]); + +impl<'tcx> LateLintPass<'tcx> for AsyncBlockWithoutAwait { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if expr.span.from_expansion() { + return; + } + + // Match only async blocks — not async function bodies or async closures. + let ExprKind::Closure(Closure { + kind: + ClosureKind::Coroutine(CoroutineKind::Desugared( + CoroutineDesugaring::Async, + CoroutineSource::Block, + )), + body, + .. + }) = &expr.kind + else { + return; + }; + + // Trait impls are constrained by the trait's signature. If the trait + // requires a method that returns a future, the implementor must produce + // an async block even when their implementation has nothing to await. + let enclosing_body_owner = cx.tcx.hir_enclosing_body_owner(expr.hir_id); + if is_def_id_trait_method(cx, enclosing_body_owner) { + return; + } + + let body = cx.tcx.hir_body(*body); + let mut visitor = AwaitVisitor { + cx, + found_await: false, + async_depth: 0, + }; + walk_expr(&mut visitor, body.value); + + if !visitor.found_await { + span_lint_and_help( + cx, + ASYNC_BLOCK_WITHOUT_AWAIT, + expr.span, + "this `async` block contains no `.await`", + None, + "consider removing the `async` block or adding the missing `.await`", + ); + } + } +} + +/// Walks the body of an async block looking for `.await` expressions. Tracks +/// nesting depth so that an `.await` inside a *nested* async block is not +/// attributed to the *outer* block. +struct AwaitVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + found_await: bool, + async_depth: usize, +} + +impl<'tcx> Visitor<'tcx> for AwaitVisitor<'_, 'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { + self.cx.tcx + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + if let ExprKind::Yield(_, YieldSource::Await { .. }) = expr.kind { + if self.async_depth == 0 { + self.found_await = true; + return; + } + } + + let is_nested_async_block = matches!( + expr.kind, + ExprKind::Closure(Closure { + kind: ClosureKind::Coroutine(CoroutineKind::Desugared( + CoroutineDesugaring::Async, + _ + )), + .. + }) + ); + + if is_nested_async_block { + self.async_depth += 1; + } + + walk_expr(self, expr); + + if is_nested_async_block { + self.async_depth -= 1; + } + } +} + +// =========================================================================== +// Tests +// =========================================================================== + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::process::Command; + + /// Build the test-fixture `gpui` crate and return rustc flags that make + /// it available to standalone UI test files via `extern crate gpui`. + fn gpui_fixture_rustc_flags() -> Vec { + let fixture_dir: PathBuf = [env!("CARGO_MANIFEST_DIR"), "test_fixture"] + .iter() + .collect(); + + let status = Command::new("cargo") + .args(["build", "--package", "gpui"]) + .current_dir(&fixture_dir) + .status() + .expect("failed to run cargo build for gpui fixture"); + assert!(status.success(), "gpui fixture build failed"); + + let rlib: PathBuf = fixture_dir.join("target/debug/libgpui.rlib"); + let deps: PathBuf = fixture_dir.join("target/debug/deps"); + + vec![ + "--edition=2021".to_string(), + format!("--extern=gpui={}", rlib.display()), + format!("-Ldependency={}", deps.display()), + ] + } + + #[test] + fn ui() { + let flags = gpui_fixture_rustc_flags(); + dylint_testing::ui::Test::src_base(env!("CARGO_PKG_NAME"), "ui") + .rustc_flags(flags) + .run(); + } + + #[test] + fn ui_shared_string() { + dylint_testing::ui_test_examples(env!("CARGO_PKG_NAME")); + } +} diff --git a/tooling/lints/src/notify_in_render.rs b/tooling/lints/src/notify_in_render.rs new file mode 100644 index 00000000000..edc5fb26f4d --- /dev/null +++ b/tooling/lints/src/notify_in_render.rs @@ -0,0 +1,58 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; + +use crate::render_helpers::{is_directly_in_render_method, is_gpui_context}; + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags calls to `Context::notify()` that execute synchronously inside a + /// `Render::render` method. + /// + /// ### Why is this bad? + /// + /// `notify()` tells the framework that the entity's state has changed and + /// it should be re-rendered. Calling it during render means every render + /// pass schedules another render pass — either an infinite loop or wasted + /// work. + pub NOTIFY_IN_RENDER, + Warn, + "calling `cx.notify()` during render schedules a redundant re-render" +} + +pub(crate) struct NotifyInRender; + +rustc_session::impl_lint_pass!(NotifyInRender => [NOTIFY_IN_RENDER]); + +impl<'tcx> LateLintPass<'tcx> for NotifyInRender { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if expr.span.from_expansion() { + return; + } + + let ExprKind::MethodCall(segment, receiver, _args, _span) = &expr.kind else { + return; + }; + + if segment.ident.name.as_str() != "notify" { + return; + } + + let receiver_ty = cx.typeck_results().expr_ty(receiver); + if !is_gpui_context(cx, receiver_ty) { + return; + } + + if !is_directly_in_render_method(cx, expr.hir_id) { + return; + } + + span_lint( + cx, + NOTIFY_IN_RENDER, + expr.span, + "`cx.notify()` called during render schedules a re-render every render pass", + ); + } +} diff --git a/tooling/lints/src/owned_string_into_shared.rs b/tooling/lints/src/owned_string_into_shared.rs new file mode 100644 index 00000000000..a47f2f43432 --- /dev/null +++ b/tooling/lints/src/owned_string_into_shared.rs @@ -0,0 +1,171 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_ast::ast::LitKind; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Ty; + +rustc_session::declare_lint! { + /// ### What it does + /// + /// Flags expressions that build an owned `String` from a string literal + /// and then immediately convert it with `.into()` into one of the + /// refcounted/shared string types: `gpui::SharedString`, `Arc`, + /// `Rc`, or `Cow<'_, str>`. + /// + /// The flagged shapes are: + /// + /// ```ignore + /// let label: SharedString = String::from("foo").into(); + /// let key: Arc = "foo".to_string().into(); + /// let value: Rc = "foo".to_owned().into(); + /// ``` + /// + /// ### Why is this bad? + /// + /// Two heap allocations and two copies of the literal happen where one is + /// enough: `String::from` (or `to_string`/`to_owned`) allocates a `String` + /// and copies the bytes; the `.into()` conversion into the refcounted + /// destination then allocates an `Arc`-like buffer and copies the + /// bytes a second time. For string literals the destination can be built + /// directly from `'static` data with no allocation at all. + pub OWNED_STRING_INTO_SHARED, + Warn, + "an owned `String` is built from a string literal only to be converted into a refcounted string" +} + +pub(crate) struct OwnedStringIntoShared; + +rustc_session::impl_lint_pass!(OwnedStringIntoShared => [OWNED_STRING_INTO_SHARED]); + +impl<'tcx> LateLintPass<'tcx> for OwnedStringIntoShared { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if expr.span.from_expansion() { + return; + } + + let ExprKind::MethodCall(segment, receiver, [], _) = &expr.kind else { + return; + }; + if segment.ident.name.as_str() != "into" { + return; + } + + let dest_ty = cx.typeck_results().expr_ty(expr); + if !is_refcounted_string_destination(cx, dest_ty) { + return; + } + + // The receiver must produce an owned `String`. Confirming this rules + // out custom `into` impls on unrelated types that just happen to look + // similar. + let receiver_ty = cx.typeck_results().expr_ty(receiver); + if !is_std_string(cx, receiver_ty) { + return; + } + + if !is_owned_string_built_from_literal(cx, receiver) { + return; + } + + span_lint( + cx, + OWNED_STRING_INTO_SHARED, + expr.span, + "this allocates an owned `String` from a string literal only to convert it into a refcounted string", + ); + } +} + +/// Returns `true` when `ty` is one of the refcounted/shared string types this +/// lint targets: `gpui::SharedString`, `Arc`, `Rc`, or +/// `Cow<'_, str>`. +fn is_refcounted_string_destination<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + let Some(adt) = ty.ty_adt_def() else { + return false; + }; + let did = adt.did(); + + if cx.tcx.crate_name(did.krate).as_str() == "gpui_shared_string" + && cx.tcx.item_name(did).as_str() == "SharedString" + { + return true; + } + + let path = cx.tcx.def_path_str(did); + let is_str_wrapper = matches!( + path.as_str(), + "alloc::sync::Arc" + | "std::sync::Arc" + | "alloc::rc::Rc" + | "std::rc::Rc" + | "alloc::borrow::Cow" + | "std::borrow::Cow" + ); + if !is_str_wrapper { + return false; + } + + let rustc_middle::ty::TyKind::Adt(_, args) = ty.kind() else { + return false; + }; + args.iter() + .find_map(|arg| arg.as_type()) + .is_some_and(|inner| inner.is_str()) +} + +/// Returns `true` when `ty` is `alloc::string::String`. +fn is_std_string(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + let Some(adt) = ty.ty_adt_def() else { + return false; + }; + let path = cx.tcx.def_path_str(adt.did()); + path == "alloc::string::String" || path == "std::string::String" +} + +/// Returns `true` if `expr` matches one of: +/// +/// * `String::from()` +/// * `.to_string()` +/// * `.to_owned()` +fn is_owned_string_built_from_literal<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, +) -> bool { + match &expr.kind { + ExprKind::Call(func, [arg]) => { + let ExprKind::Path(qpath) = &func.kind else { + return false; + }; + let Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) = cx.qpath_res(qpath, func.hir_id) + else { + return false; + }; + if cx.tcx.item_name(def_id).as_str() != "from" { + return false; + } + is_string_literal(arg) + } + ExprKind::MethodCall(segment, receiver, [], _) => { + let name = segment.ident.name.as_str(); + if name != "to_string" && name != "to_owned" { + return false; + } + is_string_literal(receiver) + } + _ => false, + } +} + +/// Returns `true` if `expr` is a string-literal expression, optionally wrapped +/// in a single layer of reference (`&"lit"`). +fn is_string_literal<'tcx>(expr: &'tcx Expr<'tcx>) -> bool { + let inner = match &expr.kind { + ExprKind::AddrOf(_, _, inner) => *inner, + _ => expr, + }; + matches!( + &inner.kind, + ExprKind::Lit(lit) if matches!(lit.node, LitKind::Str(..)) + ) +} diff --git a/tooling/lints/src/render_helpers.rs b/tooling/lints/src/render_helpers.rs new file mode 100644 index 00000000000..f85adacc0cc --- /dev/null +++ b/tooling/lints/src/render_helpers.rs @@ -0,0 +1,98 @@ +use rustc_hir::def_id::DefId; +use rustc_hir::{ExprKind, HirId, Node}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; + +/// Returns `true` when `hir_id` sits directly inside a `fn render` that +/// implements `gpui::Render` or `gpui::RenderOnce`, without an intervening +/// closure. If a closure sits between `hir_id` and the `render` method, the +/// expression executes later (e.g. in an event handler) and is not flagged. +pub(crate) fn is_directly_in_render_method(cx: &LateContext<'_>, hir_id: HirId) -> bool { + for (parent_id, node) in cx.tcx.hir_parent_iter(hir_id) { + match node { + Node::Expr(expr) if matches!(expr.kind, ExprKind::Closure(_)) => { + return false; + } + Node::ImplItem(impl_item) if impl_item.ident.name.as_str() == "render" => { + return is_render_trait_impl(cx, parent_id); + } + _ => {} + } + } + false +} + +/// Returns `true` when the `impl` block that owns `impl_item_hir_id` is an +/// implementation of `gpui::Render` or `gpui::RenderOnce`. +fn is_render_trait_impl(cx: &LateContext<'_>, impl_item_hir_id: HirId) -> bool { + let parent_owner = cx.tcx.hir_get_parent_item(impl_item_hir_id); + let node = cx.tcx.hir_node(parent_owner.into()); + if let Node::Item(item) = node { + if let rustc_hir::ItemKind::Impl(impl_block) = &item.kind { + if let Some(trait_ref) = &impl_block.of_trait { + if let rustc_hir::def::Res::Def(_, trait_def_id) = trait_ref.trait_ref.path.res { + return is_gpui_render_trait(cx, trait_def_id); + } + } + } + } + false +} + +fn is_gpui_render_trait(cx: &LateContext<'_>, trait_def_id: DefId) -> bool { + let crate_name = cx.tcx.crate_name(trait_def_id.krate); + if crate_name.as_str() != "gpui" { + return false; + } + let name = cx.tcx.item_name(trait_def_id); + name.as_str() == "Render" || name.as_str() == "RenderOnce" +} + +/// Returns `true` when `ty` is `gpui::Entity` or `gpui::WeakEntity`. +pub(crate) fn is_gpui_entity_or_weak(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + let peeled = ty.peel_refs(); + let Some(adt) = peeled.ty_adt_def() else { + return false; + }; + let did = adt.did(); + let crate_name = cx.tcx.crate_name(did.krate); + if crate_name.as_str() != "gpui" { + return false; + } + let name = cx.tcx.item_name(did); + name.as_str() == "Entity" || name.as_str() == "WeakEntity" +} + +/// Returns `true` when `ty` is `gpui::Context`. +pub(crate) fn is_gpui_context(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + let peeled = ty.peel_refs(); + let Some(adt) = peeled.ty_adt_def() else { + return false; + }; + let did = adt.did(); + let crate_name = cx.tcx.crate_name(did.krate); + if crate_name.as_str() != "gpui" { + return false; + } + cx.tcx.item_name(did).as_str() == "Context" +} + +/// Returns `true` when the expression type indicates a unit-returning update +/// call — either `()` (from `Entity::update`) or `Result<(), _>` (from +/// `WeakEntity::update`). +pub(crate) fn is_unit_or_result_unit(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + if ty.is_unit() { + return true; + } + if let Some(adt) = ty.ty_adt_def() { + let path = cx.tcx.def_path_str(adt.did()); + if path == "core::result::Result" || path == "std::result::Result" { + if let Some(substs) = ty.walk().nth(1) { + if let Some(inner_ty) = substs.as_type() { + return inner_ty.is_unit(); + } + } + } + } + false +} diff --git a/tooling/lints/test_fixture/Cargo.toml b/tooling/lints/test_fixture/Cargo.toml new file mode 100644 index 00000000000..a07c992855c --- /dev/null +++ b/tooling/lints/test_fixture/Cargo.toml @@ -0,0 +1,6 @@ +[workspace] +resolver = "2" +members = ["gpui", "gpui_shared_string", "consumer", "render_consumer"] + +[workspace.metadata.dylint] +libraries = [{ path = ".." }] diff --git a/tooling/lints/test_fixture/consumer/Cargo.toml b/tooling/lints/test_fixture/consumer/Cargo.toml new file mode 100644 index 00000000000..dd822953c2c --- /dev/null +++ b/tooling/lints/test_fixture/consumer/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "consumer" +version = "0.0.0" +edition = "2021" +publish = false + +[lib] +path = "src/lib.rs" + +[dependencies] +gpui_shared_string = { path = "../gpui_shared_string" } diff --git a/tooling/lints/test_fixture/consumer/src/lib.rs b/tooling/lints/test_fixture/consumer/src/lib.rs new file mode 100644 index 00000000000..6b5d905664c --- /dev/null +++ b/tooling/lints/test_fixture/consumer/src/lib.rs @@ -0,0 +1,63 @@ +use gpui_shared_string::SharedString; + +// Should fire: `from` with a short literal (≤ 23 bytes). +pub fn from_short() -> SharedString { + SharedString::from("Favorites") +} + +// Should fire at elevated severity: `from` with a long literal (> 23 bytes). +pub fn from_long() -> SharedString { + SharedString::from("Right-click for more options") +} + +// Should fire: explicit `.into()` from a string literal. +pub fn into_short() -> SharedString { + "hello".into() +} + +// Should fire: `SharedString::new("...")`. +pub fn new_short() -> SharedString { + SharedString::new("hi") +} + +// Should NOT fire: the zero-cost constructor. +pub fn new_static_short() -> SharedString { + SharedString::new_static("Favorites") +} + +// Should NOT fire: non-literal input. +pub fn from_variable(s: &str) -> SharedString { + SharedString::from(s) +} + +// Should NOT fire: `.into()` on a non-literal. +pub fn into_variable(s: &str) -> SharedString { + s.into() +} + +// Should fire: `.into()` on a string literal that exceeds the 23-byte cap. +pub fn into_long() -> SharedString { + "this literal is definitely longer than twenty three bytes".into() +} + +// ---- owned_string_into_shared cases targeting `SharedString` ---- + +// Should fire (owned_string_into_shared): `String::from().into()`. +pub fn shared_string_from_string_from() -> SharedString { + String::from("label").into() +} + +// Should fire (owned_string_into_shared): `.to_string().into()`. +pub fn shared_string_from_to_string() -> SharedString { + "label".to_string().into() +} + +// Should fire (owned_string_into_shared): `.to_owned().into()`. +pub fn shared_string_from_to_owned() -> SharedString { + "label".to_owned().into() +} + +// Should NOT fire owned_string_into_shared: the source is a non-literal `String`. +pub fn shared_string_from_dynamic_string(s: String) -> SharedString { + s.into() +} diff --git a/tooling/lints/test_fixture/gpui/Cargo.toml b/tooling/lints/test_fixture/gpui/Cargo.toml new file mode 100644 index 00000000000..5d01d426808 --- /dev/null +++ b/tooling/lints/test_fixture/gpui/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "gpui" +version = "0.0.0" +edition = "2021" +publish = false + +[lib] +path = "src/lib.rs" diff --git a/tooling/lints/test_fixture/gpui/src/lib.rs b/tooling/lints/test_fixture/gpui/src/lib.rs new file mode 100644 index 00000000000..7d863fbe836 --- /dev/null +++ b/tooling/lints/test_fixture/gpui/src/lib.rs @@ -0,0 +1,100 @@ +//! Minimal stand-in for the real `gpui` crate. The lints key off the crate +//! name and the type names, so we only need to reproduce the relevant API +//! surface. + +use std::marker::PhantomData; + +// --- AppContext --- + +pub trait AppContext { + fn as_app_mut(&mut self) -> &mut App; +} + +// --- App --- + +pub struct App; + +impl AppContext for App { + fn as_app_mut(&mut self) -> &mut App { + self + } +} + +// --- Context --- + +pub struct Context<'a, T> { + _marker: PhantomData<&'a mut T>, +} + +impl AppContext for Context<'_, T> { + fn as_app_mut(&mut self) -> &mut App { + unimplemented!() + } +} + +impl Context<'_, T> { + pub fn notify(&mut self) {} +} + +// --- Window --- + +pub struct Window; + +// --- Entity --- + +pub struct Entity { + _marker: PhantomData, +} + +impl Entity { + pub fn update( + &self, + _cx: &mut C, + _f: impl FnOnce(&mut T, &mut Context) -> R, + ) -> R { + unimplemented!() + } + + pub fn read<'a>(&self, _cx: &'a App) -> &'a T { + unimplemented!() + } + + pub fn read_with(&self, _cx: &App, _f: impl FnOnce(&T, &App) -> R) -> R { + unimplemented!() + } + + pub fn downgrade(&self) -> WeakEntity { + unimplemented!() + } +} + +// --- WeakEntity --- + +pub struct WeakEntity { + _marker: PhantomData, +} + +impl WeakEntity { + pub fn update( + &self, + _cx: &mut C, + _f: impl FnOnce(&mut T, &mut Context) -> R, + ) -> Result { + unimplemented!() + } +} + +// --- Render traits --- + +pub trait IntoElement {} + +pub trait Render: 'static + Sized { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement; +} + +pub trait RenderOnce: 'static { + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement; +} + +impl IntoElement for () {} +impl IntoElement for &str {} diff --git a/tooling/lints/test_fixture/gpui_shared_string/Cargo.toml b/tooling/lints/test_fixture/gpui_shared_string/Cargo.toml new file mode 100644 index 00000000000..4bb3411b825 --- /dev/null +++ b/tooling/lints/test_fixture/gpui_shared_string/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "gpui_shared_string" +version = "0.0.0" +edition = "2021" +publish = false + +[lib] +path = "src/lib.rs" diff --git a/tooling/lints/test_fixture/gpui_shared_string/src/lib.rs b/tooling/lints/test_fixture/gpui_shared_string/src/lib.rs new file mode 100644 index 00000000000..bce372c06ee --- /dev/null +++ b/tooling/lints/test_fixture/gpui_shared_string/src/lib.rs @@ -0,0 +1,31 @@ +//! Minimal stand-in for the real `gpui_shared_string` crate. The lint keys +//! off the crate name and the type name `SharedString`, so we only need to +//! reproduce those. + +#[derive(Clone)] +pub struct SharedString(String); + +impl SharedString { + pub const fn new_static(s: &'static str) -> Self { + // The real implementation stores the `'static` pointer; we just wrap + // an empty String at compile time to keep this `const`. + let _ = s; + SharedString(String::new()) + } + + pub fn new(s: impl AsRef) -> Self { + SharedString(s.as_ref().to_owned()) + } +} + +impl From<&str> for SharedString { + fn from(s: &str) -> Self { + SharedString(s.to_owned()) + } +} + +impl From for SharedString { + fn from(s: String) -> Self { + SharedString(s) + } +} diff --git a/tooling/lints/test_fixture/render_consumer/Cargo.toml b/tooling/lints/test_fixture/render_consumer/Cargo.toml new file mode 100644 index 00000000000..25a93028de2 --- /dev/null +++ b/tooling/lints/test_fixture/render_consumer/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "render_consumer" +version = "0.0.0" +edition = "2021" +publish = false + +[lib] +path = "src/lib.rs" + +[dependencies] +gpui = { path = "../gpui" } diff --git a/tooling/lints/test_fixture/render_consumer/src/lib.rs b/tooling/lints/test_fixture/render_consumer/src/lib.rs new file mode 100644 index 00000000000..e578844dcdb --- /dev/null +++ b/tooling/lints/test_fixture/render_consumer/src/lib.rs @@ -0,0 +1,154 @@ +#![allow(unused, dead_code)] + +use gpui::*; + +// ============================================================ +// Helper types for tests +// ============================================================ + +struct Editor; + +impl Editor { + fn set_text(&mut self, _text: &str) {} + fn text(&self) -> String { + String::new() + } +} + +struct Counter { + count: u32, + editor: Entity, +} + +struct CounterOnce { + editor: Entity, + weak_editor: WeakEntity, +} + +// ============================================================ +// entity_update_in_render — SHOULD WARN +// ============================================================ + +// Entity::update with unit-returning closure in Render::render +impl Render for Counter { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + self.editor.update(cx, |editor, _cx| { + editor.set_text("hello"); + }); + () + } +} + +// Entity::update with unit-returning closure in RenderOnce::render +impl RenderOnce for CounterOnce { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + self.editor.update(cx, |editor, _cx| { + editor.set_text("hello"); + }); + () + } +} + +// WeakEntity::update with unit-returning closure in RenderOnce::render +struct WeakUpdater { + weak_editor: WeakEntity, +} + +impl RenderOnce for WeakUpdater { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + let _ = self.weak_editor.update(cx, |editor, _cx| { + editor.set_text("world"); + }); + () + } +} + +// ============================================================ +// entity_update_in_render — SHOULD NOT WARN +// ============================================================ + +// Entity::update with value-returning closure (reading, not mutating) +struct ReaderView { + editor: Entity, +} + +impl RenderOnce for ReaderView { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + let _text = self.editor.update(cx, |editor, _cx| editor.text()); + () + } +} + +// Entity::update outside render entirely +fn update_outside_render(editor: &Entity, cx: &mut App) { + editor.update(cx, |editor, _cx| { + editor.set_text("fine here"); + }); +} + +// Entity::read in render (not update) +struct ReadOnlyView { + editor: Entity, +} + +impl RenderOnce for ReadOnlyView { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + let _editor = self.editor.read(cx); + () + } +} + +// Entity::update inside a closure in render (simulating an event handler +// that executes later, not during render itself) +struct ClosureView { + editor: Entity, +} + +impl RenderOnce for ClosureView { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + let editor = self.editor; + let _handler = move |cx: &mut App| { + editor.update(cx, |editor, _cx| { + editor.set_text("inside closure"); + }); + }; + () + } +} + +// ============================================================ +// notify_in_render — SHOULD WARN +// ============================================================ + +struct NotifyView { + count: u32, +} + +impl Render for NotifyView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + self.count += 1; + cx.notify(); + () + } +} + +// ============================================================ +// notify_in_render — SHOULD NOT WARN +// ============================================================ + +// notify outside render +fn notify_outside_render(cx: &mut Context<'_, T>) { + cx.notify(); +} + +// notify inside a closure in render (event handler — runs later) +struct NotifyClosureView; + +impl Render for NotifyClosureView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let _handler = |cx: &mut Context<'_, Self>| { + cx.notify(); + }; + () + } +} diff --git a/tooling/lints/ui/async_block_without_await.rs b/tooling/lints/ui/async_block_without_await.rs new file mode 100644 index 00000000000..f95de0aa1f9 --- /dev/null +++ b/tooling/lints/ui/async_block_without_await.rs @@ -0,0 +1,79 @@ +// Tests for the `async_block_without_await` lint. + +#![allow(unused)] + +async fn returns_42() -> i32 { + 42 +} + +fn sync_fn() -> i32 { + 42 +} + +fn main() { + // --- Should warn --- + + // Empty async block. + let _f = async {}; + + // Async block with a pure expression. + let _f = async { 42 }; + + // Async move block without await. + let _f = async move { + let x = 1; + x + 2 + }; + + // Async block calling a sync function. + let _f = async { sync_fn() }; + + // Nested: the *inner* async block has no await (should warn on it). + // The outer block awaits the inner future, so the outer is fine. + let _f = async { + let inner = async { 42 }; + inner.await + }; + + // --- Should NOT warn --- + + // Async block with an await. + let _f = async { returns_42().await }; + + // Async block with await in a let binding. + let _f = async { + let x = returns_42().await; + x + 1 + }; + + // Async move block with await. + let _f = async move { returns_42().await }; + + // Regular closure (not async at all). + let _f = || 42; +} + +// --- Trait impl cases --- + +use std::future::Future; + +trait AsyncWork { + fn do_work(&self) -> impl Future; +} + +struct Worker; + +// Should NOT warn: the trait requires returning a future, so the +// implementor has no choice but to use `async { ... }`. +impl AsyncWork for Worker { + fn do_work(&self) -> impl Future { + async { 42 } + } +} + +// Should warn: inherent impl — the author chose `async` freely. +impl Worker { + fn compute(&self) -> impl Future { + async { 42 } + } +} diff --git a/tooling/lints/ui/async_block_without_await.stderr b/tooling/lints/ui/async_block_without_await.stderr new file mode 100644 index 00000000000..e5f5197af5e --- /dev/null +++ b/tooling/lints/ui/async_block_without_await.stderr @@ -0,0 +1,56 @@ +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:17:14 + | +LL | let _f = async {}; + | ^^^^^^^^ + | + = help: consider removing the `async` block or adding the missing `.await` + = note: `-D async-block-without-await` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(async_block_without_await)]` + +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:20:14 + | +LL | let _f = async { 42 }; + | ^^^^^^^^^^^^ + | + = help: consider removing the `async` block or adding the missing `.await` + +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:23:14 + | +LL | let _f = async move { + | ______________^ +LL | | let x = 1; +LL | | x + 2 +LL | | }; + | |_____^ + | + = help: consider removing the `async` block or adding the missing `.await` + +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:29:14 + | +LL | let _f = async { sync_fn() }; + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider removing the `async` block or adding the missing `.await` + +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:34:21 + | +LL | let inner = async { 42 }; + | ^^^^^^^^^^^^ + | + = help: consider removing the `async` block or adding the missing `.await` + +error: this `async` block contains no `.await` + --> $DIR/async_block_without_await.rs:77:9 + | +LL | async { 42 } + | ^^^^^^^^^^^^ + | + = help: consider removing the `async` block or adding the missing `.await` + +error: aborting due to 6 previous errors + diff --git a/tooling/lints/ui/blocking_io_on_foreground.rs b/tooling/lints/ui/blocking_io_on_foreground.rs new file mode 100644 index 00000000000..f645bebbadc --- /dev/null +++ b/tooling/lints/ui/blocking_io_on_foreground.rs @@ -0,0 +1,324 @@ +// Tests for the `blocking_io_on_foreground` lint. + +#![allow(unused, let_underscore_lock)] + +extern crate gpui; + +use gpui::*; + +struct Editor; + +// ============================================================ +// SHOULD WARN — blocking IO in functions with GPUI context params +// ============================================================ + +// --- std::fs free functions --- + +fn read_config_with_app(cx: &mut App) { + let _ = std::fs::read_to_string("config.toml"); +} + +fn write_file_with_app(cx: &App) { + let _ = std::fs::write("out.txt", b"data"); +} + +fn read_with_window(window: &mut Window, cx: &mut App) { + let _ = std::fs::read("data.bin"); +} + +fn metadata_with_app(cx: &mut App) { + let _ = std::fs::metadata("file.txt"); +} + +fn create_dir_with_app(cx: &mut App) { + let _ = std::fs::create_dir_all("some/path"); +} + +fn remove_file_with_app(cx: &mut App) { + let _ = std::fs::remove_file("old.txt"); +} + +fn canonicalize_with_app(cx: &mut App) { + let _ = std::fs::canonicalize("./relative"); +} + +fn read_dir_with_app(cx: &mut App) { + let _ = std::fs::read_dir("some/dir"); +} + +fn read_link_with_app(cx: &mut App) { + let _ = std::fs::read_link("some/link"); +} + +fn symlink_metadata_with_app(cx: &mut App) { + let _ = std::fs::symlink_metadata("file.txt"); +} + +fn set_permissions_with_app(cx: &mut App) { + if let Ok(meta) = std::fs::metadata("file.txt") { + let _ = std::fs::set_permissions("file.txt", meta.permissions()); + } +} + +fn copy_with_app(cx: &mut App) { + let _ = std::fs::copy("a.txt", "b.txt"); +} + +fn rename_with_app(cx: &mut App) { + let _ = std::fs::rename("old.txt", "new.txt"); +} + +fn hard_link_with_app(cx: &mut App) { + let _ = std::fs::hard_link("original", "link"); +} + +fn create_dir_with_app_single(cx: &mut App) { + let _ = std::fs::create_dir("one_dir"); +} + +fn remove_dir_with_app(cx: &mut App) { + let _ = std::fs::remove_dir("empty_dir"); +} + +fn remove_dir_all_with_app(cx: &mut App) { + let _ = std::fs::remove_dir_all("dir_tree"); +} + +// --- std::fs::File associated functions --- + +fn file_open_with_app(cx: &mut App) { + let _ = std::fs::File::open("data.bin"); +} + +fn file_create_with_app(cx: &mut App) { + let _ = std::fs::File::create("out.bin"); +} + +fn file_create_new_with_app(cx: &mut App) { + let _ = std::fs::File::create_new("new.bin"); +} + +// --- std::fs::File instance methods --- + +fn file_sync_all_with_app(cx: &mut App) { + if let Ok(f) = std::fs::File::open("x") { + let _ = f.sync_all(); + } +} + +fn file_sync_data_with_app(cx: &mut App) { + if let Ok(f) = std::fs::File::open("x") { + let _ = f.sync_data(); + } +} + +fn file_set_len_with_app(cx: &mut App) { + if let Ok(f) = std::fs::File::open("x") { + let _ = f.set_len(0); + } +} + +fn file_metadata_with_app(cx: &mut App) { + if let Ok(f) = std::fs::File::open("x") { + let _ = f.metadata(); + } +} + +fn file_try_clone_with_app(cx: &mut App) { + if let Ok(f) = std::fs::File::open("x") { + let _ = f.try_clone(); + } +} + +// --- std::thread --- + +fn sleep_with_context(cx: &mut Context<'_, Editor>) { + std::thread::sleep(std::time::Duration::from_millis(100)); +} + +// --- std::path::Path methods --- + +fn path_metadata_with_app(cx: &mut App) { + let _ = std::path::Path::new("file.txt").metadata(); +} + +fn path_symlink_metadata_with_app(cx: &mut App) { + let _ = std::path::Path::new("link").symlink_metadata(); +} + +fn path_read_link_with_app(cx: &mut App) { + let _ = std::path::Path::new("link").read_link(); +} + +fn path_read_dir_with_app(cx: &mut App) { + let _ = std::path::Path::new("dir").read_dir(); +} + +fn path_exists_with_app(cx: &mut App) { + let _ = std::path::Path::new("file.txt").exists(); +} + +fn path_try_exists_with_app(cx: &mut App) { + let _ = std::path::Path::new("file.txt").try_exists(); +} + +fn path_is_file_with_app(cx: &mut App) { + let _ = std::path::Path::new("file.txt").is_file(); +} + +fn path_is_dir_with_app(cx: &mut App) { + let _ = std::path::Path::new("dir").is_dir(); +} + +fn path_is_symlink_with_app(cx: &mut App) { + let _ = std::path::Path::new("link").is_symlink(); +} + +fn path_canonicalize_with_app(cx: &mut App) { + let _ = std::path::Path::new("./relative").canonicalize(); +} + +// PathBuf derefs to Path, so the same methods fire. +fn pathbuf_exists_with_app(cx: &mut App) { + let _ = std::path::PathBuf::from("file.txt").exists(); +} + +// --- std::net --- + +fn tcp_listener_bind_with_app(cx: &mut App) { + let _ = std::net::TcpListener::bind("127.0.0.1:0"); +} + +fn tcp_stream_connect_with_app(cx: &mut App) { + let _ = std::net::TcpStream::connect("127.0.0.1:80"); +} + +fn tcp_stream_connect_timeout_with_app(cx: &mut App) { + let addr = std::net::SocketAddr::from(([127, 0, 0, 1], 80)); + let _ = std::net::TcpStream::connect_timeout(&addr, std::time::Duration::from_secs(1)); +} + +fn tcp_listener_accept_with_app(cx: &mut App) { + if let Ok(listener) = std::net::TcpListener::bind("127.0.0.1:0") { + let _ = listener.accept(); + } +} + +fn udp_socket_bind_with_app(cx: &mut App) { + let _ = std::net::UdpSocket::bind("127.0.0.1:0"); +} + +fn udp_socket_send_recv_with_app(cx: &mut App) { + if let Ok(socket) = std::net::UdpSocket::bind("127.0.0.1:0") { + let mut buf = [0u8; 64]; + let _ = socket.recv_from(&mut buf); + } +} + +// --- std::process --- + +fn command_output_with_app(cx: &mut App) { + let _ = std::process::Command::new("echo").output(); +} + +fn command_status_with_app(cx: &mut App) { + let _ = std::process::Command::new("echo").status(); +} + +fn command_spawn_with_app(cx: &mut App) { + let _ = std::process::Command::new("echo").spawn(); +} + +fn child_wait_with_app(cx: &mut App) { + if let Ok(mut child) = std::process::Command::new("echo").spawn() { + let _ = child.wait(); + } +} + +fn child_wait_with_output_with_app(cx: &mut App) { + if let Ok(child) = std::process::Command::new("echo").spawn() { + let _ = child.wait_with_output(); + } +} + +// --- std::sync --- + +fn mutex_lock_with_app(cx: &mut App) { + let m = std::sync::Mutex::new(42); + let _ = m.lock(); +} + +fn rwlock_read_with_app(cx: &mut App) { + let rw = std::sync::RwLock::new(42); + let _ = rw.read(); +} + +fn rwlock_write_with_app(cx: &mut App) { + let rw = std::sync::RwLock::new(42); + let _ = rw.write(); +} + +fn barrier_wait_with_app(cx: &mut App) { + let b = std::sync::Barrier::new(1); + b.wait(); +} + +fn receiver_recv_with_app(cx: &mut App) { + let (_tx, rx) = std::sync::mpsc::channel::(); + let _ = rx.recv(); +} + +fn sync_sender_send_with_app(cx: &mut App) { + let (tx, _rx) = std::sync::mpsc::sync_channel::(1); + let _ = tx.send(42); +} + +// --- Render impl --- + +struct BlockingRenderView; + +impl Render for BlockingRenderView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let _ = std::fs::read_to_string("layout.toml"); + () + } +} + +// ============================================================ +// SHOULD NOT WARN — no GPUI context, or inside closure +// ============================================================ + +// Plain function with no GPUI parameter. +fn load_config_plain() -> String { + std::fs::read_to_string("config.toml").unwrap_or_default() +} + +// Blocking IO inside a closure (could be passed to background_spawn). +fn setup_with_closure(cx: &mut App) { + let _handler = || { + let _ = std::fs::read_to_string("config.toml"); + }; +} + +// Blocking IO in a function with no GPUI types at all. +fn standalone_sleep() { + std::thread::sleep(std::time::Duration::from_millis(10)); +} + +// Path methods with no GPUI parameter. +fn path_exists_plain() -> bool { + std::path::Path::new("file.txt").exists() +} + +// Net calls with no GPUI parameter. +fn tcp_bind_plain() { + let _ = std::net::TcpListener::bind("127.0.0.1:0"); +} + +// Mutex lock with no GPUI parameter. +fn mutex_lock_plain() { + let m = std::sync::Mutex::new(0); + let _ = m.lock(); +} + +fn main() {} diff --git a/tooling/lints/ui/blocking_io_on_foreground.stderr b/tooling/lints/ui/blocking_io_on_foreground.stderr new file mode 100644 index 00000000000..fb502d22f02 --- /dev/null +++ b/tooling/lints/ui/blocking_io_on_foreground.stderr @@ -0,0 +1,395 @@ +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:18:13 + | +LL | let _ = std::fs::read_to_string("config.toml"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D blocking-io-on-foreground` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(blocking_io_on_foreground)]` + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:22:13 + | +LL | let _ = std::fs::write("out.txt", b"data"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:26:13 + | +LL | let _ = std::fs::read("data.bin"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:30:13 + | +LL | let _ = std::fs::metadata("file.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:34:13 + | +LL | let _ = std::fs::create_dir_all("some/path"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:38:13 + | +LL | let _ = std::fs::remove_file("old.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:42:13 + | +LL | let _ = std::fs::canonicalize("./relative"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:46:13 + | +LL | let _ = std::fs::read_dir("some/dir"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:50:13 + | +LL | let _ = std::fs::read_link("some/link"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:54:13 + | +LL | let _ = std::fs::symlink_metadata("file.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:58:23 + | +LL | if let Ok(meta) = std::fs::metadata("file.txt") { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:59:17 + | +LL | let _ = std::fs::set_permissions("file.txt", meta.permissions()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:64:13 + | +LL | let _ = std::fs::copy("a.txt", "b.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:68:13 + | +LL | let _ = std::fs::rename("old.txt", "new.txt"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:72:13 + | +LL | let _ = std::fs::hard_link("original", "link"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:76:13 + | +LL | let _ = std::fs::create_dir("one_dir"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:80:13 + | +LL | let _ = std::fs::remove_dir("empty_dir"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:84:13 + | +LL | let _ = std::fs::remove_dir_all("dir_tree"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:90:13 + | +LL | let _ = std::fs::File::open("data.bin"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:94:13 + | +LL | let _ = std::fs::File::create("out.bin"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:98:13 + | +LL | let _ = std::fs::File::create_new("new.bin"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:104:20 + | +LL | if let Ok(f) = std::fs::File::open("x") { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:105:17 + | +LL | let _ = f.sync_all(); + | ^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:110:20 + | +LL | if let Ok(f) = std::fs::File::open("x") { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:111:17 + | +LL | let _ = f.sync_data(); + | ^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:116:20 + | +LL | if let Ok(f) = std::fs::File::open("x") { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:117:17 + | +LL | let _ = f.set_len(0); + | ^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:122:20 + | +LL | if let Ok(f) = std::fs::File::open("x") { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:123:17 + | +LL | let _ = f.metadata(); + | ^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:128:20 + | +LL | if let Ok(f) = std::fs::File::open("x") { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:129:17 + | +LL | let _ = f.try_clone(); + | ^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:136:5 + | +LL | std::thread::sleep(std::time::Duration::from_millis(100)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:142:13 + | +LL | let _ = std::path::Path::new("file.txt").metadata(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:146:13 + | +LL | let _ = std::path::Path::new("link").symlink_metadata(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:150:13 + | +LL | let _ = std::path::Path::new("link").read_link(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:154:13 + | +LL | let _ = std::path::Path::new("dir").read_dir(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:158:13 + | +LL | let _ = std::path::Path::new("file.txt").exists(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:162:13 + | +LL | let _ = std::path::Path::new("file.txt").try_exists(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:166:13 + | +LL | let _ = std::path::Path::new("file.txt").is_file(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:170:13 + | +LL | let _ = std::path::Path::new("dir").is_dir(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:174:13 + | +LL | let _ = std::path::Path::new("link").is_symlink(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:178:13 + | +LL | let _ = std::path::Path::new("./relative").canonicalize(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:183:13 + | +LL | let _ = std::path::PathBuf::from("file.txt").exists(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:189:13 + | +LL | let _ = std::net::TcpListener::bind("127.0.0.1:0"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:193:13 + | +LL | let _ = std::net::TcpStream::connect("127.0.0.1:80"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:198:13 + | +LL | let _ = std::net::TcpStream::connect_timeout(&addr, std::time::Duration::from_secs(1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:202:27 + | +LL | if let Ok(listener) = std::net::TcpListener::bind("127.0.0.1:0") { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:203:17 + | +LL | let _ = listener.accept(); + | ^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:208:13 + | +LL | let _ = std::net::UdpSocket::bind("127.0.0.1:0"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:212:25 + | +LL | if let Ok(socket) = std::net::UdpSocket::bind("127.0.0.1:0") { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:214:17 + | +LL | let _ = socket.recv_from(&mut buf); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:221:13 + | +LL | let _ = std::process::Command::new("echo").output(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:225:13 + | +LL | let _ = std::process::Command::new("echo").status(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:229:13 + | +LL | let _ = std::process::Command::new("echo").spawn(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:233:28 + | +LL | if let Ok(mut child) = std::process::Command::new("echo").spawn() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:234:17 + | +LL | let _ = child.wait(); + | ^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:239:24 + | +LL | if let Ok(child) = std::process::Command::new("echo").spawn() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:240:17 + | +LL | let _ = child.wait_with_output(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:248:13 + | +LL | let _ = m.lock(); + | ^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:253:13 + | +LL | let _ = rw.read(); + | ^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:258:13 + | +LL | let _ = rw.write(); + | ^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:263:5 + | +LL | b.wait(); + | ^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:268:13 + | +LL | let _ = rx.recv(); + | ^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:273:13 + | +LL | let _ = tx.send(42); + | ^^^^^^^^^^^ + +error: blocking IO call on the GPUI foreground thread + --> $DIR/blocking_io_on_foreground.rs:282:17 + | +LL | let _ = std::fs::read_to_string("layout.toml"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 65 previous errors + diff --git a/tooling/lints/ui/entity_update_in_render.rs b/tooling/lints/ui/entity_update_in_render.rs new file mode 100644 index 00000000000..d9e733902a1 --- /dev/null +++ b/tooling/lints/ui/entity_update_in_render.rs @@ -0,0 +1,116 @@ +// Tests for the `entity_update_in_render` lint. + +#![allow(unused)] + +extern crate gpui; + +use gpui::*; + +struct Counter { + value: i32, +} + +// ============================================================ +// SHOULD WARN — .update() returning () inside Render::render +// ============================================================ + +struct MutatingView { + counter: Entity, +} + +impl Render for MutatingView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + self.counter.update(cx, |counter, _cx| { + counter.value += 1; + }); + () + } +} + +// WeakEntity::update returning Result<(), _> inside Render::render +struct WeakMutatingView { + counter: WeakEntity, +} + +impl Render for WeakMutatingView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let _ = self.counter.update(cx, |counter, _cx| { + counter.value += 1; + }); + () + } +} + +// Entity::update returning () inside RenderOnce::render +struct OnceMutatingView { + counter: Entity, +} + +impl RenderOnce for OnceMutatingView { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + self.counter.update(cx, |counter, _cx| { + counter.value += 1; + }); + () + } +} + +// ============================================================ +// SHOULD NOT WARN +// ============================================================ + +// .update() returning a value (reading, not mutating) +struct ReadingView { + counter: Entity, +} + +impl Render for ReadingView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let _val: i32 = self.counter.update(cx, |counter, _cx| counter.value); + () + } +} + +// .update() inside a closure (e.g. event handler), not directly in render +struct ClosureView { + counter: Entity, +} + +impl Render for ClosureView { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let counter = &self.counter; + let _handler = |cx: &mut App| { + counter.update(cx, |counter, _cx| { + counter.value += 1; + }); + }; + () + } +} + +// .update() outside of render entirely +fn update_outside_render(entity: &Entity, cx: &mut App) { + entity.update(cx, |counter, _cx| { + counter.value += 1; + }); +} + +// .update() on a non-gpui type (unrelated method named "update") +struct FakeEntity; + +impl FakeEntity { + fn update(&self) {} +} + +struct FakeView { + thing: FakeEntity, +} + +impl Render for FakeView { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + self.thing.update(); + () + } +} + +fn main() {} diff --git a/tooling/lints/ui/entity_update_in_render.stderr b/tooling/lints/ui/entity_update_in_render.stderr new file mode 100644 index 00000000000..53d9b7d9927 --- /dev/null +++ b/tooling/lints/ui/entity_update_in_render.stderr @@ -0,0 +1,30 @@ +error: entity `.update()` called during render mutates state in the render pass + --> $DIR/entity_update_in_render.rs:23:9 + | +LL | / self.counter.update(cx, |counter, _cx| { +LL | | counter.value += 1; +LL | | }); + | |__________^ + | + = note: `-D entity-update-in-render` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(entity_update_in_render)]` + +error: entity `.update()` called during render mutates state in the render pass + --> $DIR/entity_update_in_render.rs:37:17 + | +LL | let _ = self.counter.update(cx, |counter, _cx| { + | _________________^ +LL | | counter.value += 1; +LL | | }); + | |__________^ + +error: entity `.update()` called during render mutates state in the render pass + --> $DIR/entity_update_in_render.rs:51:9 + | +LL | / self.counter.update(cx, |counter, _cx| { +LL | | counter.value += 1; +LL | | }); + | |__________^ + +error: aborting due to 3 previous errors + diff --git a/tooling/lints/ui/owned_string_into_shared.rs b/tooling/lints/ui/owned_string_into_shared.rs new file mode 100644 index 00000000000..b4dd0aa9f54 --- /dev/null +++ b/tooling/lints/ui/owned_string_into_shared.rs @@ -0,0 +1,68 @@ +// Tests for the `owned_string_into_shared` lint. + +#![allow(unused)] + +use std::borrow::Cow; +use std::rc::Rc; +use std::sync::Arc; + +fn main() { + // --- Should warn --- + + // String::from().into() into Arc. + let _a: Arc = String::from("hello").into(); + + // String::from().into() into Rc. + let _b: Rc = String::from("world").into(); + + // String::from().into() into Cow<'_, str>. + let _c: Cow<'_, str> = String::from("borrowed-or-owned").into(); + + // .to_string().into() into Arc. + let _d: Arc = "via-to-string".to_string().into(); + + // .to_owned().into() into Arc. + let _e: Arc = "via-to-owned".to_owned().into(); + + // .to_string().into() into Rc. + let _f: Rc = "rc-via-to-string".to_string().into(); + + // .to_owned().into() into Cow<'_, str>. + let _g: Cow<'_, str> = "cow-via-to-owned".to_owned().into(); + + // Long literal still flagged the same way. + let _h: Arc = + String::from("this literal is definitely longer than twenty three bytes").into(); + + // --- Should NOT warn --- + + // Direct construction from the literal — already optimal. + let _ok1: Arc = Arc::from("hello"); + let _ok2: Rc = Rc::from("world"); + let _ok3: Cow<'_, str> = Cow::Borrowed("borrowed"); + + // Producing a plain `String` (not a refcounted destination). + let _ok4: String = String::from("not refcounted"); + let _ok5: String = "x".to_string(); + let _ok6: String = "x".to_owned(); + + // `.into()` from a non-literal `String` — the allocation is unavoidable. + let dynamic: String = make_string(); + let _ok7: Arc = dynamic.into(); + + // `.into()` from a `&str` directly (no owned `String` in between). + let _ok8: Arc = "direct".into(); + + // `.into()` whose destination is not one of the targeted types. + let _ok9: Box = String::from("box-str").into(); + + // `String::new()` is not built from a literal. + let _ok10: Arc = String::new().into(); + + // Method call that is not `into`. + let _ok11: String = String::from("foo").clone(); +} + +fn make_string() -> String { + String::from("dynamic") +} diff --git a/tooling/lints/ui/owned_string_into_shared.stderr b/tooling/lints/ui/owned_string_into_shared.stderr new file mode 100644 index 00000000000..ec6e13ea508 --- /dev/null +++ b/tooling/lints/ui/owned_string_into_shared.stderr @@ -0,0 +1,53 @@ +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:13:24 + | +LL | let _a: Arc = String::from("hello").into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D owned-string-into-shared` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(owned_string_into_shared)]` + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:16:23 + | +LL | let _b: Rc = String::from("world").into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:19:28 + | +LL | let _c: Cow<'_, str> = String::from("borrowed-or-owned").into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:22:24 + | +LL | let _d: Arc = "via-to-string".to_string().into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:25:24 + | +LL | let _e: Arc = "via-to-owned".to_owned().into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:28:23 + | +LL | let _f: Rc = "rc-via-to-string".to_string().into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:31:28 + | +LL | let _g: Cow<'_, str> = "cow-via-to-owned".to_owned().into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this allocates an owned `String` from a string literal only to convert it into a refcounted string + --> $DIR/owned_string_into_shared.rs:35:9 + | +LL | String::from("this literal is definitely longer than twenty three bytes").into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors +