perf(linter): reduce String allocations and clones (#4673)

This commit is contained in:
DonIsaac 2024-08-06 01:00:57 +00:00
parent f986ec31a4
commit e3abdfa379
15 changed files with 103 additions and 87 deletions

View file

@ -1,6 +1,7 @@
use rustc_hash::FxHashMap;
use schemars::JsonSchema;
use serde::Deserialize;
use std::{borrow::Borrow, hash::Hash};
/// Predefine global variables.
// TODO: list the keys we support
@ -9,15 +10,25 @@ use serde::Deserialize;
pub struct OxlintEnv(FxHashMap<String, bool>);
impl OxlintEnv {
pub fn from_vec(env: Vec<String>) -> Self {
let map = env.into_iter().map(|key| (key, true)).collect();
Self(map)
pub fn contains<Q>(&self, key: &Q) -> bool
where
String: Borrow<Q>,
Q: ?Sized + Hash + Eq,
{
self.0.get(key).is_some_and(|v| *v)
}
pub fn iter(&self) -> impl Iterator<Item = &str> + '_ {
// Filter out false values
self.0.iter().filter(|(_, v)| **v).map(|(k, _)| k.as_str())
self.0.iter().filter_map(|(k, v)| (*v).then_some(k.as_str()))
}
}
impl FromIterator<String> for OxlintEnv {
fn from_iter<T: IntoIterator<Item = String>>(iter: T) -> Self {
let map = iter.into_iter().map(|key| (key, true)).collect();
Self(map)
}
}
@ -32,7 +43,6 @@ impl Default for OxlintEnv {
#[cfg(test)]
mod test {
use itertools::Itertools;
use serde::Deserialize;
use super::OxlintEnv;
@ -44,15 +54,15 @@ mod test {
}))
.unwrap();
assert_eq!(env.iter().count(), 2);
assert!(env.iter().contains(&"browser"));
assert!(env.iter().contains(&"node"));
assert!(!env.iter().contains(&"es6"));
assert!(!env.iter().contains(&"builtin"));
assert!(env.contains("browser"));
assert!(env.contains("node"));
assert!(!env.contains("es6"));
assert!(!env.contains("builtin"));
}
#[test]
fn test_parse_env_default() {
let env = OxlintEnv::default();
assert_eq!(env.iter().count(), 1);
assert!(env.iter().contains(&"builtin"));
assert!(env.contains("builtin"));
}
}

View file

