Skip to content

Commit

Permalink
refactor(rust): apply cllippy::nursery rules (#9232)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Feb 20, 2025
1 parent 63bb214 commit 9f36181
Show file tree
Hide file tree
Showing 56 changed files with 119 additions and 111 deletions.
13 changes: 13 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,28 @@ wildcard_imports = "allow"
# used_underscore_binding= "allow"
doc_markdown = "allow"
# nursery
nursery = { level = "warn", priority = -1 }
# `const` functions do not make sense for our project because this is not a `const` library.
# This rule also confuses newcomers and forces them to add `const` blindlessly without any reason.
missing_const_for_fn = "allow"
option_if_let_else = "allow"
or_fun_call = "allow"
cognitive_complexity = "allow"
non_send_fields_in_send_ty = "allow"
use_self = "allow"
significant_drop_tightening = "allow"
branches_sharing_code = "allow"
fallible_impl_from = "allow"
useless_let_if_seq = "allow"
significant_drop_in_scrutinee = "warn"
iter_on_single_items = "warn"
unused_peekable = "warn"
too_long_first_doc_paragraph = "warn"
suspicious_operation_groupings = "warn"
redundant_clone = "warn"
redundant_pub_crate = "allow" # FIXME
debug_assert_with_mut_call = "allow" # FIXME
needless_pass_by_ref_mut = "allow" # FIXME
# cargo
cargo = { level = "warn", priority = -1 }
multiple_crate_versions = "allow"
Expand Down
2 changes: 1 addition & 1 deletion apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl Runner for LintRunner {

if fs::write(Self::DEFAULT_OXLINTRC, configuration).is_ok() {
stdout
.write_all("Configuration file created\n".as_bytes())
.write_all(b"Configuration file created\n")
.or_else(Self::check_for_writer_error)
.unwrap();
stdout.flush().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion apps/oxlint/src/output_formatter/junit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn format_junit(diagnostics: &[Error]) -> String {
let rule = diagnostic.code().map_or_else(String::new, |code| code.to_string());
let Info { message, start, .. } = Info::new(diagnostic);

let severity = if let Some(Severity::Error) = diagnostic.severity() {
let severity = if diagnostic.severity() == Some(Severity::Error) {
total_errors += 1;
error += 1;
"error"
Expand Down
8 changes: 4 additions & 4 deletions apps/oxlint/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ impl Tester {
let options = lint_command().run_inner(*args).unwrap();
let args_string = args.join(" ");

output.extend_from_slice("########## \n".as_bytes());
output.extend_from_slice(b"########## \n");
output.extend_from_slice(format!("arguments: {args_string}\n").as_bytes());
output.extend_from_slice(
format!("working directory: {}\n", relative_dir.to_str().unwrap()).as_bytes(),
);
output.extend_from_slice("----------\n".as_bytes());
output.extend_from_slice(b"----------\n");
let result = LintRunner::new(options).with_cwd(self.cwd.clone()).run(&mut output);

output.extend_from_slice("----------\n".as_bytes());
output.extend_from_slice(b"----------\n");
output.extend_from_slice(format!("CLI result: {result:?}\n").as_bytes());
output.extend_from_slice("----------\n".as_bytes());
output.extend_from_slice(b"----------\n");

output.push(b'\n');
}
Expand Down
20 changes: 12 additions & 8 deletions crates/oxc_cfg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,19 @@ impl ControlFlowGraph {

// if there is exactly one and it is a condition instruction we are in a loop so we
// check the condition to infer if it is always true.
if let EvalConstConditionResult::Eval(true) = try_eval_const_condition(only_instruction) {
if matches!(
try_eval_const_condition(only_instruction),
EvalConstConditionResult::Eval(true)
) {
get_jump_target(&self.graph, node).map(|it| (it, node))
} else if let EvalConstConditionResult::Eval(true) = self
.basic_block(backedge.source())
.instructions()
.iter()
.exactly_one()
.map_or_else(|_| EvalConstConditionResult::NotFound, try_eval_const_condition)
{
} else if matches!(
self.basic_block(backedge.source())
.instructions()
.iter()
.exactly_one()
.map_or_else(|_| EvalConstConditionResult::NotFound, try_eval_const_condition),
EvalConstConditionResult::Eval(true)
) {
get_jump_target(&self.graph, node).map(|it| (node, it))
} else {
None
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_codegen/src/binary_expr_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use oxc_syntax::{
use crate::{gen::GenExpr, Codegen, Context, Operator};

#[derive(Clone, Copy)]
pub(crate) enum Binaryish<'a> {
pub enum Binaryish<'a> {
Binary(&'a BinaryExpression<'a>),
Logical(&'a LogicalExpression<'a>),
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_codegen/src/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use oxc_syntax::identifier::is_line_terminator;

use crate::{Codegen, LegalComment};

pub(crate) type CommentsMap = FxHashMap</* attached_to */ u32, Vec<Comment>>;
pub type CommentsMap = FxHashMap</* attached_to */ u32, Vec<Comment>>;

impl Codegen<'_> {
pub(crate) fn build_comments(&mut self, comments: &[Comment]) {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_codegen/src/operator.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use oxc_syntax::operator::{BinaryOperator, UnaryOperator, UpdateOperator};

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) enum Operator {
pub enum Operator {
Binary(BinaryOperator),
Unary(UnaryOperator),
Update(UpdateOperator),
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_codegen/src/sourcemap_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ mod test {

#[test]
fn add_source_mapping_for_name() {
let output = "ac".as_bytes();
let output = b"ac";
let mut builder = SourcemapBuilder::new(Path::new("x.js"), "ab");
builder.add_source_mapping_for_name(output, Span::new(0, 1), "a");
builder.add_source_mapping_for_name(output, Span::new(1, 2), "c");
Expand All @@ -556,7 +556,7 @@ mod test {

#[test]
fn add_source_mapping_for_unordered_position() {
let output = "".as_bytes();
let output = b"";
let mut builder = SourcemapBuilder::new(Path::new("x.js"), "ab");
builder.add_source_mapping(output, 1, None);
builder.add_source_mapping(output, 0, None);
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ecmascript/src/to_primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl ToPrimitive<'_> for Expression<'_> {
}
}

pub(crate) fn maybe_object_with_to_primitive_related_properties_overridden(
pub fn maybe_object_with_to_primitive_related_properties_overridden(
obj: &ObjectExpression<'_>,
) -> bool {
obj.properties.iter().any(|prop| match prop {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/config_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{rules::RULES, RuleWithSeverity};

// TODO: support `categories` et. al. in overrides.
#[derive(Debug)]
pub(crate) struct ResolvedLinterState {
pub struct ResolvedLinterState {
// TODO: Arc + Vec -> SyncVec? It would save a pointer dereference.
pub rules: Arc<[RuleWithSeverity]>,
pub config: Arc<LintConfig>,
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
/// list of
/// environments](https://eslint.org/docs/v8.x/use/configure/language-options#specifying-environments)
/// for what environments are available and what each one provides.
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
pub struct OxlintEnv(FxHashMap<String, bool>);

impl FromIterator<String> for OxlintEnv {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use serde::{de::Visitor, Deserialize, Serialize};
/// You may also use `"readable"` or `false` to represent `"readonly"`, and
/// `"writeable"` or `true` to represent `"writable"`.
// <https://eslint.org/docs/v8.x/use/configure/language-options#using-configuration-files-1>
#[derive(Debug, Default, PartialEq, Deserialize, Serialize, JsonSchema, Clone)]
#[derive(Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema, Clone)]
pub struct OxlintGlobals(FxHashMap<String, GlobalValue>);

impl Deref for OxlintGlobals {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod rules;
mod settings;
pub use config_builder::{ConfigBuilderError, ConfigStoreBuilder};
pub use config_store::ConfigStore;
pub(crate) use config_store::ResolvedLinterState;
pub use config_store::ResolvedLinterState;
pub use env::OxlintEnv;
pub use globals::{GlobalValue, OxlintGlobals};
pub use overrides::OxlintOverrides;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/plugins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use serde::{

bitflags! {
// NOTE: may be increased to a u32 if needed
#[derive(Debug, Clone, Copy, PartialEq, Hash)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct LintPlugins: u16 {
/// Not really a plugin. Included for completeness.
const ESLINT = 0;
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/config/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ type RuleSet = FxHashSet<RuleWithSeverity>;
// - type RuleConf = SeverityConf | [SeverityConf, ...any[]];
// <https://github.com/eslint/eslint/blob/ce838adc3b673e52a151f36da0eedf5876977514/lib/shared/types.js#L12>
// Note: when update document comment, also update `DummyRuleMap`'s description in this file.
#[derive(Debug, Clone, Default)]
#[cfg_attr(test, derive(PartialEq))]
#[derive(Debug, Clone, Default, Eq, PartialEq)]
pub struct OxlintRules {
/// List of all configured rules
pub(crate) rules: Vec<ESLintRule>,
Expand All @@ -46,8 +45,7 @@ impl OxlintRules {
/// e.g. `eslint/no-console` or `react/rule-of-hooks`.
/// Includes the plugin name, the rule name, and the configuration for the rule (if any).
/// This does not imply the rule is known to the linter as that, only that it is configured.
#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct ESLintRule {
/// Name of the plugin: `eslint`, `react`, etc.
pub plugin_name: String,
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/config/settings/jsx_a11y.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
/// [eslint-plugin-jsx-a11y](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y#configurations)'s
/// configuration for a full reference.
#[derive(Debug, Clone, Deserialize, Default, Serialize, JsonSchema)]
#[cfg_attr(test, derive(PartialEq))]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub struct JSXA11yPluginSettings {
/// An optional setting that define the prop your code uses to create polymorphic components.
/// This setting will be used to determine the element type in rules that
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_linter/src/context/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use super::{plugin_name_to_prefix, LintContext};
/// - [Flyweight Pattern](https://en.wikipedia.org/wiki/Flyweight_pattern)
#[must_use]
#[non_exhaustive]
pub(crate) struct ContextHost<'a> {
pub struct ContextHost<'a> {
/// Shared semantic information about the file being linted, which includes scopes, symbols
/// and AST nodes. See [`Semantic`].
pub(super) semantic: Rc<Semantic<'a>>,
Expand Down Expand Up @@ -106,7 +106,6 @@ impl<'a> ContextHost<'a> {

/// Set the linter configuration for this context.
#[inline]
#[expect(dead_code)] // will be used in up-stack PR
pub fn with_config(mut self, config: &Arc<LintConfig>) -> Self {
let plugins = config.plugins;
self.config = Arc::clone(config);
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
};

mod host;
pub(crate) use host::ContextHost;
pub use host::ContextHost;

/// Contains all of the state and context specific to this lint rule.
///
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/frameworks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl FrameworkFlags {
}

/// <https://jestjs.io/docs/configuration#testmatch-arraystring>
pub(crate) fn is_jestlike_file(path: &Path) -> bool {
pub fn is_jestlike_file(path: &Path) -> bool {
use std::ffi::OsStr;

if path.components().any(|c| match c {
Expand All @@ -88,10 +88,10 @@ pub(crate) fn is_jestlike_file(path: &Path) -> bool {
.is_some_and(|name_or_first_ext| name_or_first_ext == "test" || name_or_first_ext == "spec")
}

pub(crate) fn has_vitest_imports(module_record: &ModuleRecord) -> bool {
pub fn has_vitest_imports(module_record: &ModuleRecord) -> bool {
module_record.import_entries.iter().any(|entry| entry.module_request.name() == "vitest")
}

pub(crate) fn has_jest_imports(module_record: &ModuleRecord) -> bool {
pub fn has_jest_imports(module_record: &ModuleRecord) -> bool {
module_record.import_entries.iter().any(|entry| entry.module_request.name() == "@jest/globals")
}
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/loader/partial_loader/astro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a> AstroPartialLoader<'a> {
break;
};
// check for the / of a self closing script tag
if let Some('/') = self.source_text.chars().nth(js_start - 2) {
if self.source_text.chars().nth(js_start - 2) == Some('/') {
js_end = pointer;
// find "</script>" if no self closing tag was found
} else if let Some(offset) =
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/options/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use super::AllowWarnDeny;
/// 2. Filter an entire category: `correctness`
/// 3. Some unknow filter. This is a fallback used when parsing a filter string,
/// and is interpreted uniquely by the linter.
#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct LintFilter {
severity: AllowWarnDeny,
kind: LintFilterKind,
Expand Down Expand Up @@ -77,8 +76,7 @@ impl<'a> From<&'a LintFilter> for (AllowWarnDeny, &'a LintFilterKind) {
}
}

#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum LintFilterKind {
Generic(Cow<'static, str>),
/// e.g. `no-const-assign` or `eslint/no-const-assign`
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{fixer::FixKind, FrameworkFlags};

/// Subset of options used directly by the linter.
#[derive(Debug, Default, Clone, Copy)]
#[cfg_attr(test, derive(PartialEq))]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub struct LintOptions {
pub fix: FixKind,
pub framework_hints: FrameworkFlags,
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_linter/src/rules/eslint/for_direction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,19 @@ impl Rule for ForDirection {
span.end = update.argument.span().start;
}

if let UpdateOperator::Increment = update.operator {
if update.operator == UpdateOperator::Increment {
new_operator_str = "--";
} else if let UpdateOperator::Decrement = update.operator {
} else if update.operator == UpdateOperator::Decrement {
new_operator_str = "++";
}
}
Expression::AssignmentExpression(update) => {
span.start = update.left.span().end;
span.end = update.right.span().start;

if let AssignmentOperator::Addition = update.operator {
if update.operator == AssignmentOperator::Addition {
new_operator_str = "-=";
} else if let AssignmentOperator::Subtraction = update.operator {
} else if update.operator == AssignmentOperator::Subtraction {
new_operator_str = "+=";
}
}
Expand Down
24 changes: 10 additions & 14 deletions crates/oxc_linter/src/rules/eslint/func_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ impl Rule for FuncStyle {
}
}

if let Some(NamedExports::Expression) = self.named_exports {
if matches!(parent.kind(), AstKind::ExportNamedDeclaration(_)) {
ctx.diagnostic(func_style_diagnostic(func.span, self.style));
}
if self.named_exports == Some(NamedExports::Expression)
&& matches!(parent.kind(), AstKind::ExportNamedDeclaration(_))
{
ctx.diagnostic(func_style_diagnostic(func.span, self.style));
}
}
FunctionType::FunctionExpression => {
Expand All @@ -266,12 +266,10 @@ impl Rule for FuncStyle {
ctx.diagnostic(func_style_diagnostic(decl.span, self.style));
}

if let Some(NamedExports::Declaration) = self.named_exports {
if is_ancestor_export {
ctx.diagnostic(func_style_diagnostic(
decl.span, self.style,
));
}
if self.named_exports == Some(NamedExports::Declaration)
&& is_ancestor_export
{
ctx.diagnostic(func_style_diagnostic(decl.span, self.style));
}
}
}
Expand Down Expand Up @@ -310,10 +308,8 @@ impl Rule for FuncStyle {
ctx.diagnostic(func_style_diagnostic(decl.span, self.style));
}

if let Some(NamedExports::Declaration) = self.named_exports {
if is_ancestor_export {
ctx.diagnostic(func_style_diagnostic(decl.span, self.style));
}
if self.named_exports == Some(NamedExports::Declaration) && is_ancestor_export {
ctx.diagnostic(func_style_diagnostic(decl.span, self.style));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/getter_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,10 @@ impl GetterReturn {
});

let does_return = return_instruction.is_some_and(|ret| {
!matches! { ret.kind,
!matches!( ret.kind,
InstructionKind::Return(ReturnInstructionKind::ImplicitUndefined)
if !self.allow_implicit
}
)
});

// Return true as the second argument to signify we should
Expand Down
Loading

0 comments on commit 9f36181

Please sign in to comment.