Metadata collection: Refining the implementation

This commit is contained in:
xFrednet 2021-03-01 12:25:14 +01:00
parent 6658db1044
commit 2ce5e368d8
4 changed files with 5836 additions and 200 deletions

2
.gitignore vendored
View File

@ -29,7 +29,7 @@ out
# gh pages docs
util/gh-pages/lints.json
**/metadata_collection.json
# **/metadata_collection.json
# rustfmt backups
*.rs.bk

View File

@ -7,7 +7,7 @@ use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, walk_stmt, NestedVisitorMap, Visitor};
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass, Lint};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::sym;

View File

@ -33,7 +33,7 @@
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{self as hir, intravisit, ExprKind, Item, ItemKind, Mutability, QPath};
use rustc_hir::{self as hir, intravisit, intravisit::Visitor, ExprKind, Item, ItemKind, Mutability, QPath};
use rustc_lint::{CheckLintNameResult, LateContext, LateLintPass, LintContext, LintId};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -52,19 +52,18 @@ use crate::utils::{
const OUTPUT_FILE: &str = "metadata_collection.json";
/// These lints are excluded from the export.
const BLACK_LISTED_LINTS: [&str; 2] = ["lint_author", "deep_code_inspection"];
/// These groups will be ignored by the lint group matcher
const BLACK_LISTED_LINT_GROUP: [&str; 1] = ["clippy::all", "clippy::internal"];
/// These groups will be ignored by the lint group matcher. This is useful for collections like
/// `clippy::all`
const IGNORED_LINT_GROUPS: [&str; 1] = ["clippy::all"];
/// Lints within this group will be excluded from the collection
const EXCLUDED_LINT_GROUPS: [&str; 1] = ["clippy::internal"];
// TODO xFrednet 2021-02-15: `span_lint_and_then` & `span_lint_hir_and_then` requires special
// handling
const SIMPLE_LINT_EMISSION_FUNCTIONS: [&[&str]; 5] = [
const LINT_EMISSION_FUNCTIONS: [&[&str]; 7] = [
&["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"],
];
const COMPLEX_LINT_EMISSION_FUNCTIONS: [&[&str]; 2] = [
&["clippy_utils", "diagnostics", "span_lint_and_then"],
&["clippy_utils", "diagnostics", "span_lint_hir_and_then"],
];
@ -270,18 +269,11 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
/// );
/// ```
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, applicability)) = extract_emission_info(cx, args) {
if let Some(args) = match_lint_emission(cx, expr) {
if let Some((lint_name, applicability, is_multi_part)) = extract_complex_emission_info(cx, args) {
let app_info = self.applicability_into.entry(lint_name).or_default();
app_info.applicability = applicability;
} else {
lint_collection_error_span(cx, expr.span, "I found this but I can't get the lint or applicability");
}
} else if let Some(args) = match_complex_lint_emission(cx, expr) {
if let Some((lint_name, applicability, is_multi_span)) = extract_complex_emission_info(cx, args) {
let app_info = self.applicability_into.entry(lint_name).or_default();
app_info.applicability = applicability;
app_info.is_multi_suggestion = is_multi_span;
app_info.is_multi_suggestion = is_multi_part;
} else {
lint_collection_error_span(cx, expr.span, "Look, here ... I have no clue what todo with it");
}
@ -292,7 +284,6 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector {
// ==================================================================
// Lint definition extraction
// ==================================================================
fn sym_to_string(sym: Symbol) -> String {
sym.as_str().to_string()
}
@ -328,10 +319,12 @@ fn extract_attr_docs(item: &Item<'_>) -> Option<String> {
fn get_lint_group_or_lint(cx: &LateContext<'_>, lint_name: &str, item: &'hir Item<'_>) -> Option<String> {
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(|| {
lint_collection_error_item(cx, item, "Unable to determine lint group");
None
})
get_lint_group(cx, lint_lst[0])
.or_else(|| {
lint_collection_error_item(cx, item, "Unable to determine lint group");
None
})
.filter(|group| !EXCLUDED_LINT_GROUPS.contains(&group.as_str()))
} else {
lint_collection_error_item(cx, item, "Unable to find lint in lint_store");
None
@ -340,7 +333,7 @@ fn get_lint_group_or_lint(cx: &LateContext<'_>, lint_name: &str, item: &'hir Ite
fn get_lint_group(cx: &LateContext<'_>, lint_id: LintId) -> Option<String> {
for (group_name, lints, _) in &cx.lint_store.get_lint_groups() {
if BLACK_LISTED_LINT_GROUP.contains(group_name) {
if IGNORED_LINT_GROUPS.contains(group_name) {
continue;
}
@ -378,31 +371,19 @@ fn lint_collection_error_span(cx: &LateContext<'_>, span: Span, message: &str) {
// ==================================================================
/// 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>]> {
SIMPLE_LINT_EMISSION_FUNCTIONS
fn match_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))
}
fn match_complex_lint_emission<'hir>(
cx: &LateContext<'hir>,
expr: &'hir hir::Expr<'_>,
) -> Option<&'hir [hir::Expr<'hir>]> {
COMPLEX_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<'hir>(
fn extract_complex_emission_info<'hir>(
cx: &LateContext<'hir>,
args: &'hir [hir::Expr<'hir>],
) -> Option<(String, Option<String>)> {
) -> Option<(String, Option<String>, bool)> {
let mut lint_name = None;
let mut applicability = None;
let mut multi_part = false;
for arg in args {
let (arg_ty, _) = walk_ptrs_ty_depth(cx.typeck_results().expr_ty(&arg));
@ -414,109 +395,45 @@ fn extract_emission_info<'hir>(
}
} else if match_type(cx, arg_ty, &paths::APPLICABILITY) {
applicability = resolve_applicability(cx, arg);
}
}
lint_name.map(|lint_name| (sym_to_string(lint_name).to_ascii_lowercase(), applicability))
}
fn extract_complex_emission_info<'hir>(
cx: &LateContext<'hir>,
args: &'hir [hir::Expr<'hir>],
) -> Option<(String, Option<String>, bool)> {
let mut lint_name = None;
let mut applicability = None;
let mut multi_span = false;
for arg in args {
let (arg_ty, _) = walk_ptrs_ty_depth(cx.typeck_results().expr_ty(&arg));
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);
}
} else if arg_ty.is_closure() {
if let ExprKind::Closure(_, _, body_id, _, _) = arg.kind {
let mut visitor = EmissionClosureVisitor::new(cx);
intravisit::walk_body(&mut visitor, cx.tcx.hir().body(body_id));
multi_span = visitor.found_multi_span();
applicability = visitor.complete();
} else {
// TODO xfrednet 2021-02-28: linked closures, see: needless_pass_by_value.rs:292
return None;
}
multi_part |= check_is_multi_part(cx, arg);
// TODO xFrednet 2021-03-01: don't use or_else but rather a comparison
applicability = applicability.or_else(|| resolve_applicability(cx, arg));
}
}
lint_name.map(|lint_name| (sym_to_string(lint_name).to_ascii_lowercase(), applicability, multi_span))
lint_name.map(|lint_name| (sym_to_string(lint_name).to_ascii_lowercase(), applicability, multi_part))
}
/// This function tries to resolve the linked applicability to the given expression.
fn resolve_applicability(cx: &LateContext<'hir>, expr: &'hir hir::Expr<'hir>) -> Option<String> {
match expr.kind {
// We can ignore ifs without an else block because those can't be used as an assignment
ExprKind::If(_con, if_block, Some(else_block)) => {
let mut visitor = ApplicabilityVisitor::new(cx);
intravisit::walk_expr(&mut visitor, if_block);
intravisit::walk_expr(&mut visitor, else_block);
visitor.complete()
},
ExprKind::Match(_expr, arms, _) => {
let mut visitor = ApplicabilityVisitor::new(cx);
arms.iter()
.for_each(|arm| intravisit::walk_expr(&mut visitor, arm.body));
visitor.complete()
},
ExprKind::Loop(block, ..) | ExprKind::Block(block, ..) => {
let mut visitor = ApplicabilityVisitor::new(cx);
intravisit::walk_block(&mut visitor, block);
visitor.complete()
},
ExprKind::Path(QPath::Resolved(_, path)) => {
// direct applicabilities are simple:
for enum_value in &paths::APPLICABILITY_VALUES {
if match_path(path, enum_value) {
return Some(enum_value[APPLICABILITY_NAME_INDEX].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);
}
}
}
// This is true for paths that are linked to function parameters. They might be a bit more work so
// not today :)
None
},
_ => None,
}
let mut resolver = ApplicabilityResolver::new(cx);
resolver.visit_expr(expr);
resolver.complete()
}
fn get_parent_local(cx: &LateContext<'hir>, hir_id: hir::HirId) -> Option<&'hir hir::Local<'hir>> {
let map = cx.tcx.hir();
match map.find(map.get_parent_node(hir_id)) {
Some(hir::Node::Local(local)) => Some(local),
Some(hir::Node::Pat(pattern)) => get_parent_local(cx, pattern.hir_id),
_ => None,
fn check_is_multi_part(cx: &LateContext<'hir>, closure_expr: &'hir hir::Expr<'hir>) -> bool {
if let ExprKind::Closure(_, _, body_id, _, _) = closure_expr.kind {
let mut scanner = IsMultiSpanScanner::new(cx);
intravisit::walk_body(&mut scanner, cx.tcx.hir().body(body_id));
return scanner.is_multi_part();
} else if let Some(local) = get_parent_local(cx, closure_expr) {
if let Some(local_init) = local.init {
return check_is_multi_part(cx, local_init);
}
}
false
}
/// This visitor finds the highest applicability value in the visited expressions
struct ApplicabilityVisitor<'a, 'hir> {
struct ApplicabilityResolver<'a, 'hir> {
cx: &'a LateContext<'hir>,
/// This is the index of hightest `Applicability` for
/// `clippy_utils::paths::APPLICABILITY_VALUES`
/// This is the index of hightest `Applicability` for `paths::APPLICABILITY_VALUES`
applicability_index: Option<usize>,
}
impl<'a, 'hir> ApplicabilityVisitor<'a, 'hir> {
impl<'a, 'hir> ApplicabilityResolver<'a, 'hir> {
fn new(cx: &'a LateContext<'hir>) -> Self {
Self {
cx,
@ -537,75 +454,104 @@ impl<'a, 'hir> ApplicabilityVisitor<'a, 'hir> {
}
}
impl<'a, 'hir> intravisit::Visitor<'hir> for ApplicabilityVisitor<'a, 'hir> {
impl<'a, 'hir> intravisit::Visitor<'hir> for ApplicabilityResolver<'a, 'hir> {
type Map = Map<'hir>;
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::All(self.cx.tcx.hir())
}
fn visit_path(&mut self, path: &hir::Path<'_>, _id: hir::HirId) {
fn visit_path(&mut self, path: &'hir hir::Path<'hir>, _id: hir::HirId) {
for (index, enum_value) in paths::APPLICABILITY_VALUES.iter().enumerate() {
if match_path(path, enum_value) {
self.add_new_index(index);
break;
}
}
}
}
/// This visitor finds the highest applicability value in the visited expressions
struct EmissionClosureVisitor<'a, 'hir> {
cx: &'a LateContext<'hir>,
/// This is the index of hightest `Applicability` for
/// `clippy_utils::paths::APPLICABILITY_VALUES`
applicability_index: Option<usize>,
suggestion_count: usize,
}
impl<'a, 'hir> EmissionClosureVisitor<'a, 'hir> {
fn new(cx: &'a LateContext<'hir>) -> Self {
Self {
cx,
applicability_index: None,
suggestion_count: 0,
}
}
fn add_new_index(&mut self, new_index: usize) {
self.applicability_index = self
.applicability_index
.map_or(new_index, |old_index| old_index.min(new_index))
.into();
}
fn found_multi_span(&self) -> bool {
self.suggestion_count > 1
}
fn complete(self) -> Option<String> {
self.applicability_index
.map(|index| paths::APPLICABILITY_VALUES[index][APPLICABILITY_NAME_INDEX].to_string())
}
}
impl<'a, 'hir> intravisit::Visitor<'hir> for EmissionClosureVisitor<'a, 'hir> {
type Map = Map<'hir>;
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::All(self.cx.tcx.hir())
}
fn visit_path(&mut self, path: &hir::Path<'_>, _id: hir::HirId) {
for (index, enum_value) in paths::APPLICABILITY_VALUES.iter().enumerate() {
if match_path(path, enum_value) {
self.add_new_index(index);
break;
return;
}
}
}
fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
let (expr_ty, _) = walk_ptrs_ty_depth(self.cx.typeck_results().expr_ty(&expr));
if_chain! {
if match_type(self.cx, expr_ty, &paths::APPLICABILITY);
if let Some(local) = get_parent_local(self.cx, expr);
if let Some(local_init) = local.init;
then {
intravisit::walk_expr(self, local_init);
}
};
// TODO xFrednet 2021-03-01: support function arguments?
intravisit::walk_expr(self, expr);
}
}
/// This returns the parent local node if the expression is a reference to
fn get_parent_local(cx: &LateContext<'hir>, expr: &'hir hir::Expr<'hir>) -> Option<&'hir hir::Local<'hir>> {
if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind {
if let hir::def::Res::Local(local_hir) = path.res {
return get_parent_local_hir_id(cx, local_hir);
}
}
None
}
fn get_parent_local_hir_id(cx: &LateContext<'hir>, hir_id: hir::HirId) -> Option<&'hir hir::Local<'hir>> {
let map = cx.tcx.hir();
match map.find(map.get_parent_node(hir_id)) {
Some(hir::Node::Local(local)) => Some(local),
Some(hir::Node::Pat(pattern)) => get_parent_local_hir_id(cx, pattern.hir_id),
_ => None,
}
}
/// This visitor finds the highest applicability value in the visited expressions
struct IsMultiSpanScanner<'a, 'hir> {
cx: &'a LateContext<'hir>,
suggestion_count: usize,
}
impl<'a, 'hir> IsMultiSpanScanner<'a, 'hir> {
fn new(cx: &'a LateContext<'hir>) -> Self {
Self {
cx,
suggestion_count: 0,
}
}
/// Add a new single expression suggestion to the counter
fn add_singe_span_suggestion(&mut self) {
self.suggestion_count += 1;
}
/// Signals that a suggestion with possible multiple spans was found
fn add_multi_part_suggestion(&mut self) {
self.suggestion_count += 2;
}
/// Checks if the suggestions include multiple spanns
fn is_multi_part(&self) -> bool {
self.suggestion_count > 1
}
}
impl<'a, 'hir> intravisit::Visitor<'hir> for IsMultiSpanScanner<'a, 'hir> {
type Map = Map<'hir>;
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
intravisit::NestedVisitorMap::All(self.cx.tcx.hir())
}
fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
// Early return if the lint is already multi span
if self.is_multi_part() {
return;
}
match &expr.kind {
ExprKind::Call(fn_expr, _args) => {
let found_function = SUGGESTION_FUNCTIONS
@ -613,29 +559,21 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for EmissionClosureVisitor<'a, 'hir> {
.any(|func_path| match_function_call(self.cx, fn_expr, func_path).is_some());
if found_function {
// These functions are all multi part suggestions
self.suggestion_count += 2;
self.add_singe_span_suggestion()
}
},
ExprKind::MethodCall(path, _path_span, arg, _arg_span) => {
let (self_ty, _) = walk_ptrs_ty_depth(self.cx.typeck_results().expr_ty(&arg[0]));
if match_type(self.cx, self_ty, &paths::DIAGNOSTIC_BUILDER) {
let called_method = path.ident.name.as_str().to_string();
let found_suggestion =
SUGGESTION_DIAGNOSTIC_BUILDER_METHODS
.iter()
.find_map(|(method_name, is_multi_part)| {
if *method_name == called_method {
Some(*is_multi_part)
} else {
None
}
});
if let Some(multi_part) = found_suggestion {
if multi_part {
// two is enough to have it marked as a multipart suggestion
self.suggestion_count += 2;
} else {
self.suggestion_count += 1;
for (method_name, is_multi_part) in &SUGGESTION_DIAGNOSTIC_BUILDER_METHODS {
if *method_name == called_method {
if *is_multi_part {
self.add_multi_part_suggestion();
} else {
self.add_singe_span_suggestion();
}
break;
}
}
}

5698
metadata_collection.json Normal file

File diff suppressed because it is too large Load Diff