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

fix(linter): remove all* remaining "Disallow <foo>" messages #5812

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
28 changes: 21 additions & 7 deletions crates/oxc_linter/src/rules/eslint/no_array_constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_array_constructor_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow `Array` constructors")
.with_help("Use array literal instead")
OxcDiagnostic::warn("Do not use `new` to create arrays")
.with_help("Use an array literal instead")
.with_label(span)
}

Expand All @@ -16,19 +16,33 @@ pub struct NoArrayConstructor;

declare_oxc_lint!(
/// ### What it does
/// Disallow array constructor
/// Disallows creating arrays with the `Array` constructor.
///
/// ### Why is this bad?
///
/// Use of the Array constructor to construct a new array is generally discouraged in favor of array literal notation because of the single-argument pitfall and because the Array global may be redefined.
/// The exception is when the Array constructor is used to intentionally create sparse arrays of a specified size by giving the constructor a single numeric argument.
/// Use of the `Array` constructor to construct a new array is generally
/// discouraged in favor of array literal notation because of the
/// single-argument pitfall and because the `Array` global may be redefined.
/// The exception is when the `Array` constructor is used to intentionally
/// create sparse arrays of a specified size by giving the constructor a
/// single numeric argument.
///
/// ### Example
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// let arr = new Array();
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// let arr = [];
/// let arr2 = Array.from(iterable);
/// let arr3 = new Array(9);
/// ```
NoArrayConstructor,
pedantic
pedantic,
pending
);

impl Rule for NoArrayConstructor {
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_linter/src/rules/eslint/no_caller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_caller_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow the use of arguments.caller or arguments.callee")
.with_help("'caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them")
fn no_caller_diagnostic(span: Span, method_name: &str) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Do not use `arguments.{method_name}`"))
.with_help("'caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them.")
.with_label(span)
}

