diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index d1d56fc84f94..42218c2c00ac 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -33,7 +33,7 @@ url = { version = "2.1.0", features = ["serde"] } deny-warnings = [] # build clippy with internal lints enabled, off by default internal-lints = ["clippy_utils/internal-lints"] -metadata-collector-lint = ["serde_json"] +metadata-collector-lint = ["serde_json", "clippy_utils/metadata-collector-lint"] [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)] diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index de94e47047b4..a16f7c2ea5e1 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -21,15 +21,11 @@ // - TODO xFrednet 2021-02-13: Collect depreciations and maybe renames use if_chain::if_chain; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashMap; use rustc_hir::{self as hir, ExprKind, Item, ItemKind, Mutability}; use rustc_lint::{CheckLintNameResult, LateContext, LateLintPass, LintContext, LintId}; -use rustc_middle::ty::{BorrowKind, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{sym, Loc, Span, Symbol}; -use rustc_trait_selection::infer::TyCtxtInferExt; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId}; -use rustc_typeck::hir_ty_to_ty; use serde::Serialize; use std::fs::{self, OpenOptions}; use std::io::prelude::*; @@ -37,8 +33,7 @@ use std::path::Path; use crate::utils::internal_lints::is_lint_ref_type; use crate::utils::{ - get_enclosing_body, get_parent_expr, get_parent_expr_for_hir, last_path_segment, match_function_call, match_qpath, - match_type, path_to_local_id, paths, span_lint, walk_ptrs_ty_depth, + last_path_segment, match_function_call, match_type, paths, span_lint, walk_ptrs_ty_depth, match_path, }; /// This is the output file of the lint collector. @@ -50,11 +45,11 @@ const BLACK_LISTED_LINTS: [&str; 2] = ["lint_author", "deep_code_inspection"]; // handling #[rustfmt::skip] const LINT_EMISSION_FUNCTIONS: [&[&str]; 5] = [ - &["clippy_lints", "utils", "diagnostics", "span_lint"], - &["clippy_lints", "utils", "diagnostics", "span_lint_and_help"], - &["clippy_lints", "utils", "diagnostics", "span_lint_and_note"], - &["clippy_lints", "utils", "diagnostics", "span_lint_hir"], - &["clippy_lints", "utils", "diagnostics", "span_lint_and_sugg"], + &["clippy_utils", "diagnostics", "span_lint"], + &["clippy_utils", "diagnostics", "span_lint_and_help"], + &["clippy_utils", "diagnostics", "span_lint_and_note"], + &["clippy_utils", "diagnostics", "span_lint_hir"], + &["clippy_utils", "diagnostics", "span_lint_and_sugg"], ]; declare_clippy_lint! { @@ -175,10 +170,10 @@ struct ApplicabilityInfo { /// [rustfix#141](https://github.com/rust-lang/rustfix/issues/141) as such suggestions can /// currently not be applied automatically. has_multi_suggestion: bool, - /// These are all the available applicability values for the lint suggestions - applicabilities: FxHashSet, + applicability: Option, } +#[allow(dead_code)] fn log_to_file(msg: &str) { let mut file = OpenOptions::new() .write(true) @@ -190,7 +185,7 @@ fn log_to_file(msg: &str) { write!(file, "{}", msg).unwrap(); } -impl<'tcx> LateLintPass<'tcx> for MetadataCollector { +impl<'hir> LateLintPass<'hir> for MetadataCollector { /// Collecting lint declarations like: /// ```rust, ignore /// declare_clippy_lint! { @@ -200,7 +195,7 @@ impl<'tcx> LateLintPass<'tcx> for MetadataCollector { /// "Who am I?" /// } /// ``` - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + fn check_item(&mut self, cx: &LateContext<'hir>, item: &'hir Item<'_>) { if_chain! { // item validation if let ItemKind::Static(ref ty, Mutability::Not, body_id) = item.kind; @@ -239,87 +234,16 @@ impl<'tcx> LateLintPass<'tcx> for MetadataCollector { /// Applicability::MachineApplicable, // <-- Extracts this constant value /// ); /// ``` - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'hir>, expr: &'hir hir::Expr<'_>) { if let Some(args) = match_simple_lint_emission(cx, expr) { - if let Some((lint_name, mut applicability)) = extract_emission_info(cx, args) { + if let Some((lint_name, applicability)) = extract_emission_info(cx, args) { let app_info = self.applicability_into.entry(lint_name).or_default(); - applicability.drain(..).for_each(|x| { - app_info.applicabilities.insert(x); - }); + app_info.applicability = applicability; } else { lint_collection_error_span(cx, expr.span, "I found this but I can't get the lint or applicability"); } } } - - /// Tracking and hopefully collecting dynamic applicability values - /// - /// Example: - /// ```rust, ignore - /// // vvv Applicability value to track - /// let mut applicability = Applicability::MachineApplicable; - /// // vvv Value Mutation - /// let suggestion = snippet_with_applicability(cx, expr.span, "_", &mut applicability); - /// // vvv Emission to link the value to the lint - /// span_lint_and_sugg( - /// cx, - /// SOME_LINT, - /// expr.span, - /// "This can be improved", - /// "try", - /// suggestion, - /// applicability, - /// ); - /// ``` - fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx hir::Local<'tcx>) { - if_chain! { - if let Some(local_ty) = get_local_type(cx, local); - if match_type(cx, local_ty, &paths::APPLICABILITY); - if let Some(body) = get_enclosing_body(cx, local.hir_id); - then { - // TODO xFrednet: 2021-02-19: Remove debug code - let span = SerializableSpan::from_span(cx, local.span); - let local_str = crate::utils::snippet(cx, local.span, "_"); - log_to_file(&format!("{} -- {}\n", local_str, span)); - - let value_hir_id = local.pat.hir_id; - let mut tracker = ValueTracker::new(cx, value_hir_id); - if let Some(init_expr) = local.init { - tracker.process_assign_expr(init_expr) - } - - // TODO xFrednet 2021-02-18: Support nested bodies - // Note: The `ExprUseVisitor` only searches though one body, this means that values - // references in nested bodies like closures are not found by this simple visitor. - cx.tcx.infer_ctxt().enter(|infcx| { - let body_owner_id = cx.tcx.hir().body_owner_def_id(body.id()); - ExprUseVisitor::new( - &mut tracker, - &infcx, - body_owner_id, - cx.param_env, - cx.typeck_results() - ) - .consume_body(body); - }); - - log_to_file(&format!("{:?}\n", tracker.value_mutations)); - } - } - } -} - -fn get_local_type<'a>(cx: &'a LateContext<'_>, local: &'a hir::Local<'_>) -> Option> { - // TODO xFrednet 2021-02-14: support nested applicability (only in tuples) - if let Some(tc) = cx.maybe_typeck_results() { - if let Some(ty) = local.ty { - return Some(hir_ty_to_ty(cx.tcx, ty)); - } else if let Some(init) = local.init { - return Some(tc.expr_ty(init)); - } - } - - None } // ================================================================== @@ -358,7 +282,7 @@ fn extract_attr_docs(item: &Item<'_>) -> Option { }) } -fn get_lint_group_or_lint(cx: &LateContext<'_>, lint_name: &str, item: &'tcx Item<'_>) -> Option { +fn get_lint_group_or_lint(cx: &LateContext<'_>, lint_name: &str, item: &'hir Item<'_>) -> Option { let result = cx.lint_store.check_lint_name(lint_name, Some(sym::clippy)); if let CheckLintNameResult::Tool(Ok(lint_lst)) = result { get_lint_group(cx, lint_lst[0]).or_else(|| { @@ -405,17 +329,19 @@ fn lint_collection_error_span(cx: &LateContext<'_>, span: Span, message: &str) { // ================================================================== // Applicability // ================================================================== -fn match_simple_lint_emission<'tcx>( - cx: &LateContext<'tcx>, - expr: &'tcx hir::Expr<'_>, -) -> Option<&'tcx [hir::Expr<'tcx>]> { +/// This function checks if a given expression is equal to a simple lint emission function call. +/// It will return the function arguments if the emission matched any function. +fn match_simple_lint_emission<'hir>( + cx: &LateContext<'hir>, + expr: &'hir hir::Expr<'_>, +) -> Option<&'hir [hir::Expr<'hir>]> { LINT_EMISSION_FUNCTIONS .iter() .find_map(|emission_fn| match_function_call(cx, expr, emission_fn)) } /// This returns the lint name and the possible applicability of this emission -fn extract_emission_info<'tcx>(cx: &LateContext<'tcx>, args: &[hir::Expr<'_>]) -> Option<(String, Vec)> { +fn extract_emission_info<'hir>(cx: &LateContext<'hir>, args: &[hir::Expr<'_>]) -> Option<(String, Option)> { let mut lint_name = None; let mut applicability = None; @@ -425,228 +351,69 @@ fn extract_emission_info<'tcx>(cx: &LateContext<'tcx>, args: &[hir::Expr<'_>]) - if match_type(cx, arg_ty, &paths::LINT) { // If we found the lint arg, extract the lint name if let ExprKind::Path(ref lint_path) = arg.kind { - lint_name = Some(last_path_segment(lint_path).ident.name) + lint_name = Some(last_path_segment(lint_path).ident.name); } } else if match_type(cx, arg_ty, &paths::APPLICABILITY) { - if let ExprKind::Path(ref path) = arg.kind { - applicability = Some(last_path_segment(path).ident.name) - } + applicability = resolve_applicability(cx, arg); } } lint_name.map(|lint_name| { ( sym_to_string(lint_name).to_ascii_lowercase(), - applicability.map(sym_to_string).map_or_else(Vec::new, |x| vec![x]), + applicability, ) }) } -#[allow(dead_code)] -struct ValueTracker<'a, 'hir> { - cx: &'a LateContext<'hir>, - value_hir_id: hir::HirId, - value_mutations: Vec>, -} - -impl<'a, 'hir> ValueTracker<'a, 'hir> { - fn new(cx: &'a LateContext<'hir>, value_hir_id: hir::HirId) -> Self { - Self { - cx, - value_hir_id, - value_mutations: Vec::new(), - } - } - - fn is_value_expr(&self, expr_id: hir::HirId) -> bool { - match self.cx.tcx.hir().find(expr_id) { - Some(hir::Node::Expr(expr)) => path_to_local_id(expr, self.value_hir_id), - _ => false, - } - } - - /// This function extracts possible `ApplicabilityModifier` from an assign statement like this: - /// - /// ```rust, ignore - /// // vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv The expression to process - /// let value = Applicability::MachineApplicable; - /// ``` - fn process_assign_expr(&mut self, expr: &'hir hir::Expr<'hir>) { - // This is a bit more complicated. I'll therefor settle on the simple solution of - // simplifying the cases we support. - match &expr.kind { - hir::ExprKind::Call(func_expr, ..) => { - // We only deal with resolved paths as this is the usual case. Other expression kinds like closures - // etc. are hard to track but might be a worthy improvement in the future - if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = func_expr.kind { - self.value_mutations.push(ApplicabilityModifier::Producer(path)); - } else { - let msg = format!( - "Unsupported assign Call expression at: {}", - SerializableSpan::from_span(self.cx, func_expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); +fn resolve_applicability(cx: &LateContext<'hir>, expr: &hir::Expr<'_>) -> Option { + match expr.kind { + // We can ignore ifs without an else block because those can't be used as an assignment + hir::ExprKind::If(_con, _if_block, Some(_else_block)) => { + // self.process_assign_expr(if_block); + // self.process_assign_expr(else_block); + return Some("TODO IF EXPR".to_string()); + }, + hir::ExprKind::Match(_expr, _arms, _) => { + // for arm in *arms { + // self.process_assign_expr(arm.body); + // } + return Some("TODO MATCH EXPR".to_string()); + }, + hir::ExprKind::Loop(block, ..) | hir::ExprKind::Block(block, ..) => { + if let Some(block_expr) = block.expr { + return resolve_applicability(cx, block_expr); + } + }, + ExprKind::Path(hir::QPath::Resolved(_, path)) => { + // direct applicabilities are simple: + for enum_value in &paths::APPLICABILITY_VALUES { + if match_path(path, enum_value) { + return Some(enum_value[2].to_string()); } - }, - hir::ExprKind::MethodCall(..) => { - let msg = format!( - "Unsupported assign MethodCall expression at: {}", - SerializableSpan::from_span(self.cx, expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); - }, - // We can ignore ifs without an else block because those can't be used as an assignment - hir::ExprKind::If(_con, if_block, Some(else_block)) => { - self.process_assign_expr(if_block); - self.process_assign_expr(else_block); - }, - hir::ExprKind::Match(_expr, arms, _) => { - for arm in *arms { - self.process_assign_expr(arm.body); - } - }, - hir::ExprKind::Loop(block, ..) | hir::ExprKind::Block(block, ..) => { - if let Some(block_expr) = block.expr { - self.process_assign_expr(block_expr); - } - }, - hir::ExprKind::Path(path) => { - for enum_value in &paths::APPLICABILITY_VALUES { - if match_qpath(path, enum_value) { - self.value_mutations - .push(ApplicabilityModifier::ConstValue(enum_value[2].to_string())); + } + + // Values yay + if let hir::def::Res::Local(local_hir) = path.res { + if let Some(local) = get_parent_local(cx, local_hir) { + if let Some(local_init) = local.init { + return resolve_applicability(cx, local_init); } } - }, - // hir::ExprKind::Field(expr, ident) => not supported - // hir::ExprKind::Index(expr, expr) => not supported - _ => { - let msg = format!( - "Unexpected assign expression at: {}", - SerializableSpan::from_span(self.cx, expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); - }, - } - } - - fn process_borrow_expr(&mut self, borrower_expr: &'hir rustc_hir::Expr<'hir>) { - match &borrower_expr.kind { - hir::ExprKind::Call(func_expr, ..) => { - // We only deal with resolved paths as this is the usual case. Other expression kinds like closures - // etc. are hard to track but might be a worthy improvement in the future - if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = func_expr.kind { - self.value_mutations.push(ApplicabilityModifier::Modifier(path)); - } else { - let msg = format!( - "Unsupported borrow in Call at: {}", - SerializableSpan::from_span(self.cx, func_expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); - } - }, - hir::ExprKind::MethodCall(..) => { - let msg = format!( - "Unsupported borrow in MethodCall at: {}", - SerializableSpan::from_span(self.cx, borrower_expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); - }, - _ => { - let msg = format!( - "Unexpected borrow at: {}", - SerializableSpan::from_span(self.cx, borrower_expr.span) - ); - self.value_mutations.push(ApplicabilityModifier::Unknown(msg)); - }, - } - } - - fn process_consume_expr(&mut self, consume_expr: &'hir rustc_hir::Expr<'hir>) { - // We are only interested in lint emissions. Other types like assignments might be - // interesting for further use or improvement but are to complex for now. - if let hir::ExprKind::Call(func_expr, ..) = &consume_expr.kind {} - } -} - -impl<'a, 'hir> Delegate<'hir> for ValueTracker<'a, 'hir> { - fn consume(&mut self, _place_with_id: &PlaceWithHirId<'hir>, expr_id: hir::HirId, _: ConsumeMode) { - if self.is_value_expr(expr_id) { - // TODO xFrednet 2021-02-17: Check if lint emission and extract lint ID - if let Some(expr) = get_parent_expr_for_hir(self.cx, expr_id) { - log_to_file(&format!("- consume {:?}\n", expr)); } } + _ => {} } - fn borrow(&mut self, _place_with_id: &PlaceWithHirId<'hir>, expr_id: hir::HirId, bk: BorrowKind) { - if_chain! { - if self.is_value_expr(expr_id); - if let BorrowKind::MutBorrow = bk; - if let Some(addr_of_expr) = get_parent_expr_for_hir(self.cx, expr_id); - if let Some(borrower_expr) = get_parent_expr(self.cx, addr_of_expr); - then { - self.process_borrow_expr(borrower_expr); - } - } + + Some("TODO".to_string()) +} + +fn get_parent_local(cx: &LateContext<'hir>, hir_id: hir::HirId) -> Option<&'hir hir::Local<'hir>> { + let map = cx.tcx.hir(); + if let Some(hir::Node::Local(local)) = map.find(map.get_parent_node(hir_id)) { + return Some(local); } - fn mutate(&mut self, _assignee_place: &PlaceWithHirId<'hir>, expr_id: hir::HirId) { - if_chain! { - if self.is_value_expr(expr_id); - if let Some(expr) = get_parent_expr_for_hir(self.cx, expr_id); - if let hir::ExprKind::Assign(_value_expr, assign_expr, ..) = expr.kind; - then { - self.process_assign_expr(assign_expr); - } - } - } -} - -/// The life of a value in Rust is a true adventure. These are the corner stones of such a -/// fairy tale. Let me introduce you to the possible stepping stones a value might have in -/// in our crazy word: -#[derive(Debug)] -#[allow(dead_code)] -enum ApplicabilityModifier<'hir> { - Unknown(String), - /// A simple constant value. - /// - /// This is the actual character of a value. It's baseline. This only defines where the value - /// started. As in real life it can still change and fully decide who it wants to be. - ConstValue(String), - /// A producer is a function that returns an applicability value. - /// - /// This is the heritage of this value. This value comes from a long family tree and is not - /// just a black piece of paper. The evaluation of this stepping stone needs additional - /// context. We therefore only add a reference. This reference will later be used to ask - /// the librarian about the possible initial character that this value might have. - Producer(&'hir hir::Path<'hir>), - /// A modifier that takes the given applicability and might modify it - /// - /// What would an RPG be without it's NPCs. The special thing about modifiers is that they can - /// be actively interested in the story of the value and might make decisions based on the - /// character of this hero. This means that a modifier doesn't just force its way into the life - /// of our hero but it actually asks him how he's been. The possible modification is a result - /// of the situation. - /// - /// Take this part of our heroes life very seriously! - Modifier(&'hir hir::Path<'hir>), - /// The actual emission of a lint - /// - /// All good things must come to an end. Even the life of your awesome applicability hero. He - /// was the bravest soul that has ever wondered this earth. Songs will be written about his - /// heroic deeds. Castles will be named after him and the world how we know it will never be - /// the same! - /// - /// Is this a happy ending? Did he archive what he wanted in his life? Yes, YES, he has lived a - /// life and he will continue to live in all the lint suggestions that can be applied or just - /// displayed by Clippy. He might be no more, but his legacy will serve generations to come. - LintEmit(LintEmission), -} - -#[derive(Debug)] -struct LintEmission { - lint: String, - is_multi_line_sugg: bool, + None } diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index d04c5f889dda..6e158c8ce723 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -15,6 +15,7 @@ rustc-semver="1.1.0" [features] internal-lints = [] +metadata-collector-lint = [] [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)] diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 94eade0c9324..d4bc42657f48 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -821,7 +821,13 @@ pub fn get_parent_node(tcx: TyCtxt<'_>, id: HirId) -> Option> { /// Gets the parent expression, if any –- this is useful to constrain a lint. pub fn get_parent_expr<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { - match get_parent_node(cx.tcx, e.hir_id) { + get_parent_expr_for_hir(cx, e.hir_id) +} + +/// This retrieves the parent for the given `HirId` if it's an expression. This is useful for +/// constraint lints +pub fn get_parent_expr_for_hir<'tcx>(cx: &LateContext<'tcx>, hir_id: hir::HirId) -> Option<&'tcx Expr<'tcx>> { + match get_parent_node(cx.tcx, hir_id) { Some(Node::Expr(parent)) => Some(parent), _ => None, }