diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f1f73c1fd2f..8ec6cf5cbe84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3966,6 +3966,7 @@ Released 2018-09-13 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord [`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq +[`disallowed_from_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_from_async [`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods diff --git a/Cargo.toml b/Cargo.toml index 5223dca073fd..0ced7495db3a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,7 +52,7 @@ serde = { version = "1.0.125", features = ["derive"] } syn = { version = "1.0", features = ["full"] } futures = "0.3" parking_lot = "0.12" -tokio = { version = "1", features = ["io-util"] } +tokio = { version = "1", features = ["io-util", "rt"] } rustc-semver = "1.1" [build-dependencies] diff --git a/clippy_lints/src/disallowed_from_async.rs b/clippy_lints/src/disallowed_from_async.rs new file mode 100644 index 000000000000..15610ade952b --- /dev/null +++ b/clippy_lints/src/disallowed_from_async.rs @@ -0,0 +1,414 @@ +use self::persistence::CrateInfo; +use crate::utils::conf; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{fn_def_id, immediate_closure_def_id, path_def_id_including_closures}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::{ + def::Res, + def_id::{DefId, DefIdMap}, + Body, Expr, ExprKind, HirId, ImplItemKind, IsAsync, ItemKind, Node, TraitItemKind, +}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; +use std::collections::VecDeque; +use std::path::PathBuf; + +mod persistence; + +declare_clippy_lint! { + /// ### What it does + /// + /// Searches for calls to disallowed functions from an async context. + /// + /// ### Why is this bad? + /// + /// Some functions will panic when called from an async context (e.g. `block_on`), or might + /// be too heavy and risk blocking the executor for an excessive duration. + /// + /// ### Example + /// + /// ```rust + /// async fn foo_task() { + /// let handle = Handle::current(); + /// handle.block_on(async move { println!("hello from nested async!" })); + /// } + /// ``` + /// Use instead: + /// ```rust + /// async fn foo_task() { + /// let handle = Handle::current(); + /// handle.spawn_blocking(move || { + /// handle.block_on(async move { + /// println!("hello from nested async!"); + /// }) + /// }) + /// } + /// ``` + #[clippy::version = "1.65.0"] + pub DISALLOWED_FROM_ASYNC, + restriction, + "prevent certain functions from being called from an async context" +} + +const DEBUG: bool = false; + +macro_rules! mprintln { + ($($arg:tt)*) => { + if DEBUG { + println!($($arg)*); + } + } +} + +#[derive(Debug, Clone, Default)] +pub struct DisallowedFromAsync { + conf_disallowed: Vec, + conf_wrappers: Vec, + disallowed_fns: Vec, + wrapper_def_ids: Vec, + callgraph: DefIdMap, + /// Stack of expression IDs which correspond to insulating wrapper calls. + insulating_exprs: Vec, +} + +impl_lint_pass!(DisallowedFromAsync => [DISALLOWED_FROM_ASYNC]); + +#[derive(Debug, Clone, Copy)] +pub struct InsulatingExpr { + /// ID of the insulating expression, used to pop from the stack in `check_expr_post`. + expr_hir_id: HirId, + /// `DefId` of the enclosing function for this insulator. + /// + /// If another function definition is nested inside the insulator expression then its + /// def ID is checked against this value to invalidate the insulation. + caller_def_id: DefId, + /// Def ID of the insulator function. + insulator_def_id: DefId, +} + +#[derive(Debug, Clone)] +pub struct ResolvedDisallowedFunction { + fn_def_id: DefId, + /// Bottom-up call stack leading to another disallowed call (if any). + callstack: Vec, +} + +/// Information about a function call relevant to this lint. +#[derive(Debug, Clone)] +pub struct FnCall { + /// Call stack in bottom-up order, so that later entries represent calls to earlier entries. + callstack: Vec, +} + +/// Information about a function relevant to this lint. +/// +/// This struct is filled in gradually as the lint traverses each crate. +#[derive(Debug, Clone, Default)] +pub struct FnInfo { + /// Whether or not this function is async. + /// + /// This is filled in by `check_fn` when visiting the function. + asyncness: Option, + /// List of functions that call this function. + callers: Vec, + /// Span for this function's declaration. + decl_span: Option, +} + +impl DisallowedFromAsync { + pub fn new( + conf_disallowed: Vec, + conf_wrappers: Vec, + ) -> Self { + Self { + conf_disallowed, + conf_wrappers, + ..Self::default() + } + } + + fn record_call(&mut self, caller: DefId, callee: DefId) { + let fn_info = self.callgraph.entry(callee).or_insert_with(FnInfo::default); + fn_info.callers.push(caller); + } + + fn record_fn_info(&mut self, fn_def_id: DefId, asyncness: IsAsync, span: Span) { + let fn_info = self.callgraph.entry(fn_def_id).or_insert_with(FnInfo::default); + fn_info.asyncness = Some(asyncness); + fn_info.decl_span = Some(span); + } + + fn record_disallowed(&mut self, fn_def_id: DefId) { + // Assume that disallowed functions are blocking. + let fn_info = self.callgraph.entry(fn_def_id).or_insert_with(FnInfo::default); + fn_info.asyncness = Some(IsAsync::NotAsync); + } +} + +fn persistence_base_dir(cx: &LateContext<'_>) -> PathBuf { + cx.tcx.output_filenames(()).out_directory.clone() +} + +impl<'tcx> LateLintPass<'tcx> for DisallowedFromAsync { + fn check_crate(&mut self, cx: &LateContext<'tcx>) { + let base_dir = persistence_base_dir(cx); + + let path_to_def_id = |segments: &[&str]| { + if let Res::Def(_, id) = clippy_utils::def_path_res(cx, segments, None) { + Some(id) + } else if let Some(local_def_id) = clippy_utils::def_path_to_local_def_id(cx, segments) { + Some(local_def_id.to_def_id()) + } else { + None + } + }; + + // Load saved crate info for dependencies. + let tcx = cx.tcx; + for crate_num in cx.tcx.crates(()) { + let stable_crate_id = tcx.stable_crate_id(*crate_num); + let crate_info = match CrateInfo::load(&base_dir, stable_crate_id) { + Ok(info) => info, + Err(e) => { + mprintln!("No crate info for crate {:?}: {}", crate_num, e); + continue; + }, + }; + + self.disallowed_fns.extend(crate_info.get_tainted_function_def_ids(tcx)); + } + + for conf in self.conf_disallowed.clone() { + let segs: Vec<_> = conf.path().split("::").collect(); + if let Some(id) = path_to_def_id(&segs) { + self.disallowed_fns.push(ResolvedDisallowedFunction { + fn_def_id: id, + callstack: vec![], + }); + self.record_disallowed(id); + } else { + mprintln!("WARNING: unable to resolve disallowed {}", conf.path()); + } + } + + for conf in self.conf_wrappers.clone() { + let segs: Vec<_> = conf.path().split("::").collect(); + if let Some(id) = path_to_def_id(&segs) { + self.wrapper_def_ids.push(id); + self.record_disallowed(id); + } else { + mprintln!("WARNING: unable to resolve wrapper {}", conf.path()); + } + } + } + + fn check_crate_post(&mut self, cx: &LateContext<'_>) { + /* + mprintln!("here's the call graph:"); + for (def_id, fn_info) in &self.callgraph { + mprintln!("[#{:?}] {:?} => {:#?}", def_id.index, def_id, fn_info); + } + */ + + // Set of visited definition IDs, to avoid revisiting the same nodes, including for + // recursive calls (FIXME(sproul): check this actually works for recursive calls). + let mut visited = FxHashSet::default(); + + // Persist disallowed functions to disk for use by downstream crates. + let stable_crate_id = cx.sess().local_stable_crate_id(); + let mut persisted_crate_info = CrateInfo::new(stable_crate_id); + + for disallowed_fn in &self.disallowed_fns { + let disallowed_def_id = disallowed_fn.fn_def_id; + + // Stack of nodes to explore. + let mut stack = VecDeque::new(); + stack.push_back(FnCall { + callstack: vec![disallowed_def_id], + }); + + while let Some(current_call) = stack.pop_front() { + let current_node_id = current_call.callstack.last().copied().expect("has last caller"); + + // Add to visited set, and skip this node if it was already present. + // It's safe to avoid re-visiting nodes because any node that is in the visited + // set is either: + // + // 1. Itself an insulating wrapper, implying that all calls below it are safe. + // 2. An async function reachable from a disallowed function, in which case + // we have already reported an error when visiting it the first time. + // 3. A blocking function reachable from a disallowed function, in which case we + // have already recorded it as tainted and added all of its callers to the + // search graph. + if !visited.insert(current_node_id) { + continue; + } + + // Check if current node is a known insulator, which terminates the traversal here. + if self.wrapper_def_ids.contains(¤t_node_id) { + mprintln!("terminating graph traversal at insulator"); + continue; + } + + let current_node = match self.callgraph.get(¤t_node_id) { + Some(fn_info) => fn_info, + None => { + // Definition is missing from callgraph, this is possibly bad. + continue; + }, + }; + mprintln!("visiting node {:#?}", current_node); + + // If current node is async then there exists a path from the forbidden function + // to this function with no insulating wrapper, so we've found a violation. + if current_node.asyncness == Some(IsAsync::Async) { + let msg = "async function calls a function which is forbidden from being \ + called in an async context"; + let span = current_node + .decl_span + .expect("span for non-terminal function should be known"); + + span_lint_and_then(cx, DISALLOWED_FROM_ASYNC, span, &msg, |diag| { + let show_def_id = |def_id| { + let path = cx.tcx.def_path_str(def_id); + let span = cx.tcx.def_span(def_id); + format!("{} @ {:?}", path, span) + }; + + diag.note(&format!( + "calls disallowed function `{}`", + show_def_id(disallowed_def_id) + )); + + let callstack_paths = current_call + .callstack + .iter() + .rev() + .copied() + .map(show_def_id) + .collect::>(); + + diag.note(&format!("called via: {}", callstack_paths.join("\n"))); + + if !disallowed_fn.callstack.is_empty() { + let other_disallowed_callstack = disallowed_fn + .callstack + .iter() + .rev() + .copied() + .map(show_def_id) + .collect::>(); + + diag.note(&format!( + "{} itself calls another disallowed function via: {}", + show_def_id(disallowed_def_id), + other_disallowed_callstack.join("\n") + )); + } + }); + } else { + // If the current function is blocking (FIXME(sproul): and public), then it needs to + // be recorded in the persisted crate info so that calls to it can be checked + // from dependent crates. + let def_path_hash = cx.tcx.def_path_hash(current_node_id); + persisted_crate_info.record_tainted_function(cx.tcx, def_path_hash, ¤t_call.callstack); + + // Check all callers of this function. + for caller in ¤t_node.callers { + let mut fn_call = current_call.clone(); + fn_call.callstack.push(*caller); + + stack.push_back(fn_call); + } + } + } + } + + // Save the persisted crate info to disk. + if let Err(e) = persisted_crate_info.store(&persistence_base_dir(cx)) { + panic!("error saving persisted crate info: {}", e); + } + } + + fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) { + let hir = cx.tcx.hir(); + let body_owner_def_id = hir.body_owner_def_id(body.id()).to_def_id(); + let asyncness = body.generator_kind.map_or(IsAsync::NotAsync, |_| IsAsync::Async); + self.record_fn_info(body_owner_def_id, asyncness, body.value.span); + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // FIXME(sproul): allow recursion as per disallowed_methods + let opt_callee = path_def_id_including_closures(cx, expr) + .or_else(|| fn_def_id(cx, expr)) + .or_else(|| immediate_closure_def_id(cx, expr)); + + let callee = match opt_callee { + Some(def_id) => def_id, + None => { + return; + }, + }; + + // Calculate the enclosing caller expression (a fn decl, closure decl or generator). + let hir = cx.tcx.hir(); + let enclosing_decl_id = hir.parent_iter(expr.hir_id).find_map(|(_, node)| { + if let Node::Item(item) = node && + let ItemKind::Fn(..) = item.kind + { + Some(item.def_id.to_def_id()) + } else if let Node::TraitItem(item) = node && + let TraitItemKind::Fn(..) = item.kind + { + Some(item.def_id.to_def_id()) + } else if let Node::ImplItem(item) = node && + let ImplItemKind::Fn(..) = item.kind + { + Some(item.def_id.to_def_id()) + } else if let Node::Expr(expr) = node && + let ExprKind::Closure(closure) = expr.kind + { + Some(hir.body_owner_def_id(closure.body).to_def_id()) + } else { + None + } + }); + let enclosing_decl_id = match enclosing_decl_id { + Some(decl_id) => decl_id, + None => return, + }; + + // If the call is nested under an insulator record the caller as the insulator rather + // than the parent function. + let caller = if let Some(insulator) = self.insulating_exprs.last() && + insulator.caller_def_id == enclosing_decl_id + { + insulator.insulator_def_id + } else { + enclosing_decl_id + }; + + self.record_call(caller, callee); + + // If this expression is a call to an insulator then record it in the stack of insulating + // expressions along with the caller ID. + if self.wrapper_def_ids.contains(&callee) { + self.insulating_exprs.push(InsulatingExpr { + expr_hir_id: expr.hir_id, + caller_def_id: caller, + insulator_def_id: callee, + }); + } + } + + fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if self + .insulating_exprs + .last() + .map_or(false, |insulating_expr| insulating_expr.expr_hir_id == expr.hir_id) + { + self.insulating_exprs.pop(); + } + } +} diff --git a/clippy_lints/src/disallowed_from_async/persistence.rs b/clippy_lints/src/disallowed_from_async/persistence.rs new file mode 100644 index 000000000000..6bc4122787de --- /dev/null +++ b/clippy_lints/src/disallowed_from_async/persistence.rs @@ -0,0 +1,90 @@ +use super::ResolvedDisallowedFunction; +use rustc_data_structures::fx::FxHashMap; +use rustc_hir::{def_id::DefId, definitions::DefPathHash}; +use rustc_macros::{Decodable, Encodable}; +use rustc_middle::ty::TyCtxt; +use rustc_serialize::{ + opaque::{FileEncoder, MemDecoder}, + Decodable, Encodable, +}; +use rustc_span::def_id::StableCrateId; +use std::fs::{create_dir_all, File}; +use std::io::Read; +use std::path::{Path, PathBuf}; + +const CRATE_INFO_DIR: &str = "disallowed_from_async"; + +#[derive(Debug, Decodable, Encodable)] +pub struct TaintedFunction { + /// Call stack leading to another disallowed call (bottom-up order). + callstack: Vec, +} + +#[derive(Debug, Decodable, Encodable)] +pub struct CrateInfo { + stable_crate_id: StableCrateId, + tainted_functions: FxHashMap, +} + +impl CrateInfo { + pub fn new(stable_crate_id: StableCrateId) -> Self { + Self { + stable_crate_id, + tainted_functions: FxHashMap::default(), + } + } + + pub fn record_tainted_function(&mut self, tcx: TyCtxt<'_>, def_path_hash: DefPathHash, callstack: &[DefId]) { + self.tainted_functions.insert( + def_path_hash, + TaintedFunction { + callstack: callstack.iter().map(|def_id| tcx.def_path_hash(*def_id)).collect(), + }, + ); + } + + // FIXME(sproul): solve nasty lifetime bounds and return impl Iterator + pub fn get_tainted_function_def_ids(&self, tcx: TyCtxt<'_>) -> Vec { + // FIXME(sproul): this error handling is quite nasty + let mut err_handler = || panic!("def ID look-up failed"); + self.tainted_functions + .iter() + .map(move |(def_path_hash, tainted_fn)| { + let fn_def_id = tcx.def_path_hash_to_def_id(*def_path_hash, &mut err_handler); + let callstack = tainted_fn + .callstack + .iter() + .map(|def_path_hash| tcx.def_path_hash_to_def_id(*def_path_hash, &mut err_handler)) + .collect(); + ResolvedDisallowedFunction { fn_def_id, callstack } + }) + .collect() + } + + pub fn crate_path(base_dir: &Path) -> PathBuf { + base_dir.join(CRATE_INFO_DIR) + } + + pub fn store(&self, base_dir: &Path) -> Result<(), String> { + let crate_path = Self::crate_path(base_dir); + create_dir_all(&crate_path).map_err(|e| format!("unable to create {}: {}", crate_path.display(), e))?; + + let path = crate_path.join(format!("{}.bin", self.stable_crate_id.to_u64())); + + let mut encoder = + FileEncoder::new(&path).map_err(|e| format!("error opening {} for writing: {}", path.display(), e))?; + self.encode(&mut encoder); + encoder.flush(); + Ok(()) + } + + pub fn load(base_dir: &Path, stable_crate_id: StableCrateId) -> Result { + let path = Self::crate_path(base_dir).join(format!("{}.bin", stable_crate_id.to_u64())); + let mut bytes = vec![]; + File::open(&path) + .and_then(|mut f| f.read_to_end(&mut bytes)) + .map_err(|e| format!("error reading crate info from {}: {}", path.display(), e))?; + let mut decoder = MemDecoder::new(&bytes, 0); + Ok(Self::decode(&mut decoder)) + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4e640d7ebf65..6637adbe8229 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -37,8 +37,10 @@ extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; extern crate rustc_lint; +extern crate rustc_macros; extern crate rustc_middle; extern crate rustc_parse; +extern crate rustc_serialize; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; @@ -100,6 +102,7 @@ mod default_union_representation; mod dereference; mod derivable_impls; mod derive; +mod disallowed_from_async; mod disallowed_macros; mod disallowed_methods; mod disallowed_names; @@ -921,6 +924,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr)); store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow)); store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv))); + let disallowed_from_async_methods = conf.disallowed_from_async_methods.clone(); + let async_wrapper_methods = conf.async_wrapper_methods.clone(); + store.register_late_pass(move |_| { + Box::new(disallowed_from_async::DisallowedFromAsync::new( + disallowed_from_async_methods.clone(), + async_wrapper_methods.clone(), + )) + }); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index ab58e9b8b68d..1537800dc4d0 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -63,6 +63,37 @@ impl DisallowedPath { } } +/// A single method that is not allowed to be called (transitively) from any async function. +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum DisallowedFromAsyncMethod { + Simple(String), + WithReason { path: String, reason: Option }, +} + +impl DisallowedFromAsyncMethod { + pub fn path(&self) -> &str { + let (Self::Simple(path) | Self::WithReason { path, .. }) = self; + path + } +} + +/// A method that allows an otherwise disallowed async method to be called from an async function. +/// +/// These methods should usually be ones like `spawn_blocking` which run a closure in a blocking +/// environment. +#[derive(Clone, Debug, Deserialize)] +#[serde(transparent)] +pub struct AsyncWrapperMethod { + path: String, +} + +impl AsyncWrapperMethod { + pub fn path(&self) -> &str { + &self.path + } +} + /// Conf with parse errors #[derive(Default)] pub struct TryConf { @@ -327,6 +358,10 @@ define_Conf! { /// /// The list of disallowed types, written as fully qualified paths. (disallowed_types: Vec = Vec::new()), + /// Lint: DISALLOWED_FROM_ASYNC. + (disallowed_from_async_methods: Vec = Vec::new()), + /// Lint: DISALLOWED_FROM_ASYNC. + (async_wrapper_methods: Vec = Vec::new()), /// Lint: UNREADABLE_LITERAL. /// /// Should the fraction of a decimal be linted to include separators. diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7458cc41ef79..c7042e385dcd 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -525,6 +525,25 @@ pub fn path_def_id<'tcx>(cx: &LateContext<'_>, maybe_path: &impl MaybePath<'tcx> path_res(cx, maybe_path).opt_def_id() } +pub fn path_def_id_including_closures<'tcx>(cx: &LateContext<'_>, maybe_path: &impl MaybePath<'tcx>) -> Option { + match path_res(cx, maybe_path) { + Res::Local(hir_id) => { + let hir = cx.tcx.hir(); + let node = hir.find_parent_node(hir_id).and_then(|id| hir.find(id)); + if let Some(Node::Local(local)) = node + && let Some(init) = local.init + && let ExprKind::Closure(closure) = init.kind + { + let closure_def_id = hir.body_owner_def_id(closure.body).to_def_id(); + Some(closure_def_id) + } else { + None + } + }, + res => res.opt_def_id(), + } +} + fn find_primitive<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator + 'tcx { let single = |ty| tcx.incoherent_impls(ty).iter().copied(); let empty = || [].iter().copied(); @@ -686,6 +705,14 @@ fn def_path_res_local(cx: &LateContext<'_>, mut path: &[&str]) -> Res { Res::Err } +pub fn def_path_to_local_def_id(cx: &LateContext<'_>, path: &[&str]) -> Option { + // FIXME(sproul): this is really slow but I can't work out how else to do it + let symbol_path = path.iter().map(|s| Symbol::intern(*s)).collect::>(); + cx.tcx + .iter_local_def_id() + .find(|local_def_id| cx.match_def_path(local_def_id.to_def_id(), &symbol_path)) +} + /// Convenience function to get the `DefId` of a trait by path. /// It could be a trait or trait alias. /// @@ -2144,10 +2171,43 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { ) => { // Only return Fn-like DefIds, not the DefIds of statics/consts/etc that contain or // deref to fn pointers, dyn Fn, impl Fn - #8850 - if let Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn, id) = - cx.typeck_results().qpath_res(qpath, *path_hir_id) - { - Some(id) + // FIXME(sproul): split this out into a separate function + let qpath_res = cx.typeck_results().qpath_res(qpath, *path_hir_id); + let result = match qpath_res { + Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn | DefKind::Closure, id) => Some(id), + Res::Local(hir_id) => { + let hir = cx.tcx.hir(); + let node = hir.find_parent_node(hir_id).and_then(|id| hir.find(id)); + // println!("node is {:?}", node); + if let Some(Node::Local(local)) = node + && let Some(init) = local.init + && let ExprKind::Closure(closure) = init.kind { + let closure_def_id = hir.body_owner_def_id(closure.body).to_def_id(); + Some(closure_def_id) + } else { + None + } + }, + _ => None, + }; + /* + println!( + "qpath_res for call to {:?} with hir_id {:?} is {:?}, def_id {:?}", + qpath, path_hir_id, qpath_res, result, + ); + */ + result + }, + _ => None, + } +} + +pub fn immediate_closure_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + match &expr.kind { + ExprKind::Closure(closure) => { + let parent = get_parent_expr(cx, expr)?; + if let ExprKind::Call(..) | ExprKind::MethodCall(..) = parent.kind { + Some(cx.tcx.hir().body_owner_def_id(closure.body).to_def_id()) } else { None } diff --git a/tests/ui-toml/disallowed_from_async1/clippy.toml b/tests/ui-toml/disallowed_from_async1/clippy.toml new file mode 100644 index 000000000000..26faedca3263 --- /dev/null +++ b/tests/ui-toml/disallowed_from_async1/clippy.toml @@ -0,0 +1,4 @@ +disallowed-from-async-methods = [ + # just a string is shorthand for path only + "std::vec::Vec::new", +] diff --git a/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.rs b/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.rs new file mode 100644 index 000000000000..6c8ed73e6358 --- /dev/null +++ b/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.rs @@ -0,0 +1,22 @@ +#![warn(clippy::disallowed_from_async)] + +pub async fn call_from_async() -> Vec { + let mut v = Vec::new(); + while v.len() < 3 { + v.push(2); + } + v +} + +pub async fn call_from_closure_in_async_indirect() -> Vec { + let f = |()| { + let mut v = Vec::::new(); + while v.len() < 3 { + v.push(2); + } + v + }; + std::iter::repeat(()).take(5).flat_map(f).collect() +} + +fn main() {} diff --git a/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.stderr b/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.stderr new file mode 100644 index 000000000000..d33a12087da3 --- /dev/null +++ b/tests/ui-toml/disallowed_from_async1/conf_disallowed_from_async1.stderr @@ -0,0 +1,38 @@ +error: async function calls a function which is forbidden from being called in an async context + --> $DIR/conf_disallowed_from_async1.rs:3:44 + | +LL | pub async fn call_from_async() -> Vec { + | ____________________________________________^ +LL | | let mut v = Vec::new(); +LL | | while v.len() < 3 { +LL | | v.push(2); +LL | | } +LL | | v +LL | | } + | |_^ + | + = note: `-D clippy::disallowed-from-async` implied by `-D warnings` + = note: calls disallowed function `std::vec::Vec::::new @ /home/michael/.rustup/toolchains/nightly-2022-05-19-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:424:5: 424:31 (#0)` + = note: called via: call_from_async::{closure#0} @ $DIR/conf_disallowed_from_async1.rs:3:44: 9:2 (#0) + std::vec::Vec::::new @ /home/michael/.rustup/toolchains/nightly-2022-05-19-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:424:5: 424:31 (#0) + +error: async function calls a function which is forbidden from being called in an async context + --> $DIR/conf_disallowed_from_async1.rs:11:64 + | +LL | pub async fn call_from_closure_in_async_indirect() -> Vec { + | ________________________________________________________________^ +LL | | let f = |()| { +LL | | let mut v = Vec::::new(); +LL | | while v.len() < 3 { +... | +LL | | std::iter::repeat(()).take(5).flat_map(f).collect() +LL | | } + | |_^ + | + = note: calls disallowed function `std::vec::Vec::::new @ /home/michael/.rustup/toolchains/nightly-2022-05-19-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:424:5: 424:31 (#0)` + = note: called via: call_from_closure_in_async_indirect::{closure#0} @ $DIR/conf_disallowed_from_async1.rs:11:64: 20:2 (#0) + call_from_closure_in_async_indirect::{closure#0}::{closure#0} @ $DIR/conf_disallowed_from_async1.rs:12:13: 18:6 (#0) + std::vec::Vec::::new @ /home/michael/.rustup/toolchains/nightly-2022-05-19-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:424:5: 424:31 (#0) + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/disallowed_from_async2/clippy.toml b/tests/ui-toml/disallowed_from_async2/clippy.toml new file mode 100644 index 000000000000..f93735443f97 --- /dev/null +++ b/tests/ui-toml/disallowed_from_async2/clippy.toml @@ -0,0 +1,6 @@ +disallowed-from-async-methods = [ + "tokio::runtime::Handle::block_on" +] +async-wrapper-methods = [ + "tokio::runtime::Handle::spawn_blocking" +] diff --git a/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.rs b/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.rs new file mode 100644 index 000000000000..0d52a193ea37 --- /dev/null +++ b/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.rs @@ -0,0 +1,69 @@ +#![warn(clippy::disallowed_from_async)] +extern crate tokio; + +use tokio::runtime::Handle; + +pub async fn insulated_block_on_example_direct() { + let handle1 = Handle::current(); + let handle2 = handle1.clone(); + + handle1 + .spawn_blocking(move || handle2.block_on(async move { println!("hello from block_on") })) + .await + .unwrap() +} + +pub async fn insulated_block_on_example_indirect_closures() { + let handle1 = Handle::current(); + let handle2 = handle1.clone(); + + let task = async move { println!("hello from block_on") }; + let f = move || handle2.block_on(task); + + handle1.spawn_blocking(f).await.unwrap() +} + +pub async fn uninsulated_nested_block_on_direct() { + let handle1 = Handle::current(); + let handle2 = handle1.clone(); + let handle3 = handle1.clone(); + + handle1 + .spawn_blocking(move || { + handle2.block_on(async move { handle3.block_on(async move { println!("naughty block_on") }) }) + }) + .await + .unwrap() +} + +pub async fn uninsulated_nested_block_on_indirect_closures() { + let handle1 = Handle::current(); + let handle2 = handle1.clone(); + let handle3 = handle1.clone(); + + let task2 = async move { println!("naughty block_on") }; + let task1 = async move { handle3.block_on(task2) }; + let f = move || handle2.block_on(task1); + + handle1.spawn_blocking(f).await.unwrap() +} + +pub async fn uninsulated_nested_block_on_indirect_functions() { + async fn task2() { + println!("hello"); + } + + async fn task1() { + let handle = Handle::current(); + handle.block_on(task2()); + } + + fn f() { + let handle = Handle::current(); + handle.block_on(task1()); + } + + Handle::current().spawn_blocking(f).await.unwrap() +} + +fn main() {} diff --git a/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.stderr b/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.stderr new file mode 100644 index 000000000000..41174c0d8e6f --- /dev/null +++ b/tests/ui-toml/disallowed_from_async2/conf_disallowed_from_async2.stderr @@ -0,0 +1,37 @@ +error: async function calls a function which is forbidden from being called in an async context + --> $DIR/conf_disallowed_from_async2.rs:33:41 + | +LL | handle2.block_on(async move { handle3.block_on(async move { println!("naughty block_on") }) }) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::disallowed-from-async` implied by `-D warnings` + = note: calls disallowed function `tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0)` + = note: called via: uninsulated_nested_block_on_direct::{closure#0}::{closure#0}::{closure#0} @ $DIR/conf_disallowed_from_async2.rs:33:41: 33:106 (#0) + tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0) + +error: async function calls a function which is forbidden from being called in an async context + --> $DIR/conf_disallowed_from_async2.rs:45:28 + | +LL | let task1 = async move { handle3.block_on(task2) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: calls disallowed function `tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0)` + = note: called via: uninsulated_nested_block_on_indirect_closures::{closure#0}::{closure#1} @ $DIR/conf_disallowed_from_async2.rs:45:28: 45:55 (#0) + tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0) + +error: async function calls a function which is forbidden from being called in an async context + --> $DIR/conf_disallowed_from_async2.rs:56:22 + | +LL | async fn task1() { + | ______________________^ +LL | | let handle = Handle::current(); +LL | | handle.block_on(task2()); +LL | | } + | |_____^ + | + = note: calls disallowed function `tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0)` + = note: called via: uninsulated_nested_block_on_indirect_functions::{closure#0}::task1::{closure#0} @ $DIR/conf_disallowed_from_async2.rs:56:22: 59:6 (#0) + tokio::runtime::Handle::block_on @ /home/michael/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.18.2/src/runtime/handle.rs:287:5: 287:62 (#0) + +error: aborting due to 3 previous errors +