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

refactor(rust): apply cllippy::nursery rules #9232

Merged
merged 1 commit into from
Feb 20, 2025
Merged
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
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
Loading