Expand Down Expand Up @@ -79,7 +79,7 @@ impl Rule for NoCaller {
if (expr.property.name == "callee" || expr.property.name == "caller")
&& expr.object.is_specific_id("arguments")
{
ctx.diagnostic(no_caller_diagnostic(expr.property.span));
ctx.diagnostic(no_caller_diagnostic(expr.property.span, &expr.property.name));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_empty_diagnostic(stmt_kind: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow empty block statements")
.with_help(format!("Add comment inside empty {stmt_kind} statement"))
.with_label(span.label(format!("Empty {stmt_kind} statement")))
OxcDiagnostic::warn("Unexpected empty block statements")
.with_help(format!("Remove this {stmt_kind} or add a comment inside it"))
.with_label(span)
}

#[derive(Debug, Default, Clone)]
Expand Down
95 changes: 86 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_empty_function.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
use oxc_ast::AstKind;
use std::borrow::Cow;

use oxc_ast::{
ast::{IdentifierName, IdentifierReference, MethodDefinitionKind},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_empty_function_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow empty functions")
.with_help("Unexpected empty function block")
fn no_empty_function_diagnostic<S: AsRef<str>>(
span: Span,
fn_kind: &str,
fn_name: Option<S>,
) -> OxcDiagnostic {
let message = match fn_name {
Some(name) => Cow::Owned(format!("Unexpected empty {fn_kind} `{}`", name.as_ref())),
None => Cow::Borrowed("Unexpected empty function"),
};
OxcDiagnostic::warn(message)
.with_help(format!("Consider removing this {fn_kind} or adding logic to it."))
.with_label(span)
}

Expand All @@ -23,26 +36,90 @@ declare_oxc_lint!(
/// intentional or not. So writing a clear comment for empty functions is a good practice.
///
/// ### Example
/// ```javascript
///
/// Examples of **incorrect** code for this rule:
/// ```javascript
/// function foo() {
/// }
///
/// const bar = () => {};
/// ```
///
/// Examples of **correct** code for this rule:
/// ```javascript
/// function foo() {
/// // do nothing
/// }
///
/// function foo() {
/// return;
/// }
/// const add = (a, b) => a + b
/// ```
NoEmptyFunction,
restriction,
);

impl Rule for NoEmptyFunction {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::FunctionBody(fb) = node.kind() {
if fb.is_empty() && !ctx.semantic().trivias().has_comments_between(fb.span) {
ctx.diagnostic(no_empty_function_diagnostic(fb.span));
}
let AstKind::FunctionBody(fb) = node.kind() else {
return;
};
if fb.is_empty() && !ctx.semantic().trivias().has_comments_between(fb.span) {
let (kind, fn_name) = get_function_name_and_kind(node, ctx);
ctx.diagnostic(no_empty_function_diagnostic(fb.span, kind, fn_name));
}
}
}

fn get_function_name_and_kind<'a>(
node: &AstNode<'a>,
ctx: &LintContext<'a>,
) -> (&'static str, Option<Cow<'a, str>>) {
for parent in ctx.nodes().iter_parents(node.id()).skip(1).map(AstNode::kind) {
match parent {
AstKind::Function(f) => {
if let Some(name) = f.name() {
let kind = if f.generator { "generator function" } else { "function" };
return (kind, Some(name.into()));
}
continue;
}
AstKind::ArrowFunctionExpression(_) => {
continue;
}
AstKind::IdentifierName(IdentifierName { name, .. })
| AstKind::IdentifierReference(IdentifierReference { name, .. }) => {
return ("function", Some(Cow::Borrowed(name.as_str())));
}
AstKind::PropertyDefinition(prop) => {
return ("function", prop.key.name());
}
AstKind::MethodDefinition(method) => {
let kind = match method.kind {
MethodDefinitionKind::Method => {
if method.r#static {
"static method"
} else {
"method"
}
}
MethodDefinitionKind::Get => "getter",
MethodDefinitionKind::Set => "setter",
MethodDefinitionKind::Constructor => "constructor",
};
return (kind, method.key.name());
}
AstKind::VariableDeclarator(decl) => {
return ("function", decl.id.get_identifier().map(Into::into));
}
_ => return ("function", None),
}
}
#[cfg(debug_assertions)]
unreachable!();
#[cfg(not(debug_assertions))]
("function", None)
}

#[test]
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/rules/eslint/no_empty_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_empty_pattern_diagnostic(pattern_type: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow empty destructuring patterns.")
OxcDiagnostic::warn(format!("Empty {pattern_type} binding pattern"))
.with_help("Passing `null` or `undefined` will result in runtime error because `null` and `undefined` cannot be destructured.")
.with_label(
span.label(format!("Empty {pattern_type} binding pattern")),
)
.with_label(span)
}

#[derive(Debug, Default, Clone)]
Expand Down
7 changes: 4 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_empty_static_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_empty_static_block_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow empty static blocks")
.with_help("Unexpected empty static block.")
OxcDiagnostic::warn("Unexpected empty static blocks")
.with_help("Remove this empty block or add content to it.")
.with_label(span)
}

Expand Down Expand Up @@ -47,7 +47,8 @@ declare_oxc_lint!(
/// }
/// ```
NoEmptyStaticBlock,
correctness
correctness,
pending // TODO: add a safe suggestion
);

