Skip to content
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

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/librustc/hir/def_id.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::ty;
use crate::ty::TyCtxt;
use crate::ty::{self, TyCtxt};
use crate::hir::map::definitions::FIRST_FREE_HIGH_DEF_INDEX;
use rustc_data_structures::indexed_vec::Idx;
use serialize;
use std::fmt;
use std::u32;
use syntax::symbol;

newtype_index! {
pub struct CrateId {
Expand Down Expand Up @@ -252,6 +252,33 @@ impl DefId {
format!("module `{}`", tcx.item_path_str(*self))
}
}

/// Check if a `DefId`'s path matches the given absolute type path usage.
// Uplifted from rust-lang/rust-clippy
pub fn match_path(self, tcx: TyCtxt<'_, '_, '_>, path: &[&str]) -> bool {
#[derive(Debug)]
struct AbsolutePathBuffer {
names: Vec<symbol::LocalInternedString>,
}

impl ty::item_path::ItemPathBuffer for AbsolutePathBuffer {
fn root_mode(&self) -> &ty::item_path::RootMode {
const ABSOLUTE: &ty::item_path::RootMode = &ty::item_path::RootMode::Absolute;
ABSOLUTE
}

fn push(&mut self, text: &str) {
self.names.push(symbol::Symbol::intern(text).as_str());
}
}

let mut apb = AbsolutePathBuffer { names: vec![] };

tcx.push_item_path(&mut apb, self, false);

apb.names.len() == path.len()
&& apb.names.into_iter().zip(path.iter()).all(|(a, &b)| *a == *b)
}
}

impl serialize::UseSpecializedEncodable for DefId {}
Expand Down
142 changes: 142 additions & 0 deletions src/librustc/lint/internal.rs
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 {
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
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();
}
}
}
}
}
}
}
}
1 change: 1 addition & 0 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ impl_stable_hash_for!(enum self::LintSource {
pub type LevelSource = (Level, LintSource);

pub mod builtin;
pub mod internal;
mod context;
mod levels;

Expand Down
3 changes: 3 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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 {
Expand Down
9 changes: 9 additions & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,9 @@ fn run_compiler_with_pool<'a>(
let codegen_backend = get_codegen_backend(&sess);

rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}

let mut cfg = config::build_configuration(&sess, cfg);
target_features::add_configuration(&mut cfg, &sess, &*codegen_backend);
Expand Down Expand Up @@ -815,10 +818,16 @@ impl<'a> CompilerCalls<'a> for RustcDefaultCalls {
if sopts.describe_lints {
let mut ls = lint::LintStore::new();
rustc_lint::register_builtins(&mut ls, Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut ls, Some(&sess));
}
describe_lints(&sess, &ls, false);
return None;
}
rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}
let mut cfg = config::build_configuration(&sess, cfg.clone());
let codegen_backend = get_codegen_backend(&sess);
target_features::add_configuration(&mut cfg, &sess, &*codegen_backend);
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_driver/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ fn test_env_with_pool<F>(
);
let cstore = CStore::new(::get_codegen_backend(&sess).metadata_loader());
rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}
let input = config::Input::Str {
name: FileName::anon_source_code(&source_string),
input: source_string.to_string(),
Expand Down
16 changes: 16 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ use nonstandard_style::*;
use builtin::*;
use types::*;
use unused::*;
use rustc::lint::internal::*;

/// Useful for other parts of the compiler.
pub use builtin::SoftLints;
Expand Down Expand Up @@ -405,3 +406,18 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
store.register_removed("bad_repr",
"replaced with a generic attribute input check");
}

pub fn register_internals(store: &mut lint::LintStore, sess: Option<&Session>) {
store.register_early_pass(sess, false, false, box DefaultHashTypes::new());
store.register_late_pass(sess, false, box TyKindUsage);
store.register_group(
sess,
false,
"internal",
None,
vec![
LintId::of(DEFAULT_HASH_TYPES),
LintId::of(USAGE_OF_TY_TYKIND),
],
);
}
3 changes: 3 additions & 0 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
let codegen_backend = rustc_driver::get_codegen_backend(&sess);
let cstore = Rc::new(CStore::new(codegen_backend.metadata_loader()));
rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}

let mut cfg = config::build_configuration(&sess, config::parse_cfgspecs(cfgs));
target_features::add_configuration(&mut cfg, &sess, &*codegen_backend);
Expand Down
6 changes: 6 additions & 0 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ pub fn run(mut options: Options) -> isize {
let codegen_backend = rustc_driver::get_codegen_backend(&sess);
let cstore = CStore::new(codegen_backend.metadata_loader());
rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}

let mut cfg = config::build_configuration(&sess,
config::parse_cfgspecs(options.cfgs.clone()));
Expand Down Expand Up @@ -279,6 +282,9 @@ fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
let codegen_backend = rustc_driver::get_codegen_backend(&sess);
let cstore = CStore::new(codegen_backend.metadata_loader());
rustc_lint::register_builtins(&mut sess.lint_store.borrow_mut(), Some(&sess));
if sess.opts.debugging_opts.internal_lints {
rustc_lint::register_internals(&mut sess.lint_store.borrow_mut(), Some(&sess));
}

let outdir = Mutex::new(
if let Some(mut path) = persist_doctests {
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc-ui/failed-doctest-output.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ error[E0425]: cannot find value `no` in this scope
3 | no
| ^^ not found in this scope

thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:351:13
thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:357:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

---- $DIR/failed-doctest-output.rs - SomeStruct (line 11) stdout ----
Expand All @@ -21,7 +21,7 @@ thread '$DIR/failed-doctest-output.rs - SomeStruct (line 11)' panicked at 'test
thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:3:1
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

', src/librustdoc/test.rs:372:17
', src/librustdoc/test.rs:378:17


failures:
Expand Down
24 changes: 24 additions & 0 deletions src/test/ui-fulldeps/internal-lints/default_hash_types.rs
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();
}
Loading