Skip to content

Use interned symbols instead of strings in more places #14855

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
8 changes: 4 additions & 4 deletions clippy_lints/src/arbitrary_source_item_ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl ArbitrarySourceItemOrdering {
self.assoc_types_order
),
Some(before_item.span),
format!("should be placed before `{}`", before_item.ident.as_str(),),
format!("should be placed before `{}`", before_item.ident.name),
);
}

Expand All @@ -216,7 +216,7 @@ impl ArbitrarySourceItemOrdering {
ident.span,
"incorrect ordering of items (must be alphabetically ordered)",
Some(before_ident.span),
format!("should be placed before `{}`", before_ident.as_str(),),
format!("should be placed before `{}`", before_ident.name),
);
}

Expand All @@ -228,7 +228,7 @@ impl ArbitrarySourceItemOrdering {
};

let (before_span, note) = if let Some(ident) = before_item.kind.ident() {
(ident.span, format!("should be placed before `{}`", ident.as_str(),))
(ident.span, format!("should be placed before `{}`", ident.name))
} else {
(
before_item.span,
Expand All @@ -255,7 +255,7 @@ impl ArbitrarySourceItemOrdering {
self.assoc_types_order
),
Some(before_item.span),
format!("should be placed before `{}`", before_item.ident.as_str(),),
format!("should be placed before `{}`", before_item.ident.name),
);
}
}
Expand Down
21 changes: 13 additions & 8 deletions clippy_lints/src/attrs/duplicated_attributes.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
use super::DUPLICATED_ATTRIBUTES;
use clippy_utils::diagnostics::span_lint_and_then;
use itertools::Itertools;
use rustc_ast::{Attribute, MetaItem};
use rustc_data_structures::fx::FxHashMap;
use rustc_lint::EarlyContext;
use rustc_span::{Span, sym};
use rustc_span::{Span, Symbol, sym};
use std::collections::hash_map::Entry;

