Skip to content

Commit

Permalink
New internal lint: interning_defined_symbol
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Wright committed Dec 13, 2020
1 parent 89c282f commit 3af09b8
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 0 deletions.
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -513,6 +513,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
#[cfg(feature = "internal-lints")]
&utils::internal_lints::INVALID_PATHS,
#[cfg(feature = "internal-lints")]
&utils::internal_lints::INTERNING_DEFINED_SYMBOL,
#[cfg(feature = "internal-lints")]
&utils::internal_lints::LINT_WITHOUT_LINT_PASS,
#[cfg(feature = "internal-lints")]
&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
Expand Down Expand Up @@ -958,6 +960,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
store.register_late_pass(|| box utils::internal_lints::InterningDefinedSymbol::default());
store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass);
Expand Down Expand Up @@ -1350,6 +1353,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
LintId::of(&utils::internal_lints::DEFAULT_LINT),
LintId::of(&utils::internal_lints::INVALID_PATHS),
LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL),
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),
LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM),
LintId::of(&utils::internal_lints::OUTER_EXPN_EXPN_DATA),
Expand Down
80 changes: 80 additions & 0 deletions clippy_lints/src/utils/internal_lints.rs
Expand Up @@ -15,6 +15,7 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind};
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::mir::interpret::ConstValue;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::{Span, Spanned};
Expand Down Expand Up @@ -247,6 +248,30 @@ declare_clippy_lint! {
"invalid path"
}

declare_clippy_lint! {
/// **What it does:**
/// Checks for interning symbols that have already been pre-interned and defined as constants.
///
/// **Why is this bad?**
/// It's faster and easier to use the symbol constant.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust,ignore
/// let _ = sym!(f32);
/// ```
///
/// Good:
/// ```rust,ignore
/// let _ = sym::f32;
/// ```
pub INTERNING_DEFINED_SYMBOL,
internal,
"interning a symbol that is pre-interned and defined as a constant"
}

declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]);

impl EarlyLintPass for ClippyLintsInternal {
Expand Down Expand Up @@ -840,3 +865,58 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths {
}
}
}

#[derive(Default)]
pub struct InterningDefinedSymbol {
// Maps the symbol to the constant name.
symbol_map: FxHashMap<String, String>,
}

impl_lint_pass!(InterningDefinedSymbol => [INTERNING_DEFINED_SYMBOL]);

impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol {
fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
if !self.symbol_map.is_empty() {
return;
}

if let Some(Res::Def(_, def_id)) = path_to_res(cx, &paths::SYM_MODULE) {
for item in cx.tcx.item_children(def_id).iter() {
if_chain! {
if let Res::Def(DefKind::Const, item_def_id) = item.res;
let ty = cx.tcx.type_of(item_def_id);
if match_type(cx, ty, &paths::SYMBOL);
if let Ok(ConstValue::Scalar(value)) = cx.tcx.const_eval_poly(item_def_id);
if let Ok(value) = value.to_u32();
then {
// SAFETY: We're converting the raw bytes of the symbol value back
// into a Symbol instance.
let symbol = unsafe { std::mem::transmute::<u32, Symbol>(value) };
self.symbol_map.insert(symbol.to_string(), item.ident.to_string());
}
}
}
}
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Call(func, [arg]) = &expr.kind;
if let ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(func).kind();
if match_def_path(cx, *def_id, &paths::SYMBOL_INTERN);
if let Some(Constant::Str(arg)) = constant_simple(cx, cx.typeck_results(), arg);
if let Some(symbol_const) = self.symbol_map.get(&arg);
then {
span_lint_and_sugg(
cx,
INTERNING_DEFINED_SYMBOL,
is_expn_of(expr.span, "sym").unwrap_or(expr.span),
"interning a defined symbol",
"try",
format!("rustc_span::symbol::sym::{}", symbol_const),
Applicability::MachineApplicable,
);
}
}
}
}
6 changes: 6 additions & 0 deletions clippy_lints/src/utils/paths.rs
Expand Up @@ -146,6 +146,12 @@ pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"];
#[cfg(feature = "internal-lints")]
pub const SYMBOL: [&str; 3] = ["rustc_span", "symbol", "Symbol"];
#[cfg(feature = "internal-lints")]
pub const SYMBOL_INTERN: [&str; 4] = ["rustc_span", "symbol", "Symbol", "intern"];
#[cfg(feature = "internal-lints")]
pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"];
#[cfg(feature = "internal-lints")]
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];
Expand Down
33 changes: 33 additions & 0 deletions tests/ui-internal/interning_defined_symbol.fixed
@@ -0,0 +1,33 @@
// run-rustfix
#![deny(clippy::internal)]
#![feature(rustc_private)]

extern crate rustc_span;

use rustc_span::symbol::Symbol;

macro_rules! sym {
($tt:tt) => {
rustc_span::symbol::Symbol::intern(stringify!($tt))
};
}

fn main() {
// Direct use of Symbol::intern
let _ = rustc_span::symbol::sym::f32;

// Using a sym macro
let _ = rustc_span::symbol::sym::f32;

// Correct suggestion when symbol isn't stringified constant name
let _ = rustc_span::symbol::sym::proc_dash_macro;

// Interning a symbol that is not defined
let _ = Symbol::intern("xyz123");
let _ = sym!(xyz123);

// Using a different `intern` function
let _ = intern("f32");
}

fn intern(_: &str) {}
33 changes: 33 additions & 0 deletions tests/ui-internal/interning_defined_symbol.rs
@@ -0,0 +1,33 @@
// run-rustfix
#![deny(clippy::internal)]
#![feature(rustc_private)]

extern crate rustc_span;

use rustc_span::symbol::Symbol;

macro_rules! sym {
($tt:tt) => {
rustc_span::symbol::Symbol::intern(stringify!($tt))
};
}

fn main() {
// Direct use of Symbol::intern
let _ = Symbol::intern("f32");

// Using a sym macro
let _ = sym!(f32);

// Correct suggestion when symbol isn't stringified constant name
let _ = Symbol::intern("proc-macro");

// Interning a symbol that is not defined
let _ = Symbol::intern("xyz123");
let _ = sym!(xyz123);

// Using a different `intern` function
let _ = intern("f32");
}

fn intern(_: &str) {}
27 changes: 27 additions & 0 deletions tests/ui-internal/interning_defined_symbol.stderr
@@ -0,0 +1,27 @@
error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:17:13
|
LL | let _ = Symbol::intern("f32");
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`
|
note: the lint level is defined here
--> $DIR/interning_defined_symbol.rs:2:9
|
LL | #![deny(clippy::internal)]
| ^^^^^^^^^^^^^^^^
= note: `#[deny(clippy::interning_defined_symbol)]` implied by `#[deny(clippy::internal)]`

error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:20:13
|
LL | let _ = sym!(f32);
| ^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32`

error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:23:13
|
LL | let _ = Symbol::intern("proc-macro");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::proc_dash_macro`

error: aborting due to 3 previous errors

0 comments on commit 3af09b8

Please sign in to comment.