-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[WIP] Internal lints #58701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Internal lints #58701
Changes from all commits
0a9baa6
121a5f7
e7fcbc7
eeb278a
bac39a4
1c0a67c
c7b4070
a06ca43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
//! Some lints that are only useful in the compiler or crates that use compiler internals, such as | ||
//! Clippy. | ||
|
||
use crate::hir::{HirId, Path, QPath, Ty, TyKind}; | ||
use crate::lint::{ | ||
EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray, LintContext, LintPass, | ||
}; | ||
use errors::Applicability; | ||
use rustc_data_structures::fx::FxHashMap; | ||
use syntax::ast::Ident; | ||
|
||
declare_lint! { | ||
pub DEFAULT_HASH_TYPES, | ||
Warn, | ||
"forbid HashMap and HashSet and suggest the FxHash* variants" | ||
} | ||
|
||
pub struct DefaultHashTypes { | ||
map: FxHashMap<String, String>, | ||
} | ||
|
||
impl DefaultHashTypes { | ||
pub fn new() -> Self { | ||
let mut map = FxHashMap::default(); | ||
map.insert("HashMap".to_string(), "FxHashMap".to_string()); | ||
map.insert("HashSet".to_string(), "FxHashSet".to_string()); | ||
Self { map } | ||
} | ||
} | ||
|
||
impl LintPass for DefaultHashTypes { | ||
fn get_lints(&self) -> LintArray { | ||
lint_array!(DEFAULT_HASH_TYPES) | ||
} | ||
|
||
fn name(&self) -> &'static str { | ||
"DefaultHashTypes" | ||
} | ||
} | ||
|
||
impl EarlyLintPass for DefaultHashTypes { | ||
fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { | ||
let ident_string = ident.to_string(); | ||
if let Some(replace) = self.map.get(&ident_string) { | ||
let msg = format!( | ||
"Prefer {} over {}, it has better performance", | ||
replace, ident_string | ||
); | ||
let mut db = cx.struct_span_lint(DEFAULT_HASH_TYPES, ident.span, &msg); | ||
db.span_suggestion( | ||
ident.span, | ||
"use", | ||
replace.to_string(), | ||
Applicability::MaybeIncorrect, // FxHashMap, ... needs another import | ||
); | ||
db.note(&format!( | ||
"a `use rustc_data_structures::fx::{}` may be necessary", | ||
replace | ||
)) | ||
.emit(); | ||
} | ||
} | ||
} | ||
|
||
declare_lint! { | ||
pub USAGE_OF_TY_TYKIND, | ||
Warn, | ||
"Usage of `ty::TyKind` outside of the `ty::sty` module" | ||
} | ||
|
||
pub struct TyKindUsage; | ||
|
||
impl LintPass for TyKindUsage { | ||
fn get_lints(&self) -> LintArray { | ||
lint_array!(USAGE_OF_TY_TYKIND) | ||
} | ||
|
||
fn name(&self) -> &'static str { | ||
"TyKindUsage" | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TyKindUsage { | ||
fn check_path(&mut self, cx: &LateContext<'_, '_>, path: &'tcx Path, _: HirId) { | ||
let segments_iter = path.segments.iter().rev().skip(1).rev(); | ||
|
||
if let Some(last) = segments_iter.clone().last() { | ||
if last.ident.as_str() == "TyKind" { | ||
let path = Path { | ||
span: path.span.with_hi(last.ident.span.hi()), | ||
def: path.def, | ||
segments: segments_iter.cloned().collect(), | ||
}; | ||
|
||
if let Some(def) = last.def { | ||
if def | ||
.def_id() | ||
.match_path(cx.tcx, &["rustc", "ty", "sty", "TyKind"]) | ||
{ | ||
cx.struct_span_lint( | ||
USAGE_OF_TY_TYKIND, | ||
path.span, | ||
"usage of `ty::TyKind::<kind>`", | ||
) | ||
.span_suggestion( | ||
path.span, | ||
"try using ty::<kind> directly", | ||
"ty".to_string(), | ||
Applicability::MaybeIncorrect, // ty maybe needs an import | ||
) | ||
.emit(); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &'tcx Ty) { | ||
if let TyKind::Path(qpath) = &ty.node { | ||
if let QPath::Resolved(_, path) = qpath { | ||
if let Some(last) = path.segments.iter().last() { | ||
if last.ident.as_str() == "TyKind" { | ||
if let Some(def) = last.def { | ||
if def | ||
.def_id() | ||
.match_path(cx.tcx, &["rustc", "ty", "sty", "TyKind"]) | ||
{ | ||
cx.struct_span_lint( | ||
USAGE_OF_TY_TYKIND, | ||
path.span, | ||
"usage of `ty::TyKind`", | ||
) | ||
.help("try using `ty::Ty` instead") | ||
.emit(); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1410,6 +1410,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, | |
merge_functions: Option<MergeFunctions> = (None, parse_merge_functions, [TRACKED], | ||
"control the operation of the MergeFunctions LLVM pass, taking | ||
the same values as the target option of the same name"), | ||
internal_lints: bool = (false, parse_bool, [UNTRACKED], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of adding a new flag we could just reuse the -Zunstable-features flag. If the lints are allow by default, we can add deny attributes to crates in steps, and even with cfg_attr to only turn them on after stage0. That will get us around having to touch bootstrap There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think that is way easier. I'll try that ASAP. |
||
"allow internal rustc lints. These lints are probably only useful in the | ||
compiler directly or in crates, that use rustc internals, such as Clippy."), | ||
} | ||
|
||
pub fn default_lib_output() -> CrateType { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// compile-flags: -Z internal-lints | ||
|
||
#![feature(rustc_private)] | ||
|
||
extern crate rustc_data_structures; | ||
|
||
use rustc_data_structures::fx::{FxHashMap, FxHashSet}; | ||
use std::collections::{HashMap, HashSet}; | ||
//~^ WARNING Prefer FxHashMap over HashMap, it has better performance | ||
//~^^ WARNING Prefer FxHashSet over HashSet, it has better performance | ||
|
||
#[deny(default_hash_types)] | ||
fn main() { | ||
let _map: HashMap<String, String> = HashMap::default(); | ||
//~^ ERROR Prefer FxHashMap over HashMap, it has better performance | ||
//~^^ ERROR Prefer FxHashMap over HashMap, it has better performance | ||
let _set: HashSet<String> = HashSet::default(); | ||
//~^ ERROR Prefer FxHashSet over HashSet, it has better performance | ||
//~^^ ERROR Prefer FxHashSet over HashSet, it has better performance | ||
|
||
// test that the lint doesn't also match the Fx variants themselves | ||
let _fx_map: FxHashMap<String, String> = FxHashMap::default(); | ||
let _fx_set: FxHashSet<String> = FxHashSet::default(); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.