fn emit_if_duplicated(
cx: &EarlyContext<'_>,
attr: &MetaItem,
attr_paths: &mut FxHashMap<String, Span>,
complete_path: String,
parent: &[Symbol],
complete_path: impl FnOnce(&str) -> String,
) {
match attr_paths.entry(complete_path) {
let parent = parent.iter().join(":");
match attr_paths.entry(complete_path(&parent)) {
Entry::Vacant(v) => {
v.insert(attr.span);
},
Expand All @@ -29,7 +32,7 @@ fn check_duplicated_attr(
cx: &EarlyContext<'_>,
attr: &MetaItem,
attr_paths: &mut FxHashMap<String, Span>,
parent: &mut Vec<String>,
parent: &mut Vec<Symbol>,
) {
if attr.span.from_expansion() {
return;
Expand All @@ -43,25 +46,27 @@ fn check_duplicated_attr(
return;
}
if let Some(direct_parent) = parent.last()
&& direct_parent == sym::cfg_trace.as_str()
&& *direct_parent == sym::cfg_trace
&& [sym::all, sym::not, sym::any].contains(&name)
{
// FIXME: We don't correctly check `cfg`s for now, so if it's more complex than just a one
// level `cfg`, we leave.
return;
}
if let Some(value) = attr.value_str() {
emit_if_duplicated(cx, attr, attr_paths, format!("{}:{name}={value}", parent.join(":")));
emit_if_duplicated(cx, attr, attr_paths, parent, |parent| {
format!("{parent}:{name}={value}")
});
} else if let Some(sub_attrs) = attr.meta_item_list() {
parent.push(name.as_str().to_string());
parent.push(name);
for sub_attr in sub_attrs {
if let Some(meta) = sub_attr.meta_item() {
check_duplicated_attr(cx, meta, attr_paths, parent);
}
}
parent.pop();
} else {
emit_if_duplicated(cx, attr, attr_paths, format!("{}:{name}", parent.join(":")));
emit_if_duplicated(cx, attr, attr_paths, parent, |parent| format!("{parent}:{name}"));
}
}

Expand Down
16 changes: 7 additions & 9 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::eq_expr_value;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::SpanRangeExt;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{eq_expr_value, sym};
use rustc_ast::ast::LitKind;
use rustc_attr_parsing::RustcVersion;
use rustc_errors::Applicability;
Expand All @@ -13,7 +13,7 @@ use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp};
use rustc_lint::{LateContext, LateLintPass, Level};
use rustc_session::impl_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, SyntaxContext, sym};
use rustc_span::{Span, Symbol, SyntaxContext};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -73,10 +73,10 @@ declare_clippy_lint! {
}

// For each pairs, both orders are considered.
const METHODS_WITH_NEGATION: [(Option<RustcVersion>, &str, &str); 3] = [
(None, "is_some", "is_none"),
(None, "is_err", "is_ok"),
(Some(msrvs::IS_NONE_OR), "is_some_and", "is_none_or"),
const METHODS_WITH_NEGATION: [(Option<RustcVersion>, Symbol, Symbol); 3] = [
(None, sym::is_some, sym::is_none),
(None, sym::is_err, sym::is_ok),
(Some(msrvs::IS_NONE_OR), sym::is_some_and, sym::is_none_or),
];

pub struct NonminimalBool {
Expand Down Expand Up @@ -440,9 +440,7 @@ fn simplify_not(cx: &LateContext<'_>, curr_msrv: Msrv, expr: &Expr<'_>) -> Optio
.iter()
.copied()
.flat_map(|(msrv, a, b)| vec![(msrv, a, b), (msrv, b, a)])
.find(|&(msrv, a, _)| {
a == path.ident.name.as_str() && msrv.is_none_or(|msrv| curr_msrv.meets(cx, msrv))
})
.find(|&(msrv, a, _)| a == path.ident.name && msrv.is_none_or(|msrv| curr_msrv.meets(cx, msrv)))
.and_then(|(_, _, neg_method)| {
let negated_args = args
.iter()
Expand Down
60 changes: 35 additions & 25 deletions clippy_lints/src/checked_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{SpanlessEq, is_in_const_context, is_integer_literal};
use clippy_utils::{SpanlessEq, is_in_const_context, is_integer_literal, sym};
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, TyKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::impl_lint_pass;
use rustc_span::Symbol;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -98,7 +99,7 @@ impl LateLintPass<'_> for CheckedConversions {
struct Conversion<'a> {
cvt: ConversionType,
expr_to_cast: &'a Expr<'a>,
to_type: Option<&'a str>,
to_type: Option<Symbol>,
}

/// The kind of conversion that is checked
Expand Down Expand Up @@ -150,7 +151,7 @@ impl<'a> Conversion<'a> {
}

/// Try to construct a new conversion if the conversion type is valid
fn try_new(expr_to_cast: &'a Expr<'_>, from_type: &str, to_type: &'a str) -> Option<Conversion<'a>> {
fn try_new(expr_to_cast: &'a Expr<'_>, from_type: Symbol, to_type: Symbol) -> Option<Conversion<'a>> {
ConversionType::try_new(from_type, to_type).map(|cvt| Conversion {
cvt,
expr_to_cast,
Expand All @@ -171,7 +172,7 @@ impl<'a> Conversion<'a> {
impl ConversionType {
/// Creates a conversion type if the type is allowed & conversion is valid
#[must_use]
fn try_new(from: &str, to: &str) -> Option<Self> {
fn try_new(from: Symbol, to: Symbol) -> Option<Self> {
if UINTS.contains(&from) {
Some(Self::FromUnsigned)
} else if SINTS.contains(&from) {
Expand All @@ -190,7 +191,7 @@ impl ConversionType {

/// Check for `expr <= (to_type::MAX as from_type)`
fn check_upper_bound<'tcx>(lt: &'tcx Expr<'tcx>, gt: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> {
if let Some((from, to)) = get_types_from_cast(gt, INTS, "max_value", "MAX") {
if let Some((from, to)) = get_types_from_cast(gt, INTS, sym::max_value, sym::MAX) {
Conversion::try_new(lt, from, to)
} else {
None
Expand All @@ -209,23 +210,23 @@ fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> O

/// Check for `expr >= (to_type::MIN as from_type)`
fn check_lower_bound_min<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option<Conversion<'a>> {
if let Some((from, to)) = get_types_from_cast(check, SINTS, "min_value", "MIN") {
if let Some((from, to)) = get_types_from_cast(check, SINTS, sym::min_value, sym::MIN) {
Conversion::try_new(candidate, from, to)
} else {
None
}
}

/// Tries to extract the from- and to-type from a cast expression
fn get_types_from_cast<'a>(
expr: &'a Expr<'_>,
types: &'a [&str],
func: &'a str,
assoc_const: &'a str,
) -> Option<(&'a str, &'a str)> {
fn get_types_from_cast(
expr: &Expr<'_>,
types: &[Symbol],
func: Symbol,
assoc_const: Symbol,
) -> Option<(Symbol, Symbol)> {
// `to_type::max_value() as from_type`
// or `to_type::MAX as from_type`
let call_from_cast: Option<(&Expr<'_>, &str)> = if let ExprKind::Cast(limit, from_type) = &expr.kind
let call_from_cast: Option<(&Expr<'_>, Symbol)> = if let ExprKind::Cast(limit, from_type) = &expr.kind
// to_type::max_value(), from_type
&& let TyKind::Path(from_type_path) = &from_type.kind
&& let Some(from_sym) = int_ty_to_sym(from_type_path)
Expand All @@ -236,12 +237,12 @@ fn get_types_from_cast<'a>(
};

// `from_type::from(to_type::max_value())`
let limit_from: Option<(&Expr<'_>, &str)> = call_from_cast.or_else(|| {
let limit_from: Option<(&Expr<'_>, Symbol)> = call_from_cast.or_else(|| {
if let ExprKind::Call(from_func, [limit]) = &expr.kind
// `from_type::from, to_type::max_value()`
// `from_type::from`
&& let ExprKind::Path(path) = &from_func.kind
&& let Some(from_sym) = get_implementing_type(path, INTS, "from")
&& let Some(from_sym) = get_implementing_type(path, INTS, sym::from)
{
Some((limit, from_sym))
} else {
Expand Down Expand Up @@ -273,32 +274,41 @@ fn get_types_from_cast<'a>(
}

/// Gets the type which implements the called function
fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function: &str) -> Option<&'a str> {
fn get_implementing_type(path: &QPath<'_>, candidates: &[Symbol], function: Symbol) -> Option<Symbol> {
if let QPath::TypeRelative(ty, path) = &path
&& path.ident.name.as_str() == function
&& path.ident.name == function
&& let TyKind::Path(QPath::Resolved(None, tp)) = &ty.kind
&& let [int] = tp.segments
{
let name = int.ident.name.as_str();
candidates.iter().find(|c| &name == *c).copied()
candidates.iter().find(|c| int.ident.name == **c).copied()
} else {
None
}
}

/// Gets the type as a string, if it is a supported integer
fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> {
fn int_ty_to_sym(path: &QPath<'_>) -> Option<Symbol> {
if let QPath::Resolved(_, path) = *path
&& let [ty] = path.segments
{
let name = ty.ident.name.as_str();
INTS.iter().find(|c| &name == *c).copied()
INTS.iter().find(|c| ty.ident.name == **c).copied()
} else {
None
}
}

// Constants
const UINTS: &[&str] = &["u8", "u16", "u32", "u64", "usize"];
const SINTS: &[&str] = &["i8", "i16", "i32", "i64", "isize"];
const INTS: &[&str] = &["u8", "u16", "u32", "u64", "usize", "i8", "i16", "i32", "i64", "isize"];
const UINTS: &[Symbol] = &[sym::u8, sym::u16, sym::u32, sym::u64, sym::usize];
const SINTS: &[Symbol] = &[sym::i8, sym::i16, sym::i32, sym::i64, sym::isize];
const INTS: &[Symbol] = &[
sym::u8,
sym::u16,
sym::u32,
sym::u64,
sym::usize,
sym::i8,
sym::i16,
sym::i32,
sym::i64,
sym::isize,
];
12 changes: 6 additions & 6 deletions clippy_lints/src/endian_bytes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::Lint;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_lint_allowed;
use clippy_utils::{is_lint_allowed, sym};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::Ty;
Expand Down Expand Up @@ -65,9 +65,9 @@ declare_clippy_lint! {

declare_lint_pass!(EndianBytes => [HOST_ENDIAN_BYTES, LITTLE_ENDIAN_BYTES, BIG_ENDIAN_BYTES]);

const HOST_NAMES: [&str; 2] = ["from_ne_bytes", "to_ne_bytes"];
const LITTLE_NAMES: [&str; 2] = ["from_le_bytes", "to_le_bytes"];
const BIG_NAMES: [&str; 2] = ["from_be_bytes", "to_be_bytes"];
const HOST_NAMES: [Symbol; 2] = [sym::from_ne_bytes, sym::to_ne_bytes];
const LITTLE_NAMES: [Symbol; 2] = [sym::from_le_bytes, sym::to_le_bytes];
const BIG_NAMES: [Symbol; 2] = [sym::from_be_bytes, sym::to_be_bytes];

#[derive(Clone, Debug)]
enum LintKind {
Expand Down Expand Up @@ -95,7 +95,7 @@ impl LintKind {
}
}

fn as_name(&self, prefix: Prefix) -> &str {
fn as_name(&self, prefix: Prefix) -> Symbol {
let index = usize::from(prefix == Prefix::To);

match self {
Expand Down Expand Up @@ -133,7 +133,7 @@ fn maybe_lint_endian_bytes(cx: &LateContext<'_>, expr: &Expr<'_>, prefix: Prefix
let le = LintKind::Little.as_name(prefix);
let be = LintKind::Big.as_name(prefix);

let (lint, other_lints) = match name.as_str() {
let (lint, other_lints) = match name {
name if name == ne => ((&LintKind::Host), [(&LintKind::Little), (&LintKind::Big)]),
name if name == le => ((&LintKind::Little), [(&LintKind::Host), (&LintKind::Big)]),
name if name == be => ((&LintKind::Big), [(&LintKind::Host), (&LintKind::Little)]),
Expand Down
8 changes: 8 additions & 0 deletions clippy_utils/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ generate! {
floor_char_boundary,
fold,
for_each,
from_be_bytes,
from_bytes_with_nul,
from_bytes_with_nul_unchecked,
from_le_bytes,
from_ne_bytes,
from_ptr,
from_raw,
from_ref,
Expand Down Expand Up @@ -184,8 +187,10 @@ generate! {
is_err,
is_file,
is_none,
is_none_or,
is_ok,
is_some,
is_some_and,
isqrt,
itertools,
join,
Expand Down Expand Up @@ -323,8 +328,11 @@ generate! {
then_some,
to_ascii_lowercase,
to_ascii_uppercase,
to_be_bytes,
to_digit,
to_le_bytes,
to_lowercase,
to_ne_bytes,
to_os_string,
to_owned,
to_path_buf,
Expand Down