impl Rule for NoEmptyStaticBlock {
Expand Down
31 changes: 17 additions & 14 deletions crates/oxc_linter/src/rules/eslint/no_eq_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use oxc_syntax::operator::BinaryOperator;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_eq_null_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Use '===' to compare with null")
.with_help("Disallow `null` comparisons without type-checking operators.")
fn no_eq_null_diagnostic(span: Span, suggested_operator: &str) -> OxcDiagnostic {
OxcDiagnostic::warn("Do not use `null` comparisons without type-checking operators.")
.with_help(format!("Use '{suggested_operator}' to compare with null"))
.with_label(span)
}

Expand Down Expand Up @@ -72,22 +72,25 @@ impl Rule for NoEqNull {
& binary_expression.left.is_null()
& bad_operator
{
let suggested_operator = if binary_expression.operator == BinaryOperator::Equality {
" === "
} else {
" !== "
};
ctx.diagnostic_with_dangerous_fix(
no_eq_null_diagnostic(Span::new(
binary_expression.span.start,
binary_expression.span.end,
)),
no_eq_null_diagnostic(
// Span::new(
// binary_expression.span.start,
// binary_expression.span.end,
// )
binary_expression.span,
suggested_operator.trim(),
),
|fixer| {
let start = binary_expression.left.span().end;
let end = binary_expression.right.span().start;
let span = Span::new(start, end);
let new_operator_str =
if binary_expression.operator == BinaryOperator::Equality {
" === "
} else {
" !== "
};
fixer.replace(span, new_operator_str)
fixer.replace(span, suggested_operator)
},
);
}
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{context::LintContext, rule::Rule, AstNode};

fn no_iterator_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Reserved name '__iterator__'")
.with_help("Disallow the use of the `__iterator__` property.")
.with_help("Consider using [Symbol.iterator] instead")
.with_label(span)
}

Expand Down Expand Up @@ -53,7 +53,8 @@ declare_oxc_lint!(
/// };
/// ```
NoIterator,
restriction
restriction,
pending // TODO: suggestion
);

impl Rule for NoIterator {
Expand Down
9 changes: 4 additions & 5 deletions crates/oxc_linter/src/rules/eslint/no_new_wrappers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ use oxc_span::Span;
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_new_wrappers_diagnostic(builtin_name: &str, new_span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow new operators with the String, Number, and Boolean objects")
.with_help(format!(
"do not use {builtin_name} as a constructor, consider removing the new operator."
))
OxcDiagnostic::warn(format!("Do not use `{builtin_name}` as a constructor"))
.with_help("Remove the `new` operator.")
.with_label(new_span)
}

Expand Down Expand Up @@ -48,7 +46,8 @@ declare_oxc_lint!(
/// var booleanObject = Boolean(value);
/// ```
NoNewWrappers,
pedantic
pedantic,
pending
);

impl Rule for NoNewWrappers {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_obj_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const GLOBAL_THIS: &str = "globalThis";
const NON_CALLABLE_GLOBALS: [&str; 5] = ["Atomics", "Intl", "JSON", "Math", "Reflect"];

fn no_obj_calls_diagnostic(obj_name: &str, span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Disallow calling some global objects as functions")
.with_help(format!("{obj_name} is not a function."))
OxcDiagnostic::warn(format!("`{obj_name}` is not a function and cannot be called"))
.with_help("This call will throw a TypeError at runtime.")
.with_label(span)
}

Expand Down
11 changes: 7 additions & 4 deletions crates/oxc_linter/src/rules/eslint/no_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{context::LintContext, rule::Rule, AstNode};

fn no_proto_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("The '__proto__' property is deprecated")
.with_help("Disallow the use of the `__proto__` property.")
.with_help("use `Object.getPrototypeOf` and `Object.setPrototypeOf` instead.")
.with_label(span)
}

Expand All @@ -16,10 +16,12 @@ pub struct NoProto;

declare_oxc_lint!(
/// ### What it does
/// Disallow the use of the __proto__ property
/// Disallow the use of the `__proto__` property
///
/// ### Why is this bad?
/// __proto__ property has been deprecated as of ECMAScript 3.1 and shouldn’t be used in the code. Use Object.getPrototypeOf and Object.setPrototypeOf instead.
/// The `__proto__` property has been deprecated as of ECMAScript 3.1 and
/// shouldn’t be used in new code. Use `Object.getPrototypeOf` and
/// `Object.setPrototypeOf` instead.
///
/// ### Example
/// ```javascript
Expand All @@ -34,7 +36,8 @@ declare_oxc_lint!(
/// obj["__proto__"] = b;
/// ```
NoProto,
restriction
restriction,
pending
);

impl Rule for NoProto {
Expand Down
Loading