@ -1,3 +1,5 @@
use std::borrow::Cow;
use rustc_hash::FxHashMap;
use schemars::JsonSchema;
use serde::Deserialize;
@ -95,25 +97,27 @@ impl Default for JSDocPluginSettings {
impl JSDocPluginSettings {
/// Only for `check-tag-names` rule
/// Return `Some(reason)` if blocked
pub fn check_blocked_tag_name(&self, tag_name: &str) -> Option<String> {
pub fn check_blocked_tag_name(&self, tag_name: &str) -> Option<Cow<str>> {
match self.tag_name_preference.get(tag_name) {
Some(TagNamePreference::FalseOnly(_)) => Some(format!("Unexpected tag `@{tag_name}`.")),
Some(TagNamePreference::ObjectWithMessage { message }) => Some(message.to_string()),
Some(TagNamePreference::FalseOnly(_)) => {
Some(Cow::Owned(format!("Unexpected tag `@{tag_name}`.")))
}
Some(TagNamePreference::ObjectWithMessage { message }) => Some(Cow::Borrowed(message)),
_ => None,
}
}
/// Only for `check-tag-names` rule
/// Return `Some(reason)` if replacement found or default aliased
pub fn check_preferred_tag_name(&self, original_name: &str) -> Option<String> {
let reason = |preferred_name: &str| -> String {
format!("Replace tag `@{original_name}` with `@{preferred_name}`.")
pub fn check_preferred_tag_name(&self, original_name: &str) -> Option<Cow<str>> {
let reason = |preferred_name: &str| -> Cow<str> {
Cow::Owned(format!("Replace tag `@{original_name}` with `@{preferred_name}`."))
};
match self.tag_name_preference.get(original_name) {
Some(TagNamePreference::TagNameOnly(preferred_name)) => Some(reason(preferred_name)),
Some(TagNamePreference::ObjectWithMessageAndReplacement { message, .. }) => {
Some(message.to_string())
Some(Cow::Borrowed(message))
}
_ => {
// https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/settings.md#default-preferred-aliases
@ -163,13 +167,13 @@ impl JSDocPluginSettings {
/// Resolve original, known tag name to user preferred name
/// If not defined, return original name
pub fn resolve_tag_name(&self, original_name: &str) -> String {
pub fn resolve_tag_name<'s>(&'s self, original_name: &'s str) -> &'s str {
match self.tag_name_preference.get(original_name) {
Some(
TagNamePreference::TagNameOnly(replacement)
| TagNamePreference::ObjectWithMessageAndReplacement { replacement, .. },
) => replacement.to_string(),
_ => original_name.to_string(),
) => replacement,
_ => original_name,
}
}
}
@ -198,6 +202,7 @@ enum TagNamePreference {
#[cfg(test)]
mod test {
use serde::Deserialize;
use std::borrow::Cow;
use super::JSDocPluginSettings;
@ -294,9 +299,9 @@ mod test {
.unwrap();
assert_eq!(
settings.check_blocked_tag_name("foo"),
Some("Unexpected tag `@foo`.".to_string())
Some(Cow::Borrowed("Unexpected tag `@foo`."))
);
assert_eq!(settings.check_blocked_tag_name("bar"), Some("do not use bar".to_string()));
assert_eq!(settings.check_blocked_tag_name("bar"), Some(Cow::Borrowed("do not use bar")));
assert_eq!(settings.check_blocked_tag_name("baz"), None);
}
@ -316,10 +321,10 @@ mod test {
.unwrap();
assert_eq!(settings.check_preferred_tag_name("foo"), None,);
assert_eq!(settings.check_preferred_tag_name("bar"), None);
assert_eq!(settings.check_preferred_tag_name("baz"), Some("baz is noop now".to_string()));
assert_eq!(settings.check_preferred_tag_name("baz"), Some("baz is noop now".into()));
assert_eq!(
settings.check_preferred_tag_name("qux"),
Some("Replace tag `@qux` with `@quux`.".to_string())
Some("Replace tag `@qux` with `@quux`.".into())
);
}
}

View file

@ -1,3 +1,5 @@
use std::borrow::Cow;
use schemars::JsonSchema;
use serde::Deserialize;
@ -9,10 +11,10 @@ pub struct NextPluginSettings {
}
impl NextPluginSettings {
pub fn get_root_dirs(&self) -> Vec<String> {
pub fn get_root_dirs(&self) -> Cow<'_, [String]> {
match &self.root_dir {
OneOrMany::One(val) => vec![val.clone()],
OneOrMany::Many(vec) => vec.clone(),
OneOrMany::One(val) => Cow::Owned(vec![val.clone()]),
OneOrMany::Many(vec) => Cow::Borrowed(vec),
}
}
}

View file

@ -86,7 +86,7 @@ impl Rule for NoNamedAsDefaultMember {
.and_then(|symbol_id| has_members_map.get(&symbol_id))
.and_then(|it| {
if it.0.exported_bindings.contains_key(entry_name) {
Some(it.1.to_string())
Some(&it.1)
} else {
None
}
@ -109,7 +109,7 @@ impl Rule for NoNamedAsDefaultMember {
},
&ident.name,
prop_str,
&module_name,
module_name,
));
};
};
@ -136,7 +136,7 @@ impl Rule for NoNamedAsDefaultMember {
decl.span,
&ident.name,
&name,
&module_name,
module_name,
));
}
}

View file

@ -1,4 +1,4 @@
use std::str::FromStr;
use std::{borrow::Cow, str::FromStr};
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::OxcDiagnostic;
@ -17,18 +17,18 @@ use crate::{
};
fn consistent_method(x1: &str, x2: &str, span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Enforce `test` and `it` usage conventions".to_string())
OxcDiagnostic::warn("Enforce `test` and `it` usage conventions")
.with_help(format!("Prefer using {x1:?} instead of {x2:?}"))
.with_label(span0)
}
fn consistent_method_within_describe(x1: &str, x2: &str, span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Enforce `test` and `it` usage conventions".to_string())
OxcDiagnostic::warn("Enforce `test` and `it` usage conventions")
.with_help(format!("Prefer using {x1:?} instead of {x2:?} within describe"))
.with_label(span0)
}
#[derive(Debug, Clone, Eq, PartialEq)]
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
enum TestCaseName {
Fit,
IT,
@ -36,16 +36,21 @@ enum TestCaseName {
Xit,
Xtest,
}
impl TestCaseName {
pub fn as_str(self) -> &'static str {
match self {
Self::Fit => "fit",
Self::IT => "it",
Self::Test => "test",
Self::Xit => "xit",
Self::Xtest => "xtest",
}
}
}
impl std::fmt::Display for TestCaseName {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Fit => write!(f, "fit"),
Self::IT => write!(f, "it"),
Self::Test => write!(f, "test"),
Self::Xit => write!(f, "xit"),
Self::Xtest => write!(f, "xtest"),
}
self.as_str().fmt(f)
}
}
@ -228,69 +233,65 @@ impl ConsistentTestIt {
}
let is_test = matches!(jest_fn_call.kind, JestFnKind::General(JestGeneralFnKind::Test));
let fn_to_str = self.within_fn.to_string();
let fn_to_str = self.within_fn.as_str();
if is_test && describe_nesting_hash.is_empty() && !jest_fn_call.name.ends_with(&fn_to_str) {
let opposite_test_keyword = Self::get_opposite_test_case(&self.within_fn);
if is_test && describe_nesting_hash.is_empty() && !jest_fn_call.name.ends_with(fn_to_str) {
let opposite_test_keyword = Self::get_opposite_test_case(self.within_fn);
if let Some((span, prefer_test_name)) = Self::get_prefer_test_name_and_span(
call_expr.callee.get_inner_expression(),
&jest_fn_call.name,
&fn_to_str,
fn_to_str,
) {
ctx.diagnostic_with_fix(
consistent_method(&fn_to_str, &opposite_test_keyword, span),
consistent_method(fn_to_str, opposite_test_keyword, span),
|fixer| fixer.replace(span, prefer_test_name),
);
}
}
let describe_to_str = self.within_describe.to_string();
let describe_to_str = self.within_describe.as_str();
if is_test
&& !describe_nesting_hash.is_empty()
&& !jest_fn_call.name.ends_with(&describe_to_str)
&& !jest_fn_call.name.ends_with(describe_to_str)
{
let opposite_test_keyword = Self::get_opposite_test_case(&self.within_describe);
let opposite_test_keyword = Self::get_opposite_test_case(self.within_describe);
if let Some((span, prefer_test_name)) = Self::get_prefer_test_name_and_span(
call_expr.callee.get_inner_expression(),
&jest_fn_call.name,
&describe_to_str,
describe_to_str,
) {
ctx.diagnostic_with_fix(
consistent_method_within_describe(
&describe_to_str,
&opposite_test_keyword,
span,
),
consistent_method_within_describe(describe_to_str, opposite_test_keyword, span),
|fixer| fixer.replace(span, prefer_test_name),
);
}
}
}
fn get_opposite_test_case(test_case_name: &TestCaseName) -> String {
fn get_opposite_test_case(test_case_name: TestCaseName) -> &'static str {
if matches!(test_case_name, TestCaseName::Test) {
TestCaseName::IT.to_string()
TestCaseName::IT.as_str()
} else {
TestCaseName::Test.to_string()
TestCaseName::Test.as_str()
}
}
fn get_prefer_test_name_and_span(
fn get_prefer_test_name_and_span<'s>(
expr: &Expression,
test_name: &str,
fix_jest_name: &str,
) -> Option<(Span, String)> {
fix_jest_name: &'s str,
) -> Option<(Span, Cow<'s, str>)> {
match expr {
Expression::Identifier(ident) => {
if ident.name.eq("fit") {
return Some((ident.span, "test.only".to_string()));
return Some((ident.span, Cow::Borrowed("test.only")));
}
let prefer_test_name = match test_name.chars().next() {
Some('x') => format!("x{fix_jest_name}"),
Some('f') => format!("f{fix_jest_name}"),
_ => fix_jest_name.to_string(),
Some('x') => Cow::Owned(format!("x{fix_jest_name}")),
Some('f') => Cow::Owned(format!("f{fix_jest_name}")),
_ => Cow::Borrowed(fix_jest_name),
};
Some((ident.span(), prefer_test_name))
}

View file

@ -19,7 +19,7 @@ use crate::{
};
fn expect_expect_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Test has no assertions".to_string())
OxcDiagnostic::warn("Test has no assertions")
.with_help("Add assertion(s) in this Test")
.with_label(span0)
}

View file

@ -64,7 +64,7 @@ impl Rule for CheckAccess {
let resolved_access_tag_name = settings.resolve_tag_name("access");
let mut access_related_tag_names = FxHashSet::default();
access_related_tag_names.insert(resolved_access_tag_name.to_string());
access_related_tag_names.insert(resolved_access_tag_name);
for level in &ACCESS_LEVELS {
access_related_tag_names.insert(settings.resolve_tag_name(level));
}

View file

@ -101,7 +101,7 @@ impl Rule for CheckPropertyNames {
.map(|span| {
LabeledSpan::at(
(span.start as usize)..(span.end as usize),
"Duplicated property".to_string(),
"Duplicated property",
)
})
.collect::<Vec<_>>();

View file

@ -158,7 +158,7 @@ impl Rule for RequireParam {
}
// Collected JSDoc `@param` tags
let tags_to_check = collect_tags(&jsdocs, &settings.resolve_tag_name("param"));
let tags_to_check = collect_tags(&jsdocs, settings.resolve_tag_name("param"));
let shallow_tags =
tags_to_check.iter().filter(|(name, _)| !name.contains('.')).collect::<Vec<_>>();

View file

@ -226,12 +226,12 @@ impl Rule for RequireReturns {
let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::<Vec<_>>();
let resolved_returns_tag_name = settings.resolve_tag_name("returns");
if is_missing_returns_tag(&jsdoc_tags, &resolved_returns_tag_name) {
if is_missing_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
ctx.diagnostic(missing_returns_diagnostic(*func_span));
continue;
}
if let Some(span) = is_duplicated_returns_tag(&jsdoc_tags, &resolved_returns_tag_name) {
if let Some(span) = is_duplicated_returns_tag(&jsdoc_tags, resolved_returns_tag_name) {
ctx.diagnostic(duplicate_returns_diagnostic(span));
}
}

View file

@ -142,7 +142,7 @@ impl Rule for RequireYields {
// Without this option, need to check `yield` value.
// Check will be performed in `YieldExpression` branch.
if config.force_require_yields
&& is_missing_yields_tag(&jsdoc_tags, &resolved_yields_tag_name)
&& is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name)
{
ctx.diagnostic(missing_yields(func.span));
return;
@ -150,7 +150,7 @@ impl Rule for RequireYields {
// Other checks are always performed
if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, &resolved_yields_tag_name)
if let Some(span) = is_duplicated_yields_tag(&jsdoc_tags, resolved_yields_tag_name)
{
ctx.diagnostic(duplicate_yields(span));
return;
@ -161,8 +161,8 @@ impl Rule for RequireYields {
if let Some(span) = is_missing_yields_tag_with_generator_tag(
&jsdoc_tags,
&resolved_yields_tag_name,
&resolved_generator_tag_name,
resolved_yields_tag_name,
resolved_generator_tag_name,
) {
ctx.diagnostic(missing_yields_with_generator(span));
}
@ -229,7 +229,7 @@ impl Rule for RequireYields {
let jsdoc_tags = jsdocs.iter().flat_map(JSDoc::tags).collect::<Vec<_>>();
let resolved_yields_tag_name = settings.resolve_tag_name("yields");
if is_missing_yields_tag(&jsdoc_tags, &resolved_yields_tag_name) {
if is_missing_yields_tag(&jsdoc_tags, resolved_yields_tag_name) {
ctx.diagnostic(missing_yields(generator_func.span));
}
}

View file

@ -195,7 +195,7 @@ impl Rule for BanTsComment {
if !re.is_match(description) {
ctx.diagnostic(comment_description_not_match_pattern(
directive,
&re.to_string(),
re.as_str(),
comm.span,
));
}

View file

@ -270,9 +270,8 @@ fn generate_fix<'a>(
let mut replace_range = if is_reject { parent.kind().span() } else { call_expr.span() };
let replacement_text = if is_reject {
let mut text =
if arg_text.is_empty() { "undefined".to_string() } else { arg_text.to_string() };
text = format!("throw {text}");
let text = if arg_text.is_empty() { "undefined" } else { arg_text };
let mut text = format!("throw {text}");
if is_yield {
replace_range = get_parenthesized_node(parent, ctx).kind().span();

View file

@ -67,15 +67,14 @@ impl Rule for PreferNodeProtocol {
let module_name = if let Some((prefix, postfix)) = string_lit_value.split_once('/') {
// `e.g. ignore "assert/"`
if postfix.is_empty() {
string_lit_value.to_string()
string_lit_value.as_str()
} else {
prefix.to_string()
prefix
}
} else {
string_lit_value.to_string()
string_lit_value.as_str()
};
if module_name.starts_with("node:")
|| NODEJS_BUILTINS.binary_search(&module_name.as_str()).is_err()
if module_name.starts_with("node:") || NODEJS_BUILTINS.binary_search(&module_name).is_err()
{
return;
}

View file

@ -101,7 +101,7 @@ pub fn should_ignore_as_avoid(
exempted_tag_names: &[String],
) -> bool {
let mut ignore_tag_names =
exempted_tag_names.iter().map(std::convert::Into::into).collect::<FxHashSet<_>>();
exempted_tag_names.iter().map(String::as_str).collect::<FxHashSet<_>>();
if settings.ignore_replaces_docs {
ignore_tag_names.insert(settings.resolve_tag_name("ignore